developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 12 years ago

#604 closed enhancement (fixed)

jImage

Reported by: bastnic Owned by:
Priority: normal Milestone: jelix 1.1
Component: jelix:utils Version: trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Plutôt que d'avoir un plugin de template qui gère tout ce qui est image, on devrait avoir une classe faisant tous les traitements qui serait appelée par le plugin.

Cela permettrait par exemple de créer le cache des photos à leur upload. Utile si on charge une page contenant 100 photos de 5mo chacune. (PHP renverra un out of memory bien avant ça..)

Attachments (12)

jimage.diff (26.2 KB) - added by bastnic 12 years ago.
jimage.2.diff (27.9 KB) - added by bastnic 12 years ago.
jImage_pres.diff (32.3 KB) - added by Lipki 12 years ago.
jImage_final.diff (37.7 KB) - added by Lipki 12 years ago.
jImage_final.2.diff (37.7 KB) - added by Lipki 12 years ago.
604-jImage.diff (12.4 KB) - added by Julien 12 years ago.
image.tpl (6.8 KB) - added by Julien 12 years ago.
sample template for jImage test
jImage_final.3.diff (39.2 KB) - added by Lipki 12 years ago.
jimage.3.diff (39.1 KB) - added by Lipki 12 years ago.
jimage.4.diff (39.1 KB) - added by Lipki 12 years ago.
trunk-jImage-#604.patch (28.6 KB) - added by bibo 12 years ago.
604-jImage-2009-10-02.diff (16.6 KB) - added by Julien 12 years ago.

Download all attachments as: .zip

Change History (68)

comment:1 Changed 12 years ago by laurentj

  • Milestone Jelix 1.1 beta 1 deleted

comment:2 Changed 12 years ago by bastnic

  • Owner set to lipki

comment:3 Changed 12 years ago by bballizlife

Any improvements on this ticket ?

comment:4 Changed 12 years ago by bastnic

  • Owner changed from lipki to bastnic
  • Status changed from new to assigned

Changed 12 years ago by bastnic

comment:5 Changed 12 years ago by bastnic

  • Milestone set to Jelix 1.1 beta 1
  • review set to review?

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 ?

comment:6 Changed 12 years ago by laurentj

  • review changed from review? to review-

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)

comment:7 Changed 12 years ago by laurentj

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

comment:8 Changed 12 years ago by bastnic

  • review changed from review- to review?
  • 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 ;)

Changed 12 years ago by bastnic

comment:9 Changed 12 years ago by laurentj

  • review changed from review? to review-
  • 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)

comment:10 Changed 12 years ago by Lipki

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 '/>';

Changed 12 years ago by Lipki

comment:11 Changed 12 years ago by Lipki

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.

Changed 12 years ago by Lipki

comment:12 Changed 12 years ago by Lipki

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

Pour bast, il te suffit de faire :

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

comment:13 Changed 12 years ago by Lipki

  • review changed from review- to review?

Changed 12 years ago by Lipki

comment:14 Changed 12 years ago by laurentj

  • review changed from review? to review-

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.

comment:15 Changed 12 years ago by Julien

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 ?

Changed 12 years ago by Julien

Changed 12 years ago by Julien

sample template for jImage test

Changed 12 years ago by Lipki

comment:16 Changed 12 years ago by Lipki

  • review changed from review- to review?

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

comment:17 Changed 12 years ago by Julien

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

comment:18 follow-up: Changed 12 years ago by Lipki

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.

comment:19 in reply to: ↑ 18 Changed 12 years ago by Julien

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.

comment:20 follow-up: Changed 12 years ago by 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.

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.

comment:21 in reply to: ↑ 20 Changed 12 years ago by Julien

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.

comment:22 Changed 12 years ago by bibo

Julien, Lipki, avez-vous fait d'autres avancées sur ce ticket ?

comment:23 Changed 12 years ago by Julien

Hello,

perso j'ai du mettre en standby cet été, là je reprends tout doucement les contribs à Jelix en parallèle des projets.

