jImage

Any improvements on this ticket ?

Dans sa version la plus simple (simple déplacement des méthodes dans une class jImage), voici jImage. Tout fonctionne en static.

Cela permet de générer l'image en cache autrement que pendant l'affichage. Du coup si l'image est grande, on peut appeler jImage à l'upload et du coup à l'affichage se sera instantané.

Ca va comme ça ou je découpe plus, en fait un objet à instancier ? Qu'en dites vous ?

Sympa, mais :

• je pense que dans l'entête de la classe, il faut rajouter le copyright des auteurs du plugins (tu n'a pas écris la plus grosse partie du code ;-)
• faire un commentaire phpdoc pour la méthode get
• peut être avoir des méthodes genre resize()
• la méthode get ne devrait pas générer la balise img, mais renvoyer simplement $att. C'est au plugin image de générer le html. Car après tout on peut vouloir utiliser jImage pour autre chose que du html
• lors de la génération du html, je pense qu'il faudrait faire un htmlspecialchars sur les valeurs
• Peut être inclure dans le tableau renvoyé un item contenant le chemin physique du fichier ? (et faire attention de ne pas l'inclure lors de la génération de la balise html)

Oubli pour la méthode resize(), ça va être trop chiant à tout casser.

* je pense que dans l'entête de la classe, il faut rajouter le copyright des auteurs du plugins (tu n'a pas écris la plus grosse partie du code ;-)

J'attendais ce genre de remarques avant de tout mettre clean.

* faire un commentaire phpdoc pour la méthode get

done.

* peut être avoir des méthodes genre resize()

Si on passe par un mécanisme comme celui-là, il faut définir des méthodes pour tout. Est-ce plus pratique que l'array de paramètres ?

* la méthode get ne devrait pas générer la balise img, mais renvoyer simplement $att. C'est au plugin image de générer le html. Car après tout on peut vouloir utiliser jImage pour autre chose que du html

En effet.

* lors de la génération du html, je pense qu'il faudrait faire un htmlspecialchars sur les valeurs

vendu.

* Peut être inclure dans le tableau renvoyé un item contenant le chemin physique du fichier ? (et faire attention de ne pas l'inclure lors de la génération de la balise html)

done ;)

* le htmlspecialchars n'est pas fait * mettre les noms de méthode selon le coding style : image_inCache -> createCache(), image_shadow -> createShadow(). * generateCachedImage -> generate() * à image_inCache et image_shadow, renommer le paramètre $array en un truc plus significatif ($params ou autre)

Alors justement, je reprend la remarque de laurentj, j'aurais plutôt créer une série de méthode :

$img = new jImage( $src, $orsrc );

resize() createCache() addShadow() zoom() align()

ect.

et

echo '<img'; 
foreach( $img->att as $key => $val ) 
if( !empty($val) ) 
echo ' '.$key.'="'.$val.'"'; 
echo '/>';

Il faut encore que je traduise et complète les commentaires, que je supprime les tab, que je vérifie si il n'y a pas de bug, ect.

J'ai hâte d'avoir votre avis :).

Pour bast, il te suffit de faire :

$cacheurl = jCacheImage::inCache( $src, $params );

D'une manière générale :

• Il y a de gros problèmes de langues dans ce patch, avec plein de mot français mélangés à de l'anglais, que ce soit dans les commentaires ou dans les noms de méthodes ou propriétés
• les commentaires des propriétés ne sont pas vraiment compréhensibles et/ou explicites. Difficile par exemple de différencier $CACHEPATH et $cache_path.
• Il faudrait que les noms des propriétés soient plus explicites. Faire la différence entre $root, $path, $www, $cache_path, $CACHEPATH etc, est difficile
• les noms des propriétés ne suivent pas le coding style, notament celles qui sont en majuscules ou qui ont des _
• Il y a encore des espaces en fin de lignes en trop, des problèmes d'indentations. Configure ton éditeur pour qu'il montre les espaces et tab
• Tout est public, alors que je n'ai pas l'impression que tout doit l'être, notament au niveau des propriétés.
• Dans bon nombre de propriétés "interne", on passe des tableaux de données. Je pense que pour certaines, il faudrait avoir des arguments classiques, comme omotiResize(), resize(), align(), shadow(),

jImage : Pour les noms des propriétés et méthodes (avec le commentaire entre parenthese) :

• origine_src -> sourcePathFilename (path + filename of the original image file. The path should be relative to your www/ directory)
• src -> targetPathFilename (path + filename of the new image file. The path should be relative to your www/ directory)
• ext -> sourceExt (extension of the original image file)
• extFinal -> targetExt (extension of the new image file)
• root -> à supprimer, inutile : il faut juste génèrer des chemin absolu dans le nom de domaine
• path -> targetFullFilePath (full path of the new image file)
• www -> targetFullUrl (full url of the new image file)
• omotiResize() -> proportionalResize()
• sauveIn() -> saveIn()
• finalise() -> finalize()

