Trop de “design” de Darktable a commencé par “ce serait cool si nous pouvions …”. Je vous dirai ce qui est cool : accrocher de belles photos à vous sur vos murs dès que possible. Les arts visuels ne sont pas des arts de la performance (comme la musique ou le théâtre), donc seul le résultat compte. Tout ce qui précède est un surcoût, et vous voulez généralement le garder minimal. Cela ne veut pas dire que le processus ne peut pas être apprécié par lui-même. Cependant, pour profiter du processus, vous devez maîtriser vos outils et les plier à votre volonté, sinon vous ne faites que les combattre et l’ensemble du processus se traduit par de la frustration. Le problème est que le “design” de Darktable met trop d’effort à être différent juste pour l’être.
Dans ce processus d’ajout de “nouvelles choses cool”, Darktable a cassé des raccourcis clavier et beaucoup de comportements basiques de l’interface graphique, remplaçant du code propre par des spaghettis et ajoutant plus de désordre à l’interface graphique sans jamais l’élaguer.
Ansel possède une
démarche de design explicite qui débute obligatoirement avec des problèmes définis rencontrés par des utilisateurs définis. Il s’avère que la quantité de code à écrire est inversement proportionnelle à la quantité de réflexion que vous avez faite sur votre solution, généralement pour repérer le problème racine à partir de ce que les utilisateurs vous disent, et trouver le chemin le plus simple vers la solution (qui n’est souvent même pas une solution logicielle…).
Mais les bugs ne vous attendent pas dans la réflexion, ils n’attendent que dans le code que vous avez écrit. Donc, plus vous réfléchissez, moins vous codez, moins vous créez de fardeau de maintenance pour vous-même dans le futur. Mais bien sûr … vous devez avoir suffisamment de temps pour tout bien réfléchir. Essentiellement, cela signifie adieu aux hacks du samedi après-midi, conduits par des amateurs !
Ne l’étendez pas si vous ne pouvez pas le simplifier d’abord
Beaucoup de hacking de Darktable a été fait en copiant-collant du code, provenant d’autres parties du logiciel, ou même d’autres projets, principalement parce que les contributeurs n’ont pas le temps ni les compétences pour entreprendre de grandes réécritures. Cela déclenche la duplication de code et augmente la longueur des fonctions, ajoutant des branches internes et introduisant des if et switch case imbriqués parfois sur plus de 4 niveaux, rendant la structure et la logique plus difficiles à comprendre et les bugs plus difficiles (et frustrants) à traquer, tout en étant plus susceptible de survenir.
Dans tous les cas, lorsque le code responsable des fonctionnalités existantes ne fait que croître (parfois d’un facteur de 10 sur 4 ans), cela soulève de sérieuses questions quant à la maintenabilité future, dans un contexte où les contributeurs ne restent pas plus de quelques années, et les développeurs ont un temps limité à investir. C’est tout simplement irresponsable, car cela sacrifie la maintenabilité à long terme pour des nouveautés éblouissantes.
Simplifier et généraliser le code, à travers des API propres, avant d’ajouter de nouvelles fonctionnalités est un must et Ansel n’accepte que le code que je comprends personnellement et ai les compétences pour maintenir. KISS.
Logique de codage basique
Les pull requests qui ne correspondent pas aux exigences minimales de qualité du code ne seront pas acceptées. Ces exigences visent à assurer la maintenabilité à long terme et la stabilité en faisant respecter un code clair, lisible et structuré avec une logique simple.
Les procédures doivent être divisées en fonctions unitaires et réutilisables, chaque fois que possible. Les exceptions à cela sont les procédures linéaires spécialisées (sans embranchement) effectuant des tâches trop spécifiques pour être réutilisées ailleurs, mais dans ce cas, utilisez des commentaires pour décomposer les procédures en “chapitres” ou étapes facilement repérables et compréhensibles.
Les fonctions ne doivent accomplir qu’une seule tâche à la fois. Par exemple, le code de l’interface utilisateur graphique (GUI) ne doit pas être mélangé avec le code SQL ou de traitement de pixels. Les accesseurs (getters) et modificateurs (setters) doivent être des fonctions distinctes.
Les fonctions doivent avoir un seul point d’entrée et un seul point de sortie (return). Les seules exceptions acceptées sont un retour anticipé si le tampon mémoire sur lequel la fonction est censée opérer n’est pas initialisé ou si un verrou de mutex est déjà capturé.
Les fonctions doivent avoir des noms et des arguments lisibles et explicites qui annoncent leur finalité. Les programmes sont conçus pour être lus par des humains, si vous codez pour la machine, faites-le en binaire.
Les fonctions ne doivent imbriquer que jusqu’à 2 structures conditionnelles if. Si plus de 2 imbriquements if sont nécessaires, la structure de votre code doit être réévaluée et probablement décomposée en fonctions plus granulaires.
Les if ne doivent tester que des cas uniformes, comme l’état ou la valeur d’une ou peut-être plusieurs variables du même type. Si des cas non uniformes doivent être testés (par exemple, IF le paramètre utilisateur EST valeur ET le tampon d'image EST initialisé ET l'image EST brute ET l'image A un profil de couleur intégré ET le coefficient du profil de couleur[0] N'EST PAS NaN), ils doivent être délégués à une fonction de vérification renvoyant un gbooleanTRUE ou FALSE et nommé de manière appropriée pour que les autres développeurs comprennent l’objectif de la vérification sans ambiguïté lors d’une lecture rapide du code, comme color_matrix_should_apply(). Le code de branchement sera alors if(color_matrix_should_apply()) pix_out = dot_product(pix_in, matrix);
Les commentaires doivent mentionner pourquoi vous avez fait ce que vous avez fait, comme vos suppositions de base, vos raisons et toute référence académique ou documentaire que vous avez utilisée comme base (les DOI et URLs doivent être inclus). Votre code doit dire explicitement ce que vous avez fait. Si vous vous trouvez à devoir expliquer ce que fait votre code dans les commentaires, c’est généralement un signe que votre code est mal structuré, que les variables et fonctions sont mal nommées, etc.
Les solutions rapides qui masquent les problèmes au lieu de les aborder à leur racine ne seront pas acceptées. Si vous êtes intéressé par celles-ci, vous pourriez envisager de contribuer à Darktable en amont à la place. Les seules exceptions seront si les problèmes bloquent (font planter le logiciel) et qu’aucune meilleure solution n’a été trouvée après une certaine somme de temps raisonnable consacrée à la recherche.
Souvenez-vous toujours que le meilleur code est le plus simple. KISS. Pour atteindre cet objectif, il est généralement préférable de coder à partir de zéro plutôt que d’essayer de mélanger et assortir des morceaux de code existant par de lourds copier-coller.
Quelques morceaux de sagesse aléatoires d’internet :
Tout le monde sait que déboguer est deux fois plus difficile que d’écrire un programme au départ. Donc si vous êtes aussi intelligent que possible lorsque vous l’écrivez, comment le déboguerez-vous un jour ?— Brian W. Kernighan
N’importe quel imbécile peut écrire un code qu’un ordinateur peut comprendre. Les bons programmeurs écrivent un code que les humains peuvent comprendre.— Martin Fowler, Refactoring: Améliorer la conception du code existant
Codez toujours comme si le type qui finira par entretenir votre code était un psychopathe violent qui sait où vous vivez.— John Woods
Chaque fois que je dois penser pour comprendre ce que fait le code, je me demande si je peux refactoriser le code pour rendre cette compréhension plus immédiatement apparente.— Martin Fowler, Refactoring: Améliorer la conception du code existant
Le code est mauvais. Il pourrit. Il nécessite un entretien périodique. Il a des bugs qui doivent être trouvés. De nouvelles fonctionnalités signifient que le code ancien doit être adapté. Plus vous avez de code, plus il y a d'endroits où les bugs peuvent se cacher. Plus les validations ou les compilations prennent de temps. Plus il faut de temps à un nouvel employé pour comprendre votre système. Si vous devez refactoriser, il y a plus de choses à déplacer.
Le code est produit par des ingénieurs. Pour faire plus de code, il faut plus d'ingénieurs. Les ingénieurs ont des coûts de communication en n^2, et tout ce code qu'ils ajoutent au système, tout en augmentant ses capacités, augmente aussi tout un ensemble de coûts. Vous devriez faire tout votre possible pour augmenter la productivité des programmeurs individuels en termes du pouvoir expressif du code qu'ils écrivent. Moins de code pour faire la même chose (et peut-être mieux). Moins de programmeurs à embaucher. Moins de coûts de communication organisationnels.
Les bons programmeurs écrivent du bon code. Les grands programmeurs n’écrivent pas de code. Les programmeurs zen suppriment le code.— John Byrd
Logique spécifique de codage en C
Ansel ainsi que Darktable sont écrits en C. Ce langage est destiné aux programmeurs avancés pour écrire des bugs rapides dans les applications système et au niveau du système d’exploitation. Il donne trop de liberté pour faire des choses nuisibles et ne peut pas être débogué avant de faire tourner le programme, ou d’écrire vos propres tests (qui peuvent eux-mêmes être bogués, ou peuvent biaiser le type de bugs qu’ils laissent passer, et de toute façon, personne n’écrit de tests). Pourtant, la plupart des contributeurs ne sont pas formés au C, beaucoup d’entre eux ne sont même pas des programmeurs professionnels (bien que les programmeurs C professionnels ne devraient probablement pas être laissés de n’importe où près des applications destinées à l’utilisateur final), donc C est un langage dangereux pour toute application open source.
C vous permettra d’écrire dans des tampons qui n’ont pas été alloués (provoquant une erreur segfault) et vous permettra de les libérer plus d’une fois, mais ne libérera pas les tampons lorsqu’ils ne sont plus nécessaires (provoquant des fuites de mémoire si vous oubliez de le faire manuellement). Le problème est que, puisque l’allocation/libération de tampon peut être très éloignée (dans la durée de vie du programme comme dans le code source) de l’endroit où vous les utilisez, il est facile de se tromper. C vous permettra aussi de caster n’importe quel pointeur vers n’importe quel type de donnée, ce qui engendre de nombreuses erreurs de programmation et des corruptions de données. Les méthodes natives de manipulation de chaînes ne sont pas sûres (pour des raisons que je n’ai jamais pris la peine de comprendre), donc nous devons utiliser celles de GLib pour prévenir les failles de sécurité.
En gros, C fait de vous votre propre et pire ennemi, et c’est à vous d’observer les règles de sécurité dont la sagesse ne deviendra claire qu’une fois que vous les aurez enfreintes. Un peu comme les bugs dans un programme en C. Considérez que vous écrivez votre code pour être lu par des imbéciles qui n’ont jamais programmé en C auparavant.
Vous devez également garder à l’esprit que le compilateur effectuera la plupart des optimisations pour vous, mais sera très conservateur à leur sujet. La règle générale est que si votre code est facilement compréhensible par un humain (logique simple), il sera correctement compris par le compilateur, qui prendra les mesures d’optimisation appropriées. Dans l’autre sens, les optimisations manuelles dans le code, qui donnent un code cryptique supposé être plus rapide sur les systèmes monocœur, se retournent généralement contre vous et entraînent des programmes plus lents après compilation.
Modèles et structures
Les boucles for sont réservées pour itérer sur des tableaux de taille connue à l’avance, de sorte que le nombre d’étapes de boucle soit connu. En étendant cette logique, elles peuvent également être utilisées pour itérer sur les éléments GList * (qui n’ont pas de propriété de taille car ils sont alloués dynamiquement), bien que cela vérifie si chaque élément (GList *) -> next n’est pas NULL. Les boucles for ne doivent généralement pas utiliser de déclarations break ou return à l’intérieur de leur flux de contrôle, sauf si la boucle recherche un élément spécifique dans le tableau et le renvoie dès qu’il est trouvé. Si votre boucle a une condition d’arrêt, utilisez while.
C n’est pas un langage orienté objet, mais vous pouvez et devez utiliser la logique OO lorsque c’est pertinent en C en utilisant des structures pour stocker des données et des pointeurs sur des méthodes, puis des accesseurs et modificateurs uniformes pour définir et accéder aux données.
à partir de structures telles que while, for, if, ou switch ne doivent pas être imbriquées sur plus de 3 (et de préférence 2) niveaux. Utilisez des fonctions si cela se produit :
Les longues séquences de vérifications doivent être placées dans une fonction retournant gboolean et clairement indiquant ce que nous vérifions, afin que dans les procédures, nous ayons :
Accédez toujours aux données des tampons en utilisant la syntaxe ressemblant à un tableau, depuis leur pointeur de base, au lieu d’utiliser des pointeurs non constants sur lesquels vous effectuez des calculs arithmétiques. Par exemple, faites :
1float*const buffer =malloc(64*sizeof(float));
2for(int i =0; i <64; i++)
3{
4 buffer[i] = ...
5}
Ne faites pas :
1float*buffer =malloc(64*sizeof(float));
2for(int i =0; i <64; i++)
3{
4*buffer++= ...
5}
La dernière version est non seulement moins claire à lire, mais empêchera la parallélisation et les optimisations du compilateur car la valeur du pointeur dépend de l’itération de la boucle et devrait être partagée entre les threads si nécessaire. La première version conduit à une logique d’accès à la mémoire indépendante de l’itération de la boucle et peut être parallélisée en toute sécurité.
L’utilisation des incréments de variable en ligne (voir un exemple cauchemardesque ici ) est strictement interdite, sauf si c’est la seule opération de la ligne. C’est un désordre qui occasionne de nombreuses erreurs de programmation. Cela est permis :
1uint32_t counter;
2for(int i =0; i <64; i++)
3{
4if(array[i] > threshold)
5 counter++;
6}
Les instructions case dans la structure switch ne doivent pas être additives. Ne faites pas :
Lors d’une lecture rapide, il ne sera pas immédiatement clair que le case pour VALUE3 hérite des clauses définies par les cas précédents, surtout dans les situations où il y a plus de cas. Faites :
Chaque cas est auto-enfermé et le résultat ne dépend pas de l’ordre de déclaration des cas.
Triez et stockez vos variables dans des structures que vous passez comme arguments de fonction au lieu d’utiliser des fonctions avec plus de 8 arguments. Ne faites pas :
Le premier exemple est tiré de darktable . Le copier-coller des appels de fonction est inutile et la multiplication des arguments positionnels rend impossible de se souvenir de qui est quoi. Cela ne montre pas non plus quels arguments sont constants sur les différentes branches, ce qui rendra le refactor difficile. Le second exemple n’est pas plus concis, mais la structure non seulement facilite l’appel de la fonction, mais la déclaration de la structure permet de définir explicitement chaque argument, avec des vérifications en ligne au besoin. La dépendance des arguments d’entrée aux conditions externes devient également immédiatement claire, et les arguments booléens sont directement définis à partir des conditions, ce qui rendra le programme plus facile à étendre à l’avenir et moins sujet aux erreurs de programmation dues à des incompréhensions dans la dépendance des variables.
Optimisations OpenMP
Les pixels sont essentiellement des vecteurs 4D RGBA. Depuis 2004, les processeurs ont des capacités spéciales pour traiter les vecteurs et appliquer des instructions simples sur des données multiples (SIMD). Cela nous permet d’accélérer les calculs en traitant un pixel entier (SSE2) jusqu’à 4 pixels (AVX-512) en même temps, économisant beaucoup de cycles CPU.
Les compilateurs modernes ont des options d’auto-vectorisation qui peuvent optimiser le C pur, et la bibliothèque OpenMP permet de fournir des indices pour améliorer cela, à condition que le code soit écrit de manière vectorisable et utilise quelques pragmas.
évitez les branches dans les boucles qui changent le flux de contrôle. Utilisez des instructions en ligne comme absolute = (x > 0) ? x : -x; de sorte qu’elles puissent être converties en masques d’octets dans SIMD,
les pixels ne doivent être référencés qu’à partir du pointeur de base de leur tableau et des indices des boucles, de sorte que vous puissiez prédire quelle adresse mémoire est accédée uniquement à partir de l’indice de la boucle,
évitez de transporter des arguments struct dans des fonctions appelées dans des boucles OpenMP, et décomprimez les membres du struct avant la boucle. La vectorisation ne peut pas être effectuée sur des structures, mais seulement sur float et int scalaires et tableaux. Par exemple :
1typedef struct iop_data_t
2{
3float[4] pixel;
4float facteur;
5} iop_data_t;
6 7float foo(float x, struct iop_data_t *bar)
8{
9return bar->factor * (x + bar->pixel[0] + bar->pixel[1] + bar->pixel[2] + bar->pixel[3]);
10}
1112void loop(constfloat*in, float*out, const size_t width, const size_t height, const struct iop_data_t bar)
13{
14for(size_t k =0; k < height * width; ++k)
15 {
16 out[k] = foo(in[k], bar);
17// la fonction non vectorisée sera appelée à chaque itération (coûteux)
18 }
19}
devrait être écrit :
1typedef struct iop_data_t
2{
3float[4] pixel DT_ALIGNED_PIXEL; // aligner sur des adresses de 16 bits
4float facteur;
5} iop_data_t;
6 7#ifdef _OPENMP 8#pragma declare simd 9#endif10/* déclarer la fonction vectorisable et l'intégrer pour éviter les appels depuis la boucle */11inline float foo(constfloat x, constfloat pixel[4], constfloat facteur)
12{
13float somme = x;
1415/* utilisez une réduction SIMD pour vectoriser la somme */16#ifdef _OPENMP17#pragma omp simd aligned(pixel:16) reduction(+:somme)18#endif19for(size_t k =0; k <4; ++k)
20 somme += pixel[k];
2122return facteur * somme;
23}
2425void boucle(constfloat*const restrict in,
26float*const restreint out,
27const size_t largeur, const size_t hauteur,
28const struct iop_data_t bar)
29{
30/* décompressez les membres de la structure */31constfloat*const restrict pixel = bar->pixel;
32constfloat facteur = bar-> facteur;
3334#ifdef _OPENMP35#pragma omp parallel for simd default(none) \36 dt_omp_firstprivate(in, out, pixel, factor, largeur, hauteur) \
37 schedule(simd:static) aligned(in, out:64)
38#endif39for(size_t k =0; k < hauteur * largeur; ++k)
40 {
41 out[k] = foo(in[k], pixel, factor);
42 }
43}
si vous utilisez des boucles imbriquées (par exemple, boucle sur la largeur et la hauteur du tableau), déclarez les pointeurs de pixels dans la boucle la plus interne et utilisez collapse(2) dans le pragma OpenMP pour que le compilateur puisse optimisr l’utilisation du cache/mémoire et diviser la boucle plus uniformément entre les différents threads,
utilisez un indexage plat des tableaux dès que possible (for(size_t k = 0 ; k < ch * largeur * hauteur ; k += ch)) au lieu de boucles imbriquées width/height/channels,
utilisez le mot-clé restrict sur les pointeurs d’image/pixels pour éviter le masquage et éviter les opérations en place sur les pixels (*out doit toujours être différent de *in) pour ne pas déclencher de dépendances variables entre les threads
alignez les tableaux sur 64 octets et les pixels sur des blocs de 16 octets afin que la mémoire soit contiguë et que le CPU puisse charger des lignes de cache complètes (et éviter les segfaults),
écrivez de petites fonctions et optimisez localement (une boucle/fonction), en utilisant OpenMP et/ou les pragmas du compilateur,
gardez votre code stupide simple, systématique et évitez les arithmétiques de pointeurs intelligentes car cela ne fera que conduire le compilateur à détecter des dépendances variables et des masquages de pointeurs là où il n’y en a pas,
évitez les casts de types dans les boucles,
déclarez les pointeurs d’entrée/sortie comme *const et les variables comme const pour éviter les faux-partages dans les boucles parallèles (utilisation du pragma shared(variable) OpenMP).
Formatage de code
Utilisez des espaces au lieu de tabulations,
L’indentation utilise 2 espaces,
Supprimez les espaces finaux,
{ et } vont sur leur propre ligne,
Lignes directrices
Faites ce que vous maîtrisez : oui, c’est bien d’apprendre de nouvelles choses, mais Ansel n’est pas un bac à sable, c’est un logiciel de production, et c’est pas le bon endroit pour obtenir votre formation.
KISS et soyez paresseux : Ansel n’a pas 50 développeurs à plein temps sur le pont, être minimaliste à la fois en fonctionnalités et en volume de code est raisonnable et sain pour la gestion actuelle, mais aussi pour la maintenance future. (KISS: keep it stupid simple).
Faites comme le reste du monde : bien sûr, si tout le monde saute par la fenêtre, vous avez le droit de ne pas les suivre, mais la plupart des problèmes concernant l’UI/UX des logiciels ont déjà été résolus quelque part et dans la plupart des cas, il a du sens de simplement réutiliser ces solutions, car la plupart des utilisateurs y seront déjà familiers.
Programmer n’est pas le but : programmer est un moyen pour atteindre un objectif, l’objectif est de pouvoir traiter un grand volume de photos en peu de temps tout en atteignant le look désiré sur chaque image. Les taches de programmation sont à considérer comme des frais généraux et doivent être réduites au minimum, et le volume de code est un passif pour tout projet.
Translated from English by :
Aurélien Pierre, ChatGPT.
In case of conflict, inconsistency or error, the English version shall prevail.