Valores

Se não está quebrado, não conserte

Boa parte do “design” do Darktable começou com “seria legal se pudéssemos …”. Vou te dizer o que é legal: pendurar boas fotos suas nas suas paredes o quanto antes. As artes visuais não são artes performáticas (como a música ou o teatro), então só o resultado importa. Tudo que vem antes é sobrecarga, e normalmente você quer mantê-la mínima. Isso não quer dizer que o processo não possa ser apreciado em si mesmo. No entanto, para apreciar o processo, você precisa dominar suas ferramentas e dobrá-las à sua vontade, caso contrário você só briga com elas e o processo todo se resume a frustração. O problema é que o “design” do Darktable investe esforço demais em ser diferente só por ser diferente.

Nesse processo de adicionar “coisas novas legais”, o Darktable quebrou atalhos de teclado e um monte de comportamentos básicos da GUI, substituindo código limpo por espaguete e adicionando mais bagunça na GUI sem nunca podar nada.

O Ansel tem um processo de design explícito que obrigatoriamente começa com problemas definidos, enfrentados por usuários definidos. Acontece que a quantidade de código a escrever é inversamente proporcional à quantidade de reflexão que você fez sobre a sua solução, normalmente para identificar o problema de raiz a partir do que os usuários te dizem, e encontrar o caminho mais simples até a solução (que muitas vezes nem sequer é uma solução de software…).

Mas os bugs não te esperam na reflexão, eles esperam só no código que você escreveu. Então, quanto mais você reflete, menos você codifica, e menos carga de manutenção você cria para si mesmo no futuro. Mas é claro… você precisa ter tempo suficiente para pensar as coisas a fundo. Essencialmente, isso significa adeus à programação amadora de tarde de sábado!

Não estenda se você não consegue simplificar primeiro

Boa parte da programação do Darktable foi feita copiando e colando código, de outras partes do software, ou até de outros projetos, principalmente porque os colaboradores não têm tempo nem habilidades para empreender grandes reescritas. Isso provoca duplicação de código e aumenta o comprimento das funções, adicionando ramificações internas e introduzindo if e switch case aninhados às vezes em mais de 4 níveis, tornando a estrutura e a lógica mais difíceis de compreender e os bugs mais difíceis (e frustrantes) de caçar, ao mesmo tempo em que se tornam mais prováveis de acontecer.

De qualquer forma, quando o código responsável pelos recursos existentes só faz crescer (às vezes por um fator de 10 ao longo de 4 anos), isso levanta sérias questões quanto à manutenibilidade futura, num contexto em que os colaboradores permanecem por não mais que alguns anos, e os desenvolvedores têm um tempo limitado para investir. É simplesmente irresponsável, pois sacrifica a manutenibilidade de longo prazo em troca de coisas novas e brilhantes.

Simplificar e generalizar o código, por meio de APIs limpas, antes de adicionar novos recursos é imprescindível, e o Ansel só aceita código que eu pessoalmente entendo e tenho as habilidades para manter. KISS.

Lógica básica de codificação