jCacheImage:

• $CACHEPATH -> $cachePath
• cache_path -> fullCacheFilePath
• cache_www -> fullCacheUrl
• cache_name -> cacheFileName

Est ce que ces trois dernières propriétés sont-elles vraiment utiles ? Je pense que ça fait doublon avec path, www et src de la classe parente, non ?

dans la methode cache, il y a un $this->racine qui traine au lieu de $this->root (qui doit de toute façon être supprimé et remplacé par un chemin absolu sans nom de domaine)

À part ça, je pense qu'il est possible de simplifier cette réorganisation de code, mais je le ferais, ça me prendra moins de temps que de tout expliquer :-) On commitera le patch en l'état une fois effectuées les modifications demandées sur les propriétés et méthodes.

Ah tiens, je n'avais pas vu ce ticket auparavant, et j'ai justement travaillé là-dessus pendant mes congés, quand j'avais un peu le temps.

Pour l'instant, toutes les fonctionnalités ne sont pas présentes (zoom, ombre), mais ça peut se rajouter facilement.

A ce sujet, il faut voir pour l'ombre, car par rapport au plugin original, je pense :

• qu'il faut vérifier le type d'image pour savoir quel type d'ombre faire : sur une source jpeg par exemple (non transparente forcement), il n'y a pas lieu de parcourir l'ensemble des pixels pour faire l'ombre.
• que l'ombre ne doit pas s'ajouter au dimensions de l'image, car ça pourrait provoquer des problèmes dans un design "au pixel".

Bref, pour l'instant, cette version supporte :

• width et/ou height, avec conservation des proportions par défaut, désactivable via paramètre
• maxwidth et/ou maxheight, avec conservation des proportions par défaut, désactivable via paramètre (nb : si l'image est plus petite, elle n'est pas agrandie)
• background pour la couleur de fond

Ok, ça ne casse pas des briques, mais j'ai surtout travaillé sur le coté "html" :

• l'attribut alt est présent de façon systématique, avec une chaîne vide. Si le paramètre alt est fourni, les caractères spéciaux sont convertis
• l'attribut title, si présent, voit ses caractères spéciaux convertis
• les attributes width et height sont présents dans tous les cas, même s'il n'y a pas GD. Comme c'est valide, je trouve que c'est plus adapté que d'utiliser un CSS inline. Ils sont également contraints par maxwidth et maxheight si nécessaire.
• j'ai supprimé les attributs on* (JS) car je n'aime pas mettre du JS avec le HTML. Je vais peut-être regarder pour affecter directement via JS les événements sur les images, mais je pense plutôt que le développeur devrait donner un ID à son image et attacher des événements.
• j'ai nommé le plugin jimage plutôt que image, et les fichiers caches vont dans www/_caches/jimages (j'ai préfixé par "_" car ça donne peut-être mieux l'idée d'un répertoire "système" et non "utilisateur")

Pour le coté "traitement GD", je voudrais ajouter :

• l'ombre portée
• le zoom
• l'incrustation d'une bordure graphique : un dossier contient les éléments de la bordure (cotés, coins, ...) en PNG alpha, et la bordure est appliquée dynamiquement sur l'image puis mis en cache.

J'envoie le diff ainsi qu'un template qui permet de voir différents essais.

Je dois continuer dans cette voie, ou bien je jette le truc ?

sample template for jImage test

ouais non mais là si tous le monde si met on va jamais y arriver ...

arf ;) perso j'ai codé ça car j'en avais besoin pour un site que j'ai mis en prod et comme il manquait le alt systématique pour la validité, ... puis de fil en aiguille j'ai écris un autre truc.

j'ai lu rapidement le dernier patch, et quelques questions :

• si GD n'est pas activé, le plugin HTML ne semble pas redimensionner les images selon les mêmes possibilités qu'avec GD activé (ne fournir qu'une des dimensions, l'autre étant calculée proportionnellement ou non suivant le paramètre d'homothétie, en tenant en plus compte de maxwidth/maxheight), ou j'ai mal lu ?

• GD activé ou pas, les attributs HTML width et height ne vont-ils pas faire merder le truc si je spécifie en paramètres : array('width'=>300, 'maxheight'=>100) sur une image de 400*200 ? Je pense que l'image sera générée correctement avec GD en tenant compte du maxheight (200*100), par contre l'attribut HTML width restera sur 300, ce qui déformera l'image. De plus, l'attribut HTML height ne sera pas présent.

• dans l'URL générée pour la balise HTML, pourquoi spécifier src="http(s)://mondomaine.com/basepath/mon_image.jpg" au lieu de juste "/basepath/mon_image.jpg" ?

• je pense tjrs que les attributs on* ne doivent pas s'appliquer directement, car il vaudrait mieux utiliser du JS non-intrusif comme c'est le cas avec jForms

dsl pour ma réaction, je suis un peu sur les nerf.

Par rapport à tes critiques.

Si gd n'est pas activé, rien n'est fait, c'est un oublie de ma part, le dernier patch le fessé mais ... En lissant ton commentaire je me rends compte que je suis passé à côté de quelque chose, tu coups je vais faire comme tu dis.

height="xx" width="xx" ou style="height:xx; width:xx;"

Partant du principe que ça concerne la forme et pas le fond j'ai mis ça sous forme de style (dans l'avant dernier patch).