Projets pour lesquels je vais avoir besoin de faire des transformations d'image hors du plugin de template, donc je vais sans doute rebosser sur ce ticket d'ici 10jours max.

comment:24 Changed 12 years ago by laurentj

  • Milestone changed from Jelix 1.1 beta 1 to Jelix 1.1 beta 2

comment:25 Changed 12 years ago by bibo

Ok, Julien. Je suis dans le même contexte que toi. Je vais partir de vos derniers patchs (lipki et toi) et poster ici mes avancées.

comment:26 Changed 12 years ago by Lipki

Il est un peut partie en sucette ce plugin.

Il faudrait a mon avis (attention :) ) bloquer les fonctionnalités, et finir de déboguer cette version.

Bon bien sur vous avez surement des envie spécifiques, la gestion du texte, ou des modifications plus complexe, mais si on continue a ajouter des morceaux au puzzle sans faire un plan, ça va être un vrai bordel.

Pour ma part j'ai commencer a penser a un plugin image2, et a mon avis c'est la dedans, qu'il faut mettre toutes les nouvelles idées.

En attendant, bibo si tu veut check le dernier patch que j'ai posté, apparement il ya pas mal de franglais, et peut-être quelque bug.

comment:27 Changed 12 years ago by Lipki

Je me trouve un peut agressif... bon je vais mettre un smiley :)

comment:28 Changed 12 years ago by laurentj

Il est un peut partie en sucette ce plugin.

ouai, je me demande même si ca devrait pas être un projet à part dans la forge... :-)

Changed 12 years ago by Lipki

Changed 12 years ago by Lipki

comment:29 Changed 12 years ago by Lipki

oh laurentj, tu lit dans mes pensées.

bon tu va surement recevoir un mail :)

comment:30 Changed 12 years ago by bastnic

Qu'est ce qu'on fait donc ?

Où en sont Julien, Bibo et Lipki ?

comment:31 Changed 12 years ago by bastnic

  • Owner bastnic deleted
  • Status changed from assigned to new

comment:32 Changed 12 years ago by bibo

  • Summary changed from jImage to n

Point sur ce ticket:

  • ce ticket avait pour but initial de sortir les fonctionnalites du plugin html image pour en faire une classe utilitaire. Le plugin conservant à sa charge le formatage html.
  • après le patch de bast transférant le code du plugin dans une classe jImage, lipki et julien ont proposé une restructuration et une réécriture du code plus importante.
  • De mon côté, j'ai pris le patch de bast j'ai effectué les modifications demandées par laurentj et aussi nettoyé le code (renommage, simplification algo).

Proposition:

  • intégrer dans la 1.1 un patch simple celui de bast/le mien
  • faire chauffer a feux doux les améliorations fonctionnelles apportées par les patch de lipki et julien (autre ticket ou projet forge)

Changed 12 years ago by bibo

comment:33 Changed 12 years ago by bibo

  • Summary changed from n to jImage

oops...

comment:34 follow-up: Changed 12 years ago by Julien

Hello,

je suis 100% ok sur ce principe, en effet, il faudrait vraiment en faire un projet à part, avec des specs et fonctionnalités bien définies.

Pour le patch intégré en 1.1, je note cependant (désolé, pas eu le temps de vérifier le code) :

  • que le code HTML généré doit être valide, donc la balise "alt" doit être présente tout le temps, même si le dev ne le spécifie pas, et les attributs doivent avoir un htmlspecialchars lorsque nécessaire.
  • que les transformations de taille fonctionnent sans GD, et notamment les attributs width et height devraient être présents tout le temps.
  • qu'il faut, dans le cadre de l'utilisation de GD et donc d'un fichier image cache, intégrer la notion de filemtime() de la source dans la génération du non de fichier cache, au cas où l'image source est remplacée sans toucher aux paramètres d'affichage : j'écrase toto.jpg avec toto2.jpg, le cache ne sera pas regénéré)
  • vérifier que maxwidth/maxheight n'agrandissent jamais l'image

de mon côté, j'utilise en ce moment http://developer.jelix.org/attachment/ticket/604/604-jImage.diff en prod (avec en plus la prise en compte de filemtime() pour le cache), je voudrais donc que le code HTML reste valide dans la nouvelle version (c'est le plus important pour moi, après le code sous-jacent y a plein de possibilités)

comment:35 in reply to: ↑ 34 Changed 12 years ago by bibo

  • que le code HTML généré doit être valide, donc la balise "alt" doit être présente tout le temps, même si le dev ne le spécifie pas

hmm, ok mais alors qu'y met on par défaut ? le débat est très fourni autour de ce sujet pour la spec HTML5. Il n'est même pas sur que le alt sera toujours requis pour <img/> http://lists.w3.org/Archives/Public/public-html/2008Aug/0759.html

et les attributs doivent avoir un htmlspecialchars lorsque nécessaire.

Done

  • que les transformations de taille fonctionnent sans GD, et notamment les attributs width et height devraient être présents tout le temps.
  • qu'il faut, dans le cadre de l'utilisation de GD et donc d'un fichier image cache, intégrer la notion de filemtime() de la source dans la génération du non de fichier cache, au cas où l'image source est remplacée sans toucher aux paramètres d'affichage : j'écrase toto.jpg avec toto2.jpg, le cache ne sera pas regénéré)

Ok. Je piquerais certainement dans ton patch ces bouts de code si tu permets.

  • vérifier que maxwidth/maxheight n'agrandissent jamais l'image

Done

comment:36 follow-up: Changed 12 years ago by Julien

vivi pas de prob pour reprendre des bouts de mon patch ;)

pour le alt, il faudrait simplement que le tag html ressemble toujours au moins à ça :

<img src="toto.jpg" width="200" height="300" alt="" /> 