Pull requests que não atendam aos requisitos mínimos de qualidade de código não serão aceitas. Esses requisitos visam garantir a manutenibilidade e a estabilidade de longo prazo, impondo um código claro e legível, estruturado com uma lógica simples.

  1. Os procedimentos precisam ser divididos em funções unitárias e reutilizáveis, sempre que possível. A exceção a isso são procedimentos lineares especializados (sem ramificações) que fazem tarefas específicas demais para serem reutilizadas em qualquer lugar, mas, nesse caso, use comentários para dividir os procedimentos em “capítulos” ou etapas que possam ser facilmente identificados e compreendidos.
  2. As funções devem realizar apenas uma tarefa por vez. Por exemplo, o código da GUI não deve ser misturado com código de SQL ou de processamento de pixels. Getters e setters devem ser funções diferentes.
  3. As funções devem ter apenas um ponto de entrada e um ponto de saída (return). As únicas exceções aceitas são um retorno antecipado caso o buffer de memória sobre o qual a função deveria operar não esteja inicializado, ou caso um mutex lock de thread já esteja capturado.
  4. As funções devem ter nomes legíveis e explícitos, e nomes de argumentos que anunciem seu propósito. Os programas são feitos para serem lidos por seres humanos; se você programa para a máquina, faça-o em binário.
  5. As funções só podem aninhar até 2 estruturas condicionais if. Se forem necessários mais de 2 if aninhados, a estrutura do seu código precisa ser reavaliada e provavelmente dividida em funções mais granulares.
  6. O if só deve testar casos uniformes, como o estado ou o valor de idealmente uma (mas talvez mais) variável(is) do mesmo tipo. Se casos não uniformes precisarem ser testados (como IF user param IS value AND picture buffer IS initialized AND picture IS raw AND picture HAS embedded color profile AND color profile coeff[0] IS NOT NaN), eles devem ser delegados a uma função de verificação que retorne um gboolean TRUE ou FALSE e nomeada adequadamente, para que os demais desenvolvedores entendam o propósito da verificação sem ambiguidade numa leitura superficial do código, como color_matrix_should_apply(). O código de ramificação então será if(color_matrix_should_apply()) pix_out = dot_product(pix_in, matrix);
  7. Os comentários devem mencionar por que você fez o que fez, como suas suposições de base, seus motivos e qualquer referência acadêmica ou de documentação que você usou como base (DOI e URLs devem estar ali). Seu código deve dizer o que você fez de forma explícita. Se você se pega tendo que explicar em comentários o que o seu código faz, geralmente é um sinal de que o seu código está mal estruturado, as variáveis e funções estão mal nomeadas, etc.
  8. Soluções paliativas rápidas que escondem problemas em vez de atacá-los na raiz não serão aceitas. Se você tem interesse nisso, considere contribuir com o darktable a montante. As únicas exceções serão se os problemas forem bloqueadores (fizerem o software travar) e nenhuma solução melhor tiver sido encontrada após uma quantidade decente de tempo investido em pesquisa.
  9. Lembre-se sempre de que o melhor código é o mais simples. KISS. Para alcançar esse objetivo, geralmente é melhor escrever o código do zero do que tentar combinar pedaços de código existente por meio de copiar-e-colar pesado.

Em um mundo ideal, qualquer PR seguiria as melhores práticas de padrões de projeto .

Alguns pedaços aleatórios de sabedoria da internet:

Todo mundo sabe que depurar é duas vezes mais difícil do que escrever um programa em primeiro lugar. Então, se você for tão esperto quanto consegue ser ao escrevê-lo, como é que você vai conseguir depurá-lo? — Brian W. Kernighan
Qualquer tolo consegue escrever código que um computador entenda. Bons programadores escrevem código que os seres humanos entendam. — Martin Fowler, Refactoring: Improving the Design of Existing Code
Sempre programe como se o cara que acabar dando manutenção no seu código fosse um psicopata violento que sabe onde você mora. — John Woods
Sempre que tenho que pensar para entender o que o código está fazendo, me pergunto se consigo refatorar o código para tornar esse entendimento mais imediatamente aparente. — Martin Fowler, Refactoring: Improving the Design of Existing Code
Código é ruim. Ele apodrece. Exige manutenção periódica. Tem bugs que precisam ser encontrados. Novos recursos significam que código antigo tem que ser adaptado. Quanto mais código você tem, mais lugares há para os bugs se esconderem. Mais tempo levam os checkouts ou as compilações. Mais tempo leva para um novo funcionário entender o seu sistema. Se você tiver que refatorar, há mais coisas para mover de lugar.
Código é produzido por engenheiros. Para fazer mais código, são necessários mais engenheiros. Engenheiros têm custos de comunicação de n^2, e todo aquele código que eles adicionam ao sistema, embora expanda sua capacidade, também aumenta uma cesta inteira de custos. Você deveria fazer tudo o que for possível para aumentar a produtividade dos programadores individuais em termos do poder expressivo do código que escrevem. Menos código para fazer a mesma coisa (e possivelmente melhor). Menos programadores para contratar. Menos custos de comunicação organizacional.
— Rich Skrenta 
Bons programadores escrevem bom código. Grandes programadores não escrevem código nenhum. Programadores zen deletam código. — John Byrd 