Pour l'url, c'est un oublie, un autre :)

Et pour le JS, même si je suis parfaitement d'accord avec toi, ces attributs sont valides et je ne vois pas de raison valable d'empêcher d'autres développeur de les utiliser.

---

Sinon je reprends ton précédent poste a tête reposée.

qu'il faut vérifier le type d'image pour savoir quel type d'ombre faire : sur une source jpeg par exemple (non transparente forcement), il n'y a pas lieu de parcourir l'ensemble des pixels pour faire l'ombre.

La je ne vois pas, explique mieux.

que l'ombre ne doit pas s'ajouter au dimensions de l'image, car ça pourrait provoquer des problèmes dans un design "au pixel".

Je suis d'accord, ça pose quelque problème avec l'ancienne implémentation, mais maintenant on peut le faire, par contre cela modifie le comportement du plugin, donc ce ne sera pas avant la version 1.1

(nb : si l'image est plus petite, elle n'est pas agrandie)

Pourquoi pas, mais pas par défaut.

jimage

ouaip puisque c'est intégré par défaut.

"_cache"

Voir avec laurentj

l'incrustation d'une bordure graphique : un dossier contient les éléments de la bordure (côté, coins, ...) en PNG alpha et la bordure est appliquée dynamiquement sur l'image puis mis en cache.

Bonne idée ! Je pense également à un système de calque pour _par exemple_ ajouter un logo en transparence. Et un background image.

Replying to Lipki:

dsl pour ma réaction, je suis un peu sur les nerf.

pas de problème ;)

Par rapport à tes critiques.

plutôt des remarques, interrogations ;)

Si gd n'est pas activé, rien n'est fait, c'est un oublie de ma part, le dernier patch le fessé mais ... En lissant ton commentaire je me rends compte que je suis passé à côté de quelque chose, tu coups je vais faire comme tu dis.

oui en fait, pour coder mon truc, je me suis assuré que les transformations de bases (dimensions) marcheraient si GD est désactivé ou bien si les transformations GD échouent pour X raisons (d'où mon try-catch qui attrape les exceptions mais ne fait rien).

Me suis aussi assuré que le code HTML était valide (d'où le alt systématique)

height="xx" width="xx" ou style="height:xx; width:xx;" Partant du principe que ça concerne la forme et pas le fond j'ai mis ça sous forme de style (dans l'avant dernier patch).

oui j'avais vu ça dans le plugin original, mais c'est vrai que comme width et height sont des attribs valides, j'ai préféré les utiliser. En plus c'est plus facile que de fusionner avec l'attribut style, surtout si celui-ci existe déjà.

Pour l'url, c'est un oublie, un autre :)

ok

Et pour le JS, même si je suis parfaitement d'accord avec toi, ces attributs sont valides et je ne vois pas de raison valable d'empêcher d'autres développeur de les utiliser.

je ne veux pas les empêcher, mais je voudrais faire leur assignation via javascript, en attachant des événements via un script, et non dans des attributs HTML.

Je ferai une version avec ça demain sans doute, histoire que vous puissiez voir le truc

Sinon je reprends ton précédent poste a tête reposée. qu'il faut vérifier le type d'image pour savoir quel type d'ombre faire : sur une source jpeg par exemple (non transparente forcement), il n'y a pas lieu de parcourir l'ensemble des pixels pour faire l'ombre. La je ne vois pas, explique mieux.

En fait, en vacances, j'ai terminé de mettre en prod un site client, et j'ai voulu appliquer une ombre portée sur des images JPEG. Sur mon hyper vieux portable p3-600 192Mo de ram, ça mettait des plombes pour appliquer l'ombre portée, alors que ça ne devrait pas. En lisant le code que tu avais mis en place rapidement, j'ai cru voir que tu regardes pixel par pixel s'il faut créer une ombre pour ce point, ce qui est bien dans le cas d'un PNG transparent. Dans le cas d'une JPEG, il suffit de tracer l'ombre de façon rectiligne sur les cotés de l'image (suivant l'angle) car le fond n'est pas transparent. Du coup, pas besoin de parcourir chaque pixel. Ou alors je me vautre complètement sur ton algo d'ombre, ce qui est fort possible, j'étais en vacances ;)

que l'ombre ne doit pas s'ajouter au dimensions de l'image, car ça pourrait provoquer des problèmes dans un design "au pixel". Je suis d'accord, ça pose quelque problème avec l'ancienne implémentation, mais maintenant on peut le faire, par contre cela modifie le comportement du plugin, donc ce ne sera pas avant la version 1.1

oui, de toute façon je ne voyais mon plugin que pour la 1.1

(nb : si l'image est plus petite, elle n'est pas agrandie) Pourquoi pas, mais pas par défaut.

par contre là je ne suis pas d'accord : les paramètres maxwidth/maxheight ne doivent JAMAIS agrandir l'image, seulement la réduire si nécessaire.

jimage ouaip puisque c'est intégré par défaut. "_cache" Voir avec laurentj

ce sont des suggestions, vite modifiées si nécessaire.

l'incrustation d'une bordure graphique : un dossier contient les éléments de la bordure (côté, coins, ...) en PNG alpha et la bordure est appliquée dynamiquement sur l'image puis mis en cache. Bonne idée !

oui, ça peut être très pratique, pour par exemple mettre un cadre avec bords arrondis et ombre portée. Et puis comme ça les graphistes peuvent se faire plaisir ;)

Je pense également à un système de calque pour _par exemple_ ajouter un logo en transparence. Et un background image.

Ces 2 choses tu peux déjà le faire avec ma classe, ou presque : la méthode pasteImageFromFile de l'objet jImage permet d'importer n'importe quel fichier image dans l'image gd courante. Pour l'instant on ne peut que spécifier les largeur et la hauteur, mais je veux rajouter le x, le y de destination ainsi que le recadrage sur la source avant importation, pour juste coller une partie.

Je voudrais aussi mettre une méthode pasteImage qui importe la ressource GD d'une jImage dans une autre jImage.

Je veux aussi mettre la méthode fillWithImage qui fonctionnera comme fillWithColor, mais avec un fichier image répété en mosaïque.

bref, je vais sans doute mettre en place cela demain, puis on verra ce qu'on retient, ce qu'on jette, ce qu'on fusionne, etc...

hop, encore une petite remarque : je n'aime pas trop l'idée d'avoir des notions d'attributs HTML dans une classe jCacheImage, et la méthode inHtml() de celle-ci. A la limite, pourquoi pas une classe jImageHTML qui surchargerait la classe jImage, et qui serait utilisée pour le plugin HTML jimage. Car j'aimerai bien me servir de jImage et ses possibilités de cache (éventuellement dans un dossier de mon choix) pour préparer des images que je mets ensuite dans un PDF, donc rien à voir avec les attributs HTML. Il y a encore des choses à dissocier à ce niveau.

ok pour l'ombre, en fait j'ai eu le processus de pensé inverse. J'ai d'abord fait une ombre carré, puis je me suis dit qu'il fallait que ça marche aussi pour le png 24 et je suis rester la dessus.

Il est possible que les noms de paramètres soit mal choisit (ce serait pas la première fois) mais pour moi le but de maxwidth/maxheight et de pouvoir placer les images dans une galerie, sous une forme semblable a un tableau. Donc les images ne doive pas être plus grande que le "carré" spécifier, ni plus petite. Rien n'empêche par contre de rajouter un paramètre "noenlarge" par exemple.

Replying to Lipki:

ok pour l'ombre, en fait j'ai eu le processus de pensé inverse. J'ai d'abord fait une ombre carré, puis je me suis dit qu'il fallait que ça marche aussi pour le png 24 et je suis rester la dessus.

ok

Il est possible... Donc les images ne doive pas être plus grande que le "carré" spécifier, ni plus petite.

voir ticket:628 qui a été intégré et qui dit que l'image n'est pas agrandie avec maxwidth/maxheight.

sinon, bien compris ton envie de "même taille pour toutes les images", mais le problème est que ça ne marchera que avec GD. De plus les images seraient éventuellement générées avec du "vide" autour, et si on exporte en JPG, alors pas de transparence... bref, je suis pas sûr du résultat. Il vaut sans doute mieux imaginer une histoire à base de style CSS, avec des marges calculées automatiquement. Je pense que c'est pas si compliqué à envisager et solutionne les problèmes évoqués ci-dessus.

Bon, soirée avec des amis, si je suis pas trop fatigué en rentrant je lirai tes éventuelles réponses. En tout cas j'aime bien échanger avec toi, ça me donne plein d'idées sur des points où je me posais bcp de questions et là ça va bcp mieux.