donc avec un alt vide ( ce qui équivaut normalement à une image de "décoration" et pas une image "de contenu" qui devrait normalement toujours avoir un alt spécifié pour l'accessibilité ).

Pas encore suivi les discuss sur HTML5, mais étant donné que today Jelix fourni de l'XHTML par défault, alt est obligatoire.

Si en HTML5 alt n'est plus obligatoire, il ne sera en aucun cas interdit, donc l'avoir vide sera tjrs Ok je pense.

PS : avec mon exemple de tag ci-dessus, je fais une autre remarque : l'attribut "src" de l'image doit être spécifié sans le nom de domaine, et utiliser le basePath pour être absolu.

PS2 : décidement, les idées s'enchainent : je crois que dans le code originel (et repris dans la patch de bast), les images à manipuler sont ouvertes via HTTP, ce qui est vraiment à éviter (perfs, etc...), surtout dans une classe jImage qui serait totalement indépendante du contexte HTTP :

$origine_www = 'http'.(empty($_SERVER['HTTPS'])?'':'s').'://'.$_SERVER['HTTP_HOST'].$gJConfig->urlengine['basePath'].$src;
....
$image = imagecreatefromjpeg($origine_www);

en plus, dans une config PHP un peu + secure, les ouvertures de fichiers distants peuvent être désactivés par défaut (quoique je crois qu'ils ont séparé l'accès aux fichiers distants et les includes de fichiers distants, je suis plus sûr depuis quand).

Bref, y a quand même pas mal de choses à changer dans l'existant.

C'est pour ça que j'étais parti sur un tout nouveau truc, en reprenant des idées bien entendu. Aujourd'hui, mon patch supporte moins de fonctionnalités, mais ne rentre pas en conflit avec l'existant.

L'idée au final, serait peut-être de modifier l'existant comme on le peut. Mais surtout la question c'est : est-ce que le plugin image actuel a besoin d'être revu ? certes, il y aurait pas mal de corrections à effectuer etc... mais a-t-on besoin de créer la classe jImage pour ça ?

Si on créé jImage maintenant juste pour séparer le code, j'ai peur que cela constitue un frein pour le futur développement.

A mon avis, le dév de jImage va devoir partir de 0 (ou de mon mon patch (j'ai une version plus récente ou d'un autre); avec des specs et fonctionnalités de base sur lesquelles il faut s'entrendre ; puis y ajouter les fonctionnalités par la suite; ainsi on entre pas en conflit avec le plugin image actuel et on ne risque pas de casser quoique ce soit.

Bref, l'idée serait de juste corriger le plugin image dans les quelques défauts qu'il a, puis définir les fonctionnalités/specs de jImage.

Je vais refaire un patch avec ce que j'utilise en prod.

comment:37 Changed 12 years ago by bastnic

Salut les gens,

Comme dit précédemment, vous faites ce que vous voulez. Mon seul besoin est de générer le cache AVANT. C'est tout. Donc il faut une classe. Peu importe le mode de fonctionne derrière.

comment:38 in reply to: ↑ 36 Changed 12 years ago by bibo

donc avec un alt vide ( ce qui équivaut normalement à une image de "décoration" et pas une image "de contenu" qui devrait normalement toujours avoir un alt spécifié pour l'accessibilité

pas de problème à supporter alt="". Les discussions du whatwg vont dans ce sens pour l'instant.

PS : avec mon exemple de tag ci-dessus, je fais une autre remarque : l'attribut "src" de l'image doit être spécifié sans le nom de domaine, et utiliser le basePath pour être absolu.

Bien sur. C'est deja le cas il me semble mais je vérifierais.

$origine_www = 'http'.(empty($_SERVER['HTTPS'])?'':'s').'://'.$_SERVER['HTTP_HOST'].$gJConfig->urlengine['basePath'].$src;
....
$image = imagecreatefromjpeg($origine_www);

bonne remarque !

Si on créé jImage maintenant juste pour séparer le code, j'ai peur que cela constitue un frein pour le futur développement.

je ne vois pas vraiment pourquoi... pour l'instant le plugin image garderait exactement la même interface donc aucun soucis de compatibilité. Ensuite pour les futures versions de Jelix rien n'empeche de modifier/enrichir la classe jImage sachant que sa signature sera vraiment tres simple au démarrage.

Mon seul besoin est de générer le cache AVANT. C'est tout. Donc il faut une classe.

C'est pris en compte bien sur.

comment:39 Changed 12 years ago by Julien

Yop,

je viens de jeter un oeil au dernier patch 29/09, mais j'avoue que ça ne me convient pas au niveau de la logique de la classe jImage :

  • on ne doit pas tenir compte d'attributs HTML, car je peux trèsbien vouloir utiliser jImage pour travailler des photos que je veux mettre dans du PDF.
  • comme je l'ai signalé, ouvrir les images via http n'est pas pertinent je pense, et peut poser des problèmes si la config de php est un peu stricte
  • toute notion d'URL web n'a pas de sens ici
  • la classe ne doit en aucun cas imposer le nom et l'emplacement où on enregistre le fichier.
  • la gestion de l'ombre est à revoir en fonction qu'on soit sur une image transparente ou pas, car si on veut ajouter une ombre sur une jpg, alors pas nécessaire de réaliser l'ombre pour chaque pixel, mais uniquement à la périphérie des bordures.

au niveau du plugin html :

  • les transformations de base, genre les dimensions, doivent fonctionner sans GD
  • width et height devraient être présents tout le temps, même si non spécifié par le développeur, afin de gagner en perf d'affichage.

Je joins la dernière version de mon approche, que j'utilise en prod sur certains sites.

Il y a à ce stade moins de fonctionnalités, c'est certain, mais ce sera assez facile d'en greffer de nouvelles.

Le fonctionnement basique de jImage :

  • une image jImage a une taille et peut être enregistrée dans un fichier arbitraire.
  • cette image est composées de calques, des objets jImageLayer, ayant chacun un "id"
  • on peut ajouter/supprimer des calques
  • sur chaque calque, on peut utiliser les fonctions GD classiques sur la ressource GD
  • lors de l'enregistrement de l'image, les calques sont "empilés" les uns sur les autres et l'image finale est sauvegardée puis détruite de la mémoire. Les calques sont toujours là, on peut toujours travailler dessus individuellement.

Je vais ajouter :

  • la possibilité de récupérer les données brutes de l'image sans l'enregistrer
  • la possibilité de réordonner les calques
  • un wrapper aux fonctions GD, du type : $layer->gdMethod($name,$params);

le plugin de template HTML jimage :

  • génère du xhtml valide
  • l'url de l'image commence à la racine du site web, sans la notion d'hôte qui est inutile.
  • les transformations de dimensions marchent sans GD

C'est donc dans jImageLayer que l'on ajouterai les effets de type ombre portée, etc... et puis à terme ce sera "photoshop is now powered by jelix" ;)

Bref, est-ce que je continue à coder dans l'optique de ce ticket, ou bien j'en fais un truc à part ? Il est clair que mon approche de jImage est assez incompatible avec l'existant.

Changed 12 years ago by Julien

comment:40 Changed 12 years ago by laurentj

  • review review? deleted

photoshop is now powered by jelix

LOL

J'aime bien cette histoire de layer, ça va permettre je pense pas mal de possibilité.

Je suis d'accord que le code html devrait être généré uniquement par l'appelant à jImage. Par contre, je trouve dommage que jImage ne gère pas directement un cache, ça pourrait être utile pour d'autres (ne serait-ce que par exemple, un plugin de template jimage pour xul).

comment:41 Changed 12 years ago by laurentj

ne serait-ce que par exemple, un plugin de template jimage pour xul

désolé, pas terminé ma phrase : ça éviterai d'avoir trop de code à dupliquer.

comment:42 Changed 12 years ago by Julien

Peut-être une classe du type jImageUtils qui servirait à faire les calculs de taille, générer un nom de cache, voir si ce fichier de cache existe, ... ?

Ces méthodes seraient appelées à partir des plugins de template (html, xul, ...)

De mon point de vue, le cache ne peut-être élaboré qu'à partir d'un fichier source et d'une liste de params qui permettent une mise en cache. Or dans mon approche, on n'a pas de paramètres avec jImage, on a que des actions unitaires, avec les calques, etc... ce qui laisse justement une grosse liberté.

Les params, et donc la mise en cache, sont fournis aux plugins de template, ou alors transmis à la future jImageUtils

J'ai oublié de préciser : mon patch peut être testé avec le template "image.tpl" qui est dans les fichers attachés à ce ticket

comment:43 Changed 12 years ago by laurentj

je ne comprend pas pourquoi tu n'intègre pas les opérations de changement de taille et de gestion de cache dans jImage...

pour moi, jImage devrait représenter une image. Donc on devrait pouvoir manipuler cet objet dans tous les sens : redimensionnement, transformations, application de layers et cie. jImage possède nécessairement une image de départ (ne serait-ce qu'une image vide), avec un nom de fichier, garde une trace de toutes les manipulations effectuées (pour calculer un numéro de série, un md5 par ex), et quand on lui demande d'appliquer toutes ces manipulations, c'est à dire avoir le résultat final, récupère soit le cache correspondant au numéro de série, soit génère ce cache si il n'existe pas.

Pour l'implémentation, je verrais bien un truc comme ça : chaque manipulation de l'image donnerai lieu à l'instanciation d'un objet que l'on ajoute dans une liste de manipulation (au niveau d'un layer je pense). Un objet de ce type aurait des methodes

  • getHash qui renverrait un hash en fonction de ses propriétés (une bête serialisation d'un tableau contenant son type, et différentes valeurs ?)
  • render($imgid), qui effectue la transformation proprement dite

(on pourrait appeler cette interface jIImageEffect, ou jIImageOperator ou un truc dans le genre)

Au moment où on demande à jImage de nous donner l'image résultat, jImage appelle d'abord toutes les méthodes getHash pour récupérer les hash, et avec ces hash + nom de l'image -> il en deduits un md5 pour avoir un nom de fichier cache. Si le cache doit être généré, il appelle alors toutes les méthodes render() pour génèrer l'image.

Cerise sur le gateau : ces objets de transformations pourraient être des plugins, ce qui permettrait d'étendre facilement les possibilités de jImage.

Vala, c'etait juste des idées en l'air, faites ce que vous en voulez :-)

comment:44 Changed 12 years ago by Lipki

Je voulais vous dire que je vous lit :) Mais je ne réagis pas, la tournure de ce ticket a fini de me dépassé, maintenant il me dégoute.

Enfin j'imagine que avec autant de monde dessus ce plugin/classe/utilitaire/photoshop va devenir génial ... si vous vous mettez d'accord.

comment:45 Changed 12 years ago by laurentj

maintenant il me dégoute.

tu devrais plutôt être content :-/ Ton petit plugin de rien du tout est peut être en train de venir un super truc :-p Bref, tu serais à l'origine de quelques choses de sympa..

comment:46 Changed 12 years ago by Lipki

Et je suis pour. c'était même l'idée, créer jImage une class utilitaire de manipulation d'image, utilisé par le plugin entre autre.

Mais ce ticket n'était que la première étape, le déplacement du plugin vers une classe, pour commencer.

Il devrait être bouclé depuis des lustres pour pouvoir repartir sur une bonne base.

En plus des idées il y en a, et des talents aussi, vous savez tous ce que vous voulez, si chaque personne arrêter une bonne fois pour toutes de réécrire le patch de zéro a chaque fois, on pourrais progresser, vous avez vu la liste de patch la haut, pas moins de quatre auteur différent( et alternativement ).

comment:47 Changed 12 years ago by Julien

Lipki, je suis du même avis que Laurent, tu as initié un truc qui risque de devenir très puissant.

Laurent, j'ai bien lu ton explication, effectivement c'est faisable, sans doute pas très simple niveau implémentation, mais pour l'instant je fais les calculs des tailles dans le plugin et pas dans jImage car je veux que le plugin fonctionne pour les tailles même si GD n'existe pas (et comme GD est utilisé dans jImage), d'où jImageUtils qui servirait à faire ce genre de choses récurrentes.

Le cache dans jImage, j'avoue ne pas très bien savoir comment le faire, ou plutôt répondre à ces problématiques :

  • on ne stock pas forcement les fichiers caches au même endroit. Ça peut être dans le rép www si on veut les afficher en html, par contre si c'est des images destinées à être intégrées dans du PDF, j'ai peut-être pas envie de les mettre dans le www.
  • les appels "directs" à jImage sont là pour créer/modifier des images selon un code explicite et précis, si ce code est exécuté, c'est que l'on veut effectuer les opération et générer une nouvelle image finale (par exemple à l'upload d'une image produit sur un shop). Pas besoin de cache, car cette opération n'est effectuée que lorsque nécessaire, alors que dans un plugin de template c'est super utile pour justement ne pas faire les transfos à chaque affichage.

Bref, de mon point de vue, jImage n'est pas là pour faire des transformations "à la volée" (le terme n'est pas vraiment bon, mais j'en ai pas d'autre), mais pour créer/modifier des images et les enregistrer UNE fois. Si le résultat image final ne change pas, alors on ne doit pas utiliser jImage, mais implémenter une gestion de cache dans la couche appelante.

Je vais faire un essai d'implémentation de jImageUtils, tu verras, le code du plugin en sera largement allégé.

comment:48 Changed 12 years ago by bballizlife

Tout d'abord, il est vrai que je n'interviens pas dans jImage, regardant ça un peu de loin (mais j'ai hâte de jouer avec quand même).

Cependant je me dois de donner mon avis. jImage n'a *plus* à être un ticket sur le bugtracker de Jelix. Cela va trop loin. Trop de patchs, de fonctionnalités, de retours, d'idées... C'est génial mais ça devient ingérable.

Je ne suis pas là pour faire le méchant de service mais je pense que cette fois il faut vraiment dire stop à l'illustre ticket 604 et le fermer. En contre partie je vous propose d'ouvrir un projet sur la forge car c'est là bas que tout doit continuer. jImage n'est pas une feature de Jelix, c'est devenu un véritable projet à part entière.

Avec un projet sur la forge, vous pourrez discuter plus calmement et distinctement : vous aurez un wiki dédié et surtout un bugtracker rien que pour vous. Votre organisation n'en sera que renforcée, les égos apaisés et tout le monde pourra tirer dans le même sens. Et comme il y a du talent et de l'envie chez vous, vous pourrez facilement exploiter vos idées car vous aurez votre dépôt svn rien qu'à vous ;)

Donc laurent, si tu pouvais créer un projet jImage sur la forge et donner accès à ces fou-furieux qui sont en train de nous faire un photoshop-lite, ça serait extra ! :)

