Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent resize behaviour re. premultiplication #1629

Closed
jaubourg opened this issue Apr 27, 2020 · 8 comments
Closed

inconsistent resize behaviour re. premultiplication #1629

jaubourg opened this issue Apr 27, 2020 · 8 comments

Comments

@jaubourg
Copy link
Contributor

I'm using libvips 8.9.1 and I've reduced my case to the following snippet:

VImage v_image = VImage::new_from_file(
    "input.png",
    VImage::option()
        ->set( "access", VIPS_ACCESS_RANDOM )
        ->set( "fail", false )
);
v_image = v_image.colourspace( VIPS_INTERPRETATION_sRGB );
v_image = v_image.premultiply();
v_image = v_image.resize( 1.5 );
v_image = v_image.unpremultiply();
v_image = v_image.flatten(
    VImage::option()->set( "background", std::vector<double>( { 255., 255., 255. } ) )
);
v_image.write_to_file( "output.png" );

This works splendidly with a png32 but there are artefacts appearing at opacity edges with a png8 image (see images attached at the end of this post).

flatten() is only used to clearly and visually exhibit the issue (artefacts appear on a transparent output all the same).

From what I can gather:

  • colourspace() is not to be blamed (the same artefacts appear without it)
  • removing .premultiply() and unpremultiply() removes the issue

I honestly have no clue what the problem can be. Am I supposed not to premultiply for upscaling?

PNG 8 INPUT

input png8

PNG 8 OUTPUT

output png8

@jcupitt
Copy link
Member

jcupitt commented Apr 30, 2020

Hi @jaubourg,

You're right, there's a bug here, or an inconsistency anyway. resize uses different resampling operations depending on the resize ratio -- for upsizing (x1.5 in this case) it uses affine, and affine will premultiply/unpremultiply for you. For downsize, it uses reduce which does not call premultiply automatically.

As a workaround, you can just remove the premultiply/unpremultiply for upsizing.

For 8.10, we need to make resize consistent. I'll have a look.

Thanks for reporting this dumbness!

@jcupitt jcupitt changed the title Odd resize behaviour on premultiplied png8 inconsistent resize behaviour re. premultiplication Apr 30, 2020
@jcupitt
Copy link
Member

jcupitt commented Apr 30, 2020

Python test program:

#!/usr/bin/python3 
  
import sys
import pyvips

image = pyvips.Image.new_from_file(sys.argv[1])
#image = image.premultiply()
image = image.resize(1.5)
#image = image.unpremultiply()
image = image.flatten(background=255)
image.write_to_file(sys.argv[2])

@jaubourg
Copy link
Contributor Author

Ideal solution would be to have a premultiplied flag in the options for resize like we have for embed. I admit I kinda micro-manage the orders I do premultiplications, color transformations and re-orientation of images to gain a little speed here and there.

@jaubourg
Copy link
Contributor Author

The question becomes : should resize assume premultiplied or not premultiplied by default ;)

jcupitt added a commit that referenced this issue May 19, 2020
vips_resize() uses vips_affine() for upsizing and vips_reduce() for
downsizing. Affine automaticaly does a vips_premultiply() for images
with an alpha channel, but reduce does not. This meant that we could
sometimes premultiply twice.

This patch adds a "premultiplied" flag for affine which turns automatic
premultiuplication off, vips_resize() uses this to block affine's auto
premul feature, and the resize docs are clarified to stress that the
operation does not do premultiplication for you.

See #1629
@jcupitt
Copy link
Member

jcupitt commented May 19, 2020

OK, I think this is fixed. I've revised the docs to stress that you need to premultiply before resize if the image has an alpha (thumbnail has always done this). Affine has a new flag to disable auto premultiplication.

@jcupitt jcupitt closed this as completed May 19, 2020
@jcupitt
Copy link
Member

jcupitt commented May 19, 2020

(and thanks again for the report!)

@jaubourg
Copy link
Contributor Author

jaubourg commented May 19, 2020

Well, thank you for the fix! :) Looks perfect. Which version will it be in (looks like kinda of a breaking change)?

@jcupitt
Copy link
Member

jcupitt commented May 19, 2020

It's in git master, so it'll be part of 8.10, due in a few months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants