libvips / ruby-vips

Ruby extension for the libvips image processing library.
MIT License
828 stars 62 forks source link

Gray line on the edges in the multiply process #400

Open JohnAnon9771 opened 1 month ago

JohnAnon9771 commented 1 month ago

Describe the bug When I try to perform a composite process with multiply, for some reason the overlay image gets a gray border. This is only happening with the multiply process. When checking with Photoshop, this issue does not occur, and the image appears without the border as it should from the beginning.

To Reproduce

def compose_images(art_image_path:, base_image_path:, x:, y:)
  base_image = Vips::Image.new_from_file(base_image_path)
  art_image = Vips::Image.new_from_file(art_image_path)

  base_image.composite(art_image, :multiply, x:, y:)
end

Resulting image:

testing4

Expected image:

composed_image20240715-2-oo8fl7

Additional context

To solve this problem, I applied a white background instead of transparent, but I'm not sure if this is the correct way to do it...

def compose_images(art_image_path:, base_image_path:, x:, y:)
  base_image = Vips::Image.new_from_file(base_image_path)
  art_image = Vips::Image.new_from_file(art_image_path)
  art_image = art_image.flatten(background: [255, 255, 255])

  base_image.composite(art_image, :multiply, x:, y:)
end
jcupitt commented 1 month ago

Hi @JohnAnon9771, could you upload your overlay and base images too?

JohnAnon9771 commented 1 month ago

Hi @JohnAnon9771, could you upload your overlay and base images too?

Sure! @jcupitt

Positions: X: -448, Y: 395

base_image:

shirt_pocket

art_image:

testing

jcupitt commented 1 month ago

I think your art image has been premultiplied -- ie. the alpha has already been blended into the RGB layer. If I run:

$ vips composite2 base_image.jpg art_image.png x.png multiply \
    --x=-448 --y=395 --premultiplied

I get:

x

Which looks better to me.

You can either save your art image as unpremultiplied (PNG images are supposed to be unpremultiplied), or pass the --premultiplied flag to composite.

jcupitt commented 1 month ago

Oh, maybe not, I tried unpremultiplying your art image and it looks bad. But I looked at the sources again and I can't see anything wrong with MULTIPLY :( Puzzling!

kleisauke commented 1 month ago

Looking at: https://gitlab.freedesktop.org/pixman/pixman/-/blob/865e6ce00bb79a6b925ed4c2c436e1533e4472aa/pixman/pixman-combine-float.c#L372-374

It seems Pixman does (if I understand correctly):

xR = \frac{1}{aR} \times \left[(1 - aB) \times xaA + (1 - aA) \times xaB + \mathbf{f}(xA, xB) \right]

instead of:

xR = \frac{1}{aR} \times \left[(1 - aB) \times xaA + (1 - aA) \times xaB + aA \times aB + \mathbf{f}(xA, xB) \right]