comment:49 Changed 12 years ago by Julien

Lipki, je comprends ta position vis à vis de la profusion de patchs sur ce ticket.

Comme je l'avais expliqué, le changement initial de code que j'ai proposé a été fait pendant mes congés, sans connexion, bref, j'ai découvert ce ticket à mon retour.

Pourquoi je n'ai pas utilisé ton plugin ? A cause des éléments déjà évoqués (code html, marche sans GD, ...) et surtout j'avais besoin de pouvoir faire ces manips hors d'un template.

Donc j'ai posté mon code sur ce ticket comme ça répondait (de loin) à la problématique.

J'avais fait attention lors de la création de ce code de ne pas modifier l'existant, car je savais que tu travaillais dessus, en étant l'initiateur ; et comme je n'avais pas de connexion, je ne pouvais pas suivre les éventuelles avancées.

je propose donc pour ma part de quitter ce ticket, je vais renommer :

tpl plugin : jimage2 classes : jImage2, jImage2Layer, jImage2Utils

et continuer à bosser là-dessus sans interférer dans le ticket actuel.

NB : c'est pas du tout de la "vexation" de ma part, mais comme l'approche que je propose diffère trop de l'existant tout en étant moins complet niveau fonctionnalités, ça perturbera moins le truc ;)

comment:50 Changed 12 years ago by Lipki

