Permettre d'utiliser d'autres Image que GDImage dans les filtres image

• refactor de la signature de _image_valeurs_trans() pour avoir un tableau d'option en 5eme argument
• refactor des fonctions d'ecriture de l'image en fin de filtre
  • _image_gd_output() devient _image_object_output() avec support de l'ancien nommage, et permet de gérer l'enregistrement de tout type d'objet Image via une fonction _image_xxx_write() a implémenter pour chaque classe xxx utilisée pour manipuler des images dans les filtres images.

Ce refactor permet l'utilisation de la librairie Intervention dans le plugin images par exemple images!4741 ou d'utiliser directement Imagick dans un filtre image comme dans le filtre image_imagick qui était traité de manière dérogatoire

added 1 commit

• ef10d839 - fix: un peu plus de log, et eviter une erreur si $img est une ressource et pas une class

Compare with previous version

mentioned in issue images#4717

added 1 commit

• 241a61b0 - fix: il faut que _image_imagepng fasse passer la qualite pour produire un png...

Compare with previous version

added 1 commit

• 601229f5 - fix: il faut que _image_imagepng fasse passer la qualite pour produire un png...

Compare with previous version

Même commentaire que dans l'autre MR, on pat du principe que avif et webp sont disponibles de facto avec toutes les libs, ce qui n'est pas forcément le cas.

added 3 commits

• 1bf1b114 - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 2339fccd - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• 1b074099 - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...

Compare with previous version

added 1 commit

• 14735109 - doc: fix la doc qui dit le contraire de ce que fait le filtre

Compare with previous version

marked this merge request as ready

added 2 commits

• 692a97e4 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 977dd71c - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable

Compare with previous version

added 28 commits

• 977dd71c...9cc97e43 - 10 commits from branch 5.x
• 9cc97e43...90c85ee7 - 8 earlier commits
• 69b60e00 - fix: utiliser un log debug ici, il y a pas d'erreur
• d8287937 - refactor: l'enregristrement des images sur disque sait gérer n'importe quel...
• 25d2a4ce - fix: un peu plus de log, et eviter une erreur si $img est une ressource et pas une class
• 227fcac6 - fix: il faut que _image_imagepng fasse passer la qualite pour produire un png...
• 36078c83 - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 7c90a06b - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• 9b67a59c - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• dd48d4fb - doc: fix la doc qui dit le contraire de ce que fait le filtre
• ee6a78c1 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 3e74622e - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable

Compare with previous version

added 14 commits

• 3e74622e...c8cd9df4 - 4 earlier commits
• 9910e90c - refactor: l'enregristrement des images sur disque sait gérer n'importe quel...
• 51a5f89a - fix: un peu plus de log, et eviter une erreur si $img est une ressource et pas une class
• 3446d94b - fix: il faut que _image_imagepng fasse passer la qualite pour produire un png...
• 98196368 - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• e1ebae6e - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• 1d881653 - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• de5cd308 - doc: fix la doc qui dit le contraire de ce que fait le filtre
• 999b0390 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 8e0d94e0 - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable
• 69599476 - styles: coding standard

Compare with previous version

@cpol oui il faut que je fixe ça, mais actuellement c'est compensé par un autre bug qui est qu'on ne prend en compte que ce que GD detecte, meme si on utilise imagick :p

added 2 commits

• e432e7b7 - fix: tests plus complets pour le cas gd2 + appeler...
• ac32f936 - refactor: _image_extensions_acceptees_en_entree() et...

Compare with previous version

added 12 commits

• ac32f936...e87d8811 - 2 earlier commits
• cec05898 - fix: il faut que _image_imagepng fasse passer la qualite pour produire un png...
• 8611f61f - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 5c5b3650 - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• bc5bb5db - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• d4e00734 - doc: fix la doc qui dit le contraire de ce que fait le filtre
• 5e9cf1a6 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 6c76cb92 - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable
• c2d31f2c - styles: coding standard
• 5fce23f5 - fix: tests plus complets pour le cas gd2 + appeler...
• 964b91d7 - refactor: _image_extensions_acceptees_en_entree() et...

Compare with previous version

added 1 commit

• f2cad78c - fix: simplifier l'eciture de l'image en sortie de process_image_reduire(). Le...

Compare with previous version

added 1 commit

• 8007bd82 - fix: simplifier l'eciture de l'image en sortie de process_image_reduire(). Le...

Compare with previous version

Donc voilà j'ai également traité la detection des formats image lisibles/exportables en fonction de la librairie graphique choisie, action/tester déléguant aux filtres_image_process_xxxx_dist() qu'il appelle avec un argument true pour indiquer qu'il faut faire les tests de format.

A noter également un bugfix sur |image_reduire sur une image avec exif qu'il faudra sans doute reporter en 4.x !28 (8007bd82) (en gros, image_reduire renvoie parfois l'image d'origine qu'il a simplement copié, et dans ce cas on a de nouveau des exifs à la sortie, le plus propre pour s'en sortir dans tous les cas étant de faire un taille_image avant d'écrire la balise image finale)

Il reste la question du répertoire où l'on stocke les images générées par les filtres images. Actuellement c'est cache-gd2 ce qui porte plus très bien son nom si on fabrique les images avec imagick.

• je ne pense pas qu'avoir un répertoire cache par processeur (un cache-gd2 ET un cache-imagick) soit une bonne idée, car si tu switch tu dois donc regénérer toutes les images de ton site et tu mets le serveur à genoux. L'idée étant que le changement de librairie produit le même résultat, ou presque, on a donc un seul dossier cache.
• on peut renommer cache-gd2 en cache-images et dans ce cas il faudrait gérer ça dans la fonction _image_valeurs_trans() à la première génération d'une image, quand on voit que cache-images n'existe pas encore mais qu'il y a un cache-gd2
• MAIS si on fait ça, on perd tout d'un coup tous les liens vers des images cache-gd2 (caches navigateurs, images référencées par g**gle, caches CDN....)
  • ça peut se compenser avec une RewriteRule qu'on ajoute dans le htaccess par défaut, mais ça sera pas ajouté automatiquement lors de l'upgrade vers SPIP 5 (il faudrait alors le mentionner dans les notes de release)
• alternativement, on ne renomme pas le dossier, mais on copie les images de cache-gd2 dans cache-images au fur et à mesure (ce qui évite de repartir d'un cache image vide) et on pose dans cache-gd2 un fichier texte qui dit "ce dossier n'est plus utilisé par SPIP depuis le xx/xx/xxxx, vous pourrez le supprimer après un délai raisonnable une fois que votre site aura été réindexé etc..." (formulation à améliorer)
• enfin, plus brutalement, on repart de zéro : on laisse le cache-gd2 obsolète avec un fichier texte comme ci-dessus, et on génère les nouvelles images dans un cache-image/. C'est alors l'occasion de revoir le nommage des images générées pour y conserver autant que possible le nom du fichier d'origine, ce qui est une demande ancienne pour le référencement des images (on ajouterait juste un hash au nom d'origine et/ou un rangement par dossier hashé)

La question reste à trancher, vu qu'on est sur un changement de version majeur je pencherai pour la dernière option, mais ça aura un coût perf sur les gros site lors de la mise à jour, toutes les images cache devant alors être regénérées...

Dans tous les cas on ne traite pas ça dans cette PR, on le traitera séparemment, ensuite, quand/si cette PR est intégrée, ça permet de tester cette PR sans rien casser de son dossier cache et de revenir sur master après

Le ticket que j'ai retrouvé sur le nom des images images#4706 (mais je suis à peu près certain qu'il y a eu un autre ticket bien plus ancien)

Je sais pas comment ça calcule le cache attendu, mais est-ce que c'est lourd de :

• calculer le chemin legacy attendu pour cache-gd2/truc_attendu, regarder si le fichier existe et si pas obsolète continuer de l'utiliser tel quel
• si fichier n'existe pas ou trop vieux, calculer le chemin attendu pour cache-images (qui peut être totalement différent donc) et regarder si le fichier existe et pas obsolète et l'utiliser
• si ce deuxième fichier n'existe pas ou trop vieux, seulement enfin créer la nouvelle image transformée et la stocker dans le chemin moderne cache-images/truc_attendu_moderne

Si c'est possible, il me semble que ça permettrait ta dernière option mais sans casser les URL dans la nature (nav, google etc) qui continueraient d'être valables tant que l'image transformée n'est pas obsolète (changement de params etc). Mais j'oublie peut-être des cas…

continuer à utiliser le vieux cache-gd2/xxx/xxx/ est compliqué et embetant car ça va continuer à stocker des urls de ce type dans le cache SPIP et à référencer des images dans ce dossier. Par contre on peut en effet calculer l'ancien nom, et si le fichier existe dans cache-gd2 on le copie dans cache-images avec le nouveau nom et du coup on repart pas de zero pour les cache images (et après un certain temps il sera possible de supprimer cache-gd2 qui restera totalement inchangé après passage à SPIP 5)

le changement de librairie produit le même résultat, ou presque, on a donc un seul dossier cache

AMHA, c'est une mauvaise idée de garder les anciennes images. Si tu changes de lib, c'est probablement parce que tu as une bonne raison pour le faire, et probablement que tu veux gagner en qualité sur un point précis. Et la raison appartient au webmestre, on ne peut pas vraiment la discuter.

Du coup, je trouve que ta 1ère idée d'avoir un répertoire cache par processeur est très bien, c'est clair, c'est lisible et ça fonctionne comme attendu. La dernière option de repartir de 0 n'est pas mauvaise non plus, à condition qu'on ne serve pas un mix de imagick /gd2 / whatever.

Pour les perfs, normalement on ne change pas de lib graphique tous les jours, je pense que c'est un coût acceptable d'avoir un ralentissement temporairement. Et c'est quelque chose sur lequel on peut prévenir, histoire que le webmestre soit au courant des conséquences.

non @cpol je ne suis pas d'accord : si tu es un•e utilisateur•ice avancé qui veut changer de lib pour avoir des rendus plus précis/respectant les profils couleurs etc. tu vides le répertoire cache images et tu repars de zéro. Mais pour l'utilisateur standard il clique juste sur le choix d'une lib qui marche, et en particulier quand tu utilises GD2 il y a le soucis historique récurrent de page blanche quand l'image est trop grosse, et si l'utilisateur•ice switch il faut pas que ça mette l'hébergement à genoux parce que tout d'un coup on repart de zéro sur le cache image.

Par ailleurs l'utilisateur•ice avancé•e verra peut-être une diff entre les images générées par une lib ou par une autre, mais pour l'utilisateur•ice moyen, c'est tout du pareil au même

Raaah je m'aperçois que ce message est resté en draft et pas parti. Bon, je le remets avec plusieurs jours de retard, désolé...

Hmmm je trouve que ça reste du "filoutage". D'un point de vue utilisateur lambda, je pense qu'il s'attend à ce que s'il change d'option, tout son site prenne l'option en compte sans garder de reliquat de l'ancienne option (peu importe l'option modifiée). Mais là, comme c'est pour des images on sort du fonctionnement "implicite" (notez bien les guillemets ;) ) pour faire une règle qui mélange du vieux avec du neuf. Je n'ai pas d'exemple en tête d'un autre service / site / CMS qui fonctionnerait de la sorte.

Pour la petite histoire, quand j'ai commencé à mettre le nez dans imagick c'était sur une histoire de pertes de couleurs remontés par des utilisateurs "moyens" qui se plaignaient de "photos moche dans spip" (en fait c'était gd2 le coupable). Je suis d'accord que c'est peut être pas le use case majoritaire, mais des utilisateurs moyens qui voient une diff il y en a.

Je ne suis pas au courant du "soucis historique récurrent de page blanche", mais j'ai une préférence pour les choses qui ne se font pas "dans le dos" de l'utilisateur. Partant du principe que l'on ne peut pas forcément prévoir tous les cas et toutes les tailles de site, est ce qu'il serait possible de documenter la subtilité (si on part sur pas de regénération complète) quelque part à minima? Doc? Infobulle? "Attention, les vignettes déjà générées ne seront recalculées qu'au prochain vidage de cache" (exemple).

Pour répondre sur ce point, c'est déjà ce qui se passe avec |image_reduire : quand tu changes de lib GD2/Imagick/Convert ça ne recalcule pas les images, on garde les images déjà calculées.

Et on a jamais eu de plaintes en 25 ans :p

Donc je pense que c'est pas choquant, mais ça mérite sans doute un peu plus de documentation pour les utilisateurs avancés comme toi qui veulent basculer pour des raisons de qualité de rendu et pas juste "parceque c'est ce qui marche"

Hello, désolé pour mon absence de réponse, la grippe est passée par chez moi... Je te concède le point de "aucune plainte en 25 ans" !

mentioned in merge request images!4741

approved this merge request

added 54 commits

• 8007bd82...1d89b960 - 31 commits from branch 5.x
• 1d89b960...cdff9787 - 13 earlier commits
• 5ba2905f - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 76dcfb47 - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• ba166385 - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• 17fb324b - doc: fix la doc qui dit le contraire de ce que fait le filtre
• 4bfbe72b - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 6cc545a7 - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable
• 8d9c599a - styles: coding standard
• 1dd586f5 - fix: tests plus complets pour le cas gd2 + appeler...
• ebc0c8b2 - refactor: _image_extensions_acceptees_en_entree() et...
• 03958e71 - fix: simplifier l'eciture de l'image en sortie de process_image_reduire(). Le...

Compare with previous version

added 1 commit

• fbc2d0e3 - fix: simplifier l'ecriture de l'image en sortie de process_image_reduire().

Compare with previous version

added 24 commits

• fbc2d0e3...e44e516b - 8 commits from branch 5.x
• e44e516b...7d0c50e4 - 6 earlier commits
• 746fa9c6 - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 1af61ce4 - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• 5b0d9a33 - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• 05c6b292 - doc: fix la doc qui dit le contraire de ce que fait le filtre
• dc1cd2a8 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 54696188 - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable
• 3dc70c90 - styles: coding standard
• 42ded173 - fix: tests plus complets pour le cas gd2 + appeler...
• 2801fc6e - refactor: _image_extensions_acceptees_en_entree() et...
• 0cba8acb - fix: simplifier l'ecriture de l'image en sortie de process_image_reduire().

Compare with previous version

@cpol ca donne quoi ta grippe ? tu penses pouvoir valider ou pas ?

je ne pense pas avoir les droits ni pour resolve les threads, ni pour approuver. Mais ok pour moi oui!

je ne pensais pas à la validation formelle, mais plutot à la validation effective : est-ce que tu pense que c'est bon ?

Pour moi le seul point gênant c'est ce que je racontais en haut du fil: !28 (comment 201323)

Après c'est peut être anecdotique, mais comme j'ai rencontré le cas je le signale.

hum oui ok @cerdic tu vois comment on pourrait gérer ce cas?

ben oui est c'est traité ici pour imagick images!4741 (diffs) où l'on teste si on arrive à trairer les webp et avif en lecture et en ecriture. T'avais même demandé pourquoi on testait que ces 2 formats :p

Ah, très bien. Ça a dû être rajouté après mon 1er commentaire, ou alors je l'ai loupé. Juste pour la forme, je pense que Imagick::queryFormats aurait pu être plus concis mais en l'état je n'ai pas d'objection à merger.

added 37 commits

• 0cba8acb...036c7609 - 21 commits from branch 5.x
• 036c7609...8ce7e51e - 6 earlier commits
• 8976756d - fix: ne pas forcer de valeur par defaut pour l'argument $qualite dans les...
• 12297a45 - fix: tester le cas is_string pour $options car certains filtres appellent avec...
• b78627fe - feat: une fonction svg_filter_colorize() pour modifier les composantes RGB...
• fee74d6f - doc: fix la doc qui dit le contraire de ce que fait le filtre
• e084e420 - refactor: une fonction exif_lire() plus generique qu'on utilise ensuite dans...
• 62419223 - fix: un rayon de flou plus proche de ce qu'on obtient en gd/imagick avec une image pixel semblable
• 997c185e - styles: coding standard
• 894e8bb1 - fix: tests plus complets pour le cas gd2 + appeler...
• 279f8252 - refactor: _image_extensions_acceptees_en_entree() et...
• bb7fcc89 - fix: simplifier l'ecriture de l'image en sortie de process_image_reduire().

Compare with previous version