algoo / preview-generator

generates previews of files with cache management
https://pypi.org/project/preview-generator/
MIT License
226 stars 50 forks source link

Wand is used instead of preview_generator for png (and probably most image format) #235

Closed inkhey closed 3 years ago

inkhey commented 3 years ago

Wand builder in 0.17+ version is used by default instead of the Imagemagick builder to generate png. This cause issue as the wand builder is not properly designed and don't produce good quality png image (black background, low quality image).

The issue was introduced here: https://github.com/algoo/preview-generator/commit/7e192da2cee83228fb3ed233b1a156d97103619e

The problem is related to the import line, which change the load ordering and make wand by used by default for png

from preview_generator.preview.builder.image__imconvert import ImagePreviewBuilderIMConvert

We should now deprecate wand builder and make it not registered by default, but for later, it may be very useful to have a way to force a strict order of builder, to avoid this kind of issue. There are other couple of builder which support same format like svg file (supported by both cairosvg and inkscape).

lebouquetin commented 3 years ago

We should now deprecate wand builder and make it not registered by default

Not registering is not required to let image magick to be the default builder. Not registering Wand builder will potentially introduce regressions (in case some formats are supported by wand and not by image magick)

lebouquetin commented 3 years ago

There are other couple of builder which support same format like svg file (supported by both cairosvg and inkscape).

This is not a problem: the support may be different and one be better in some cases and the other in other cases. The right thing to do for these cases is (for the future):

I think the right way to do so is to implement something like a priority weight which would be resolved at load time. This way, it also allows users to define there own builders.

inkhey commented 3 years ago

We should now deprecate wand builder and make it not registered by default

Not registering is not required to let image magick to be the default builder. Not registering Wand builder will potentially introduce regressions (in case some formats are supported by wand and not by image magick)

As discussed with @lebouquetin , we will disabled wand builder by default, as the list of supported format are the same (to be more exact supported format by wand are contained in imagemagick supported format list) , so, there is no case where wand builder disabling will remove a supported format.

inkhey commented 3 years ago

There are other couple of builder which support same format like svg file (supported by both cairosvg and inkscape).

This is not a problem: the support may be different and one be better in some cases and the other in other cases. The right thing to do for these cases is (for the future):

* explicitly define load order

* allow user to adjust the order

I think the right way to do so is to implement something like a priority weight which would be resolved at load time. This way, it also allows users to define there own builders.

Issue created there about this:https://github.com/algoo/preview-generator/issues/237

inkhey commented 3 years ago

fixed in 0.21

a523 commented 2 years ago

Hi, @inkhey Can you elaborate on this? Or provide an example?This cause issue as the wand builder is not properly designed and don't produce good quality png image (black background, low quality image). You know I want use wand instead of pillow to resize image in the end,so I would like to know what was the reason to disable the wand builder in the first place.

inkhey commented 2 years ago

You know I want use wand instead of pillow to resize image in the end,so I would like to know what was the reason to disable the wand builder in the first place.

The reason of disabling it was because: