← Back to team overview

phatch-dev team mailing list archive

Re: [Merge] lp:~jochym/phatch/Actions into lp:phatch

 

> I have implemented it. It works but I do not like my implementation.
> Unfortunately convert('P') does not handle transparency properly or I dont
> know how to use it.
Sorry, I forgot to say that we've solved most PIL issues in phatch/lib/imtools.py Please use as much code from that as possible. So to convert the mode use:
imtools.convert(image, 'L')
and for transparent images:
imtools.convert(image, 'LA')
http://bazaar.launchpad.net/~stani/phatch/trunk/annotate/head%3A/phatch/lib/imtools.py#L580

So never use PILs image.convert method directly. We have a whole test suite with all kinds of images and image.convert is not bullet proof. After Pycon we will fire your actions in the test suite to see if any issue arises.

If the imtools.convert function would not behave properly, than we will fix it. It is important to have all the convert fixes centrally in one function.

> For now I've pushed it to the save function - so the action returns RGB/RGBA
> and the save makes the conversion. It works but there is no control over
> palette type 
Our rule is that if an action needs another color mode, that we don't convert it back. The amount of conversions should be limited to the minimum. So you did the right thing by pushing the conversion to save action. Note that the user in this case could add an extra convert action, although I think we don't support yet there palette suboptions. Probably we might in the future.

> and there is ugly warning about conversion in the log. 
The warning is not meant to be ugly, but informative. It mostly should warn the user. Maybe based on this warning he decides to save it as png instead of gif. We chose to inform the user always when a conversion is needed to save in a certain file type.

> On the
> other hand the action changes the palette completely so one should convert it
> properly in the script anyway.
Let's see how imtools.convert handles this.  The use of this action for gif P images is probably a corner case, but the most important is that transparency doesn't get lost.

> I have improved the speed slightly also.
> 
> > Please make the label 'Warm Up' instead of 'WarmUp'. Use the following tags
> > for now:
> > - warm up: filter
> > - grid: size, filter
> Done

Great! Could you please use other icons. If you are lazy I prefer you copy the icons from these actions:
- grid: tile/mirror
- warm up: brightness

If you have a bit more time, look for a nice GPL icon. We prefer shiny icons to make them coherent. We only use SVG icons, as you can find in the source tarball. With Inkscape you export to a png of 48x48. You can use img2py to convert the png to py code.

Examples of icons can be found in the source:
images/source/igor
images/source/openclipart

Keep up the good work!
-- 
https://code.edge.launchpad.net/~jochym/phatch/Actions/+merge/19100
Your team Phatch Developers is subscribed to branch lp:phatch.



Follow ups

References