Y a moyen d'avoir les emails de tous les contributeur actifs sur ce ticket ( pour ce qui est de la vexation, i am the best ) J'envoie sur le champ une demande officiel pour un projet sur la forge.

comment:51 Changed 12 years ago by Julien

bballizlife : tout à fait d'accord

s'il y a nécessité d'avoir une jImage dans jelix, qui découple simplement le code originel de Lipki, alors il doit être fait dans ce ticket (ce que je laisse faire à ceux qui sont plus dans cette approche).

Pour le projet de la forge, je vais même plus loin : la manipulation d'image doit faire partie d'un module Jelix complet, nommé peut-être mediaManager, avec tout ce qu'il faudrait pour gérer des fichiers et en particulier des images, avec un éditeur wysiwyg d'images, etc...

comment:52 Changed 12 years ago by laurentj

Ok, bon, je crois que je vais intégrer le deuxième patch de bastnic, avec les corrections que j'avais évoqué (ou bastnic, prépare une nouvelle version du patch). Je pense qu'on va appeler la classe jSimpleImage, pour éviter un conflit si on intègre un jour un "super" jImage.

Ça permettra de satisfaire le besoin à l'origine du ticket.

Pour le super "jImage", pas de souci pour créer un projet dans la forge. Donnez moi juste un nom qui résume bien ce que vous voulez faire (par mail ou sur le forum).

Pour les discussions techniques sur le gros projet de jImage : ouvrez une discussion dans le forum dédié sur jelix.org.

Ce serait bien que la suite de la discussion ici se recentre sur le patch et la demande originale :-)

comment:53 Changed 12 years ago by Lipki

Laurentj je t'ai envoyer un mail pour la création de jMediaManager.

Tu ne ma pas répondu, est-ce que tu la reçu ?

comment:54 Changed 12 years ago by laurentj

  • Milestone changed from Jelix 1.1 beta 2 to jelix 1.1

Milestone Jelix 1.1 beta 2 deleted

comment:55 Changed 12 years ago by Lipki

http://forge.jelix.org/projects/mediamanager

voici le projet MédiaManager?, lâcher vos com dans le wiki.

comment:56 Changed 12 years ago by laurentj

  • Resolution set to fixed
  • Status changed from new to closed

Je viens d'intégrer le patch de bastnic et bibo. svn 1151.

J'ai juste changé le nom de la classe en jImageModifier.

Note: See TracTickets for help on using tickets.