(i.e. the equation documented at https://www.cairographics.org/operators/)

So, there's a missing $aA \times aB$ in Pixman's implementation. By applying this patch:

Details ```diff --- a/libvips/conversion/composite.cpp +++ b/libvips/conversion/composite.cpp @@ -488,7 +488,6 @@ vips_composite_base_blend(VipsCompositeBase *composite, double aR; double t1; double t2; - double t3; double f[MAX_BANDS + 1]; /* Load and scale the pixel to 0 - 1. @@ -701,9 +700,8 @@ vips_composite_base_blend(VipsCompositeBase *composite, t1 = 1 - aB; t2 = 1 - aA; - t3 = aA * aB; for (int b = 0; b < bands; b++) - B[b] = t1 * A[b] + t2 * B[b] + t3 * f[b]; + B[b] = t1 * A[b] + t2 * B[b] + f[b]; break; } @@ -732,7 +730,6 @@ vips_composite_base_blend3(VipsCompositeSequence *seq, float aR; float t1; float t2; - float t3; v4f f; v4f g; @@ -934,8 +931,7 @@ vips_composite_base_blend3(VipsCompositeSequence *seq, t1 = 1 - aB; t2 = 1 - aA; - t3 = aA * aB; - B = t1 * A + t2 * B + t3 * f; + B = t1 * A + t2 * B + f; break; } ```

I see: x-multiply

When running:

$ vips composite2 base_image.jpg art_image.png x-multiply.png multiply --x=-448 --y=395
jcupitt commented 1 month ago

Ah great detective work. But how strange! That will change all 11 of the PDF blend modes, won't it? You'd think we'd have noticed this before.

kleisauke commented 1 month ago

Indeed, that will change all 11 separable PDF blend modes. :(

Perhaps the aA and aB calculations are integrated into other blend modes? For example, I noticed the screen blend mode in Pixman is actually implemented as:

f = A \times  aA + B \times  aB - A \times B

instead of just:

f = A + B - A \times B

See: https://gitlab.freedesktop.org/pixman/pixman/-/blob/865e6ce00bb79a6b925ed4c2c436e1533e4472aa/pixman/pixman-combine-float.c#L403

JohnAnon9771 commented 1 month ago

Looking at this comment, it seems that this problem has already been seen.

https://github.com/libvips/libvips/issues/1622#issuecomment-630855734

kleisauke commented 1 month ago

These two scripts appear to produce visually identical images: cairo.c:

/* compile with:
 * gcc -g -Wall cairo.c `pkg-config cairo --cflags --libs` -o cairo
 */

#include <cairo.h>

int
main(int argc, char *argv[])
{
    cairo_surface_t *surface;
    cairo_t *cr;

    surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 160, 120);
    cr = cairo_create(surface);

    cairo_rectangle(cr, 0, 0, 120, 90);
    cairo_set_source_rgba(cr, 0.7, 0, 0, 0.8);
    cairo_fill(cr);

    cairo_set_operator(cr, CAIRO_OPERATOR_MULTIPLY);

    cairo_rectangle(cr, 40, 30, 120, 90);
    cairo_set_source_rgba(cr, 0, 0, 0.9, 0.4);
    cairo_fill(cr);

    cairo_destroy(cr);
    cairo_surface_write_to_png(surface, "cairo.png");
    cairo_surface_destroy(surface);

    return 0;
}

vips.py:

import pyvips

base = (pyvips.Image.black(120, 90)
        .copy(interpretation='srgb')
        .new_from_image([179, 0, 0, 204])
        .embed(0, 0, 160, 120))

overlay = (pyvips.Image.black(120, 190)
           .copy(interpretation='srgb')
           .new_from_image([0, 0, 230, 102])
           .embed(40, 30, 160, 120))

im = base.composite2(overlay, 'multiply')
im.write_to_file('vips.png')

Run with:

$ gcc -g -Wall cairo.c `pkg-config cairo --cflags --libs` -o cairo
$ ./cairo
$ python vips.py
$ vips subtract vips.png cairo.png x.v
$ vips abs x.v x2.v
$ vips max x2.v
1.000000
Output: vips.png cairo.png
vips cairo

So, it looks there's nothing wrong with VIPS_BLEND_MODE_MULTIPLY.

kleisauke commented 1 month ago

Hmm, for VIPS_BLEND_MODE_SCREEN you also need to pass premultiplied=True to ensure the images are visually identical to those produced by Cairo.

--- a/cairo.c
+++ b/cairo.c
@@ -17,7 +17,7 @@ main(int argc, char *argv[])
    cairo_set_source_rgba(cr, 0.7, 0, 0, 0.8);
    cairo_fill(cr);

-   cairo_set_operator(cr, CAIRO_OPERATOR_MULTIPLY);
+   cairo_set_operator(cr, CAIRO_OPERATOR_SCREEN);

    cairo_rectangle(cr, 40, 30, 120, 90);
    cairo_set_source_rgba(cr, 0, 0, 0.9, 0.8);
--- a/vips.py
+++ b/vips.py
@@ -10,5 +10,5 @@ overlay = (pyvips.Image.black(120, 190)
            .new_from_image([0, 0, 230, 204])
            .embed(40, 30, 160, 120))

-im = base.composite2(overlay, 'multiply')
+im = base.composite2(overlay, 'screen', premultiplied=True)
 im.write_to_file('vips.png')
If you don't do this, it will produce: vips.png cairo.png
vips cairo

Perhaps the separable PDF blend modes in Cairo always operates on unpremultiplied values?