Lógica específica de codificação em C

O Ansel, assim como o darktable, é escrito em C. Essa linguagem é feita para programadores avançados escreverem bugs rápidos em sistemas operacionais e aplicações de nível de sistema. Ela dá liberdade demais para fazer coisas prejudiciais e não pode ser depurada antes de executar o programa, ou de escrever seus próprios testes (que podem ter bugs eles mesmos, ou podem enviesar o tipo de bugs que deixam passar, e, de todo modo, ninguém escreve testes). Ainda assim, a maioria dos colaboradores não é treinada em C, muitos deles nem sequer são programadores profissionais (embora programadores profissionais de C provavelmente não devessem ser deixados perto de aplicações para o usuário final), então C é uma linguagem perigosa para qualquer aplicação de código aberto.

O C vai deixar você escrever em buffers que não foram alocados (resultando em erro de segfault) e vai deixar você liberá-los mais de uma vez, mas não vai liberar buffers quando eles não forem mais necessários (resultando em vazamentos de memória se você esqueceu de fazê-lo manualmente). O problema é que, como a alocação/liberação de buffers pode estar longe (tanto no tempo de vida do programa quanto no código-fonte) de onde você os usa, é fácil bagunçar isso. O C também vai deixar você fazer cast de qualquer ponteiro para qualquer tipo de dado, o que possibilita muitos erros de programação e corrupção de dados. Os métodos nativos de manipulação de strings não são seguros (por razões que nunca me dei o trabalho de entender), então temos que usar os da GLib para prevenir exploits de segurança.

Basicamente, o C faz de você o seu próprio e pior inimigo, e cabe a você observar regras de segurança cuja sabedoria só se tornará clara quando você as quebrar. Muito parecido com os bugs em um programa C. Considere que você escreve o seu código para ser lido por leigos que nunca programaram em C antes.

Você também precisa ter em mente que o compilador fará a maior parte das otimizações por você, mas será superconservador quanto a elas. A regra geral é: se o seu código é facilmente compreensível por um ser humano (lógica simples), ele será devidamente compreendido pelo compilador, que tomará as medidas de otimização apropriadas. No sentido inverso, otimizações manuais no código, que produzem um código enigmático supostamente mais rápido em sistemas de thread único, geralmente saem pela culatra e produzem programas mais lentos após a compilação.

Padrões e estruturas

  1. Laços for são reservados para iterar sobre arrays de tamanho conhecido de antemão, de modo que o número de passos do laço é conhecido. Estendendo essa lógica, eles também podem ser usados para iterar sobre itens GList * (que não têm propriedade de tamanho, já que são alocados dinamicamente), embora isso verifique se cada item (GList *)->next não é NULL. Laços for geralmente não devem usar declarações break ou return dentro do seu fluxo de controle, a menos que o laço esteja procurando por um item específico dentro do array e o retorne assim que ele for encontrado. Se o seu laço tem uma condição de parada, use while.

  2. C não é uma linguagem orientada a objetos, mas você pode e deve usar a lógica OO quando relevante em C, usando estruturas para armazenar dados e ponteiros para métodos, e então getters e setters  uniformes para definir e acessar os dados.

  3. estruturas como while, for, if ou switch não devem ser aninhadas em mais de 3 (e preferencialmente 2) níveis. Use funções se isso acontecer:

     1// Bad
 2void stuff(float *array, char *output)
 3{
 4  if(condition)
 5  {
 6    for(int i = 0; i < 5; i++)
 7    {
 8      if(array[i] > 1.f)
 9        array[i] = ...
10    }
11    output = "true";
12  }
13  else
14  {
15    ...
16  }
17}
18
19// Good
20char *_process(float *array)
21{
22  for(int i = 0; i < 5; i++)
23  {
24    if(array[i] > 1.f)
25      array[i] = ...
26  }
27  return "true";
28}
29void stuff(float *array, char *output)
30{
31  if(condition)
32  {
33    output = _process(array);
34  }
35  else
36  {
37    output = _something_else(array);
38  }
39}

  4. Longas sequências de verificações devem ser colocadas em funções que retornam gboolean, declarando claramente o que estamos verificando, de modo que, nos procedimentos, obtemos:

     1gboolean _is_raw(dt_image_t *image)
 2{
 3  return (image->flag & DT_RAW == DT_RAW) &&
 4         (image->buffer != NULL) &&
 5         strcmp(image->ext, "dng");
 6}
 7
 8void stuff(dt_image_t *image)
 9{
10  if(_is_raw(image))
11    ...
12  else if(_is_raster(image))
13    ...
14}

    em vez de

    1if((image->flag & DT_RAW == DT_RAW) && (image->buffer != NULL) && strcmp(image->ext, "dng"))
2  ...
3else if(...)
4  ...

  5. Sempre acesse dados dos buffers usando a sintaxe de array, a partir do ponteiro base deles, em vez de usar ponteiros não constantes sobre os quais você faz aritmética. Por exemplo, faça:

    1float *const buffer = malloc(64 * sizeof(float));
2for(int i = 0; i < 64; i++)
3{
4  buffer[i] = ...
5}

    Não faça:

    1float *buffer = malloc(64 * sizeof(float));
2for(int i = 0; i < 64; i++)
3{
4  *buffer++ = ...
5}

    A última versão não só é menos clara de ler, como também impedirá a paralelização e as otimizações do compilador, porque o valor do ponteiro depende da iteração do laço e precisaria ser compartilhado entre as threads, caso houvesse alguma. A primeira versão leva a uma lógica de acesso à memória independente da iteração do laço e pode ser paralelizada com segurança.

  6. O uso de incrementos de variável inline (veja um exemplo de pesadelo aqui ) é estritamente proibido, a menos que seja a única operação da linha. Eles são uma bagunça que dá margem a muitos erros de programação. Isto é permitido:

    1uint32_t counter;
2for(int i = 0; i < 64; i++)
3{
4  if(array[i] > threshold)
5    counter++;
6}

  7. As declarações case na estrutura switch não devem ser aditivas. Não faça:

     1int tmp = 0;
 2switch(var)
 3{
 4  case VALUE1:
 5  case VALUE2:
 6    tmp += 1;
 7  case VALUE3:
 8    do_something(tmp);
 9    break;
10  case VALUE4:
11    do_something_else();
12    break;
13}

    Numa leitura superficial, não ficará imediatamente claro que o caso VALUE3 herda as cláusulas definidas pelos casos anteriores, especialmente em situações onde há mais casos. Faça:

     1int tmp = 0;
 2switch(var)
 3{
 4  case VALUE1:
 5  case VALUE2:
 6    do_something(tmp + 1);
 7    break;
 8  case VALUE3:
 9    do_something(tmp);
10    break;
11  case VALUE4:
12    do_something_else();
13    break;
14}

    Cada caso é autocontido e o resultado não depende da ordem de declaração dos casos.

  8. Ordene e armazene suas variáveis em estruturas que você passa como argumentos de função, em vez de usar funções com mais de 8 argumentos. Não faça:

     1void function(float value, gboolean is_green, gboolean is_big, gboolean has_hair, int width, int height, ...)
 2{
 3  ...
 4}
 5
 6void main()
 7{
 8  if(condition1)
 9    function(3.f, TRUE, FALSE, TRUE, 80, 90, ...);
10  else if(condition2)
11    function(3.f, FALSE, TRUE, TRUE, 80, 90, ...);
12  else
13    function(3.f, FALSE, FALSE, FALSE, 110, 90, ...);
14}

    Faça:

     1typedef struct params_t
 2{
 3  gboolean is_green;
 4  gboolean is_big;
 5  gboolean has_hair;
 6  int width;
 7  int height;
 8} params_t;
 9
10void function(float value, params_t p)
11{
12  ...
13}
14
15void main()
16{
17  params_t p = { .is_green = (condition1),
18                .is_big = (condition2),
19                .has_hair = (condition1 || condition2),
20                .width =  (condition1 || condition2) ? 80 : 110,}
21                .height = 90 };
22  function(3.0f, p);
23}

    O primeiro exemplo foi tirado do darktable . O copiar-e-colar das chamadas de função é desnecessário e a multiplicação de argumentos posicionais torna impossível lembrar qual é qual. Ele também não mostra quais argumentos são constantes entre os diferentes ramos, o que tornará a refatoração difícil. O último exemplo não é mais conciso, no entanto a estrutura não só torna a função mais fácil de chamar, como a declaração da estrutura permite definir explicitamente cada argumento, com verificações inline se necessário. A dependência dos argumentos de entrada em relação às condições externas também fica imediatamente clara, e os argumentos booleanos são definidos diretamente a partir das condições, o que tornará o programa mais fácil de estender no futuro e menos propenso a erros de programação decorrentes de mal-entendidos na dependência das variáveis.

Otimizações OpenMP

Os pixels são, essencialmente, vetores RGBA 4D. Desde 2004, os processadores têm capacidades especiais para processar vetores e aplicar Instruções Únicas sobre Múltiplos Dados (SIMD). Isso nos permite acelerar os cálculos processando um pixel inteiro (SSE2) até 4 pixels (AVX-512) ao mesmo tempo, economizando muitos ciclos de CPU.

Os compiladores modernos têm opções de autovetorização que podem otimizar C puro, e a biblioteca OpenMP permite fornecer dicas para melhorar isso, desde que o código seja escrito de forma vetorizável e use alguns pragmas.

Escreva código vetorizável: https://info.ornl.gov/sites/publications/files/Pub69214.pdf 

Melhores práticas para a autovetorização:

  • evite ramificações em laços que alteram o fluxo de controle. Use declarações inline como absolute = (x > 0) ? x : -x; para que possam ser convertidas em máscaras de bytes em SIMD,
  • os pixels só devem ser referenciados a partir do ponteiro base do seu array e dos índices dos laços, de modo que você possa prever qual endereço de memória é acessado apenas a partir do índice do laço,
  • evite carregar argumentos struct em funções chamadas dentro de laços OpenMP, e desempacote os membros da struct antes do laço. A vetorização não pode ser realizada sobre estruturas, mas apenas sobre escalares e arrays de float e int. Por exemplo:
     1typedef struct iop_data_t
 2{
 3  float[4] pixel;
 4  float factor;
 5} iop_data_t;
 6
 7float foo(float x, struct iop_data_t *bar)
 8{
 9  return bar->factor * (x + bar->pixel[0] + bar->pixel[1] + bar->pixel[2] + bar->pixel[3]);
10}
11
12void loop(const float *in, float *out, const size_t width, const size_t height, const struct iop_data_t bar)
13{
14  for(size_t k = 0; k < height * width; ++k)
15  {
16    out[k] = foo(in[k], bar);
17    // the non-vectorized function will be called at each iteration (expensive)
18  }
19}
    deveria ser escrito:
     1typedef struct iop_data_t
 2{
 3  float[4] pixel DT_ALIGNED_PIXEL; // align on 16-bits addresses
 4  float factor;
 5} iop_data_t;
 6
 7#ifdef _OPENMP
 8#pragma declare simd
 9#endif
10/* declare the function vectorizable and inline it to avoid calls from within the loop */
11inline float foo(const float x, const float pixel[4], const float factor)
12{
13  float sum = x;
14
15  /* use a SIMD reduction to vectorize the sum */
16  #ifdef _OPENMP
17  #pragma omp simd aligned(pixel:16) reduction(+:sum)
18  #endif
19  for(size_t k = 0; k < 4; ++k)
20    sum += pixel[k];
21
22  return factor * sum;
23}
24
25void loop(const float *const restrict in,
26          float *const restrict out,
27          const size_t width, const size_t height,
28          const struct iop_data_t bar)
29{
30  /* unpack the struct members */
31  const float *const restrict pixel = bar->pixel;
32  const float factor = bar-> factor;
33
34  #ifdef _OPENMP
35  #pragma omp parallel for simd default(none) \
36  dt_omp_firstprivate(in, out, pixel, factor, width, height) \
37  schedule(simd:static) aligned(in, out:64)
38  #endif
39  for(size_t k = 0; k < height * width; ++k)
40  {
41    out[k] = foo(in[k], pixel, factor);
42  }
43}
  • se você usa laços aninhados (por exemplo, laço sobre a largura e a altura do array), declare os ponteiros de pixel no laço mais interno e use collapse(2) no pragma OpenMP, para que o compilador consiga otimizar o uso de cache/memória e dividir o laço de forma mais uniforme entre as diferentes threads,
  • use indexação plana de arrays sempre que possível (for(size_t k = 0 ; k < ch * width * height ; k += ch)) em vez de laços aninhados de largura/altura/canais,
  • use a palavra-chave restrict nos ponteiros de imagem/pixels para evitar aliasing e evite operações in-place sobre pixels (*out deve ser sempre diferente de *in) para que você não provoque dependências de variáveis entre threads
  • alinhe arrays em blocos de 64 bytes e pixels em blocos de 16 bytes, para que a memória seja contígua e a CPU possa carregar linhas de cache inteiras (e evitar segfaults),
  • escreva funções pequenas e otimize localmente (um laço/função), usando OpenMP e/ou pragmas do compilador,
  • mantenha seu código idiotamente simples, sistemático, e evite aritmética de ponteiros de espertinho, porque isso só levará o compilador a detectar dependências de variáveis e aliasing de ponteiros onde não há nenhum,
  • evite casts de tipo dentro do laço,
  • declare os ponteiros de entrada/saída como *const e as variáveis como const para evitar false-sharing em laços paralelos (usando o pragma OpenMP shared(variable)).

Formatação de código

  • Use espaços em vez de tabulações,
  • A indentação usa 2 espaços,
  • Remova espaços à direita,
  • { e } vão para a sua própria linha,

Diretrizes

  1. Faça coisas que você domina: sim, é bom aprender coisas novas, mas o Ansel não é uma caixa de areia, é um software de produção, e não é o lugar certo para você fazer o seu treinamento.
  2. KISS e seja preguiçoso: o Ansel não tem 50 desenvolvedores em tempo integral a postos, ser minimalista tanto nos recursos quanto no volume de código é razoável e sensato para a gestão atual, mas também para a manutenção futura. (KISS: keep it stupid simple, mantenha idiotamente simples).
  3. Faça como o resto do mundo: claro, se todo mundo está pulando pela janela, você tem o direito de não segui-los, mas a maioria das questões sobre UI/UX de software já foi resolvida em algum lugar e, na maioria dos casos, faz sentido simplesmente reutilizar essas soluções, porque a maioria dos usuários já estará familiarizada com elas.
  4. Programar não é o objetivo: programar é um meio para um fim, o fim é ser capaz de processar grandes volumes de fotos em um curto período de tempo, alcançando o visual desejado em cada foto. As tarefas de programação devem ser consideradas sobrecarga e devem ser mantidas mínimas, e o volume de código é um passivo para qualquer projeto.
Translated from English by : Claude. In case of conflict, inconsistency or error, the English version shall prevail.