VSE: Fix black borders on cropped and scaled images
AbandonedPublic
Actions

Authored by Isabelle Knott (blacklemori) on Mar 24 2021, 4:59 AM.

Details

Reviewers

Richard Antalik (ISS)
Sergey Sharybin (sergey)

Summary

This patch resolves a problem in the sequencer where a cropped and scaled image will have a black border around it. This occurs because the cropped area is filled with the RGBA colour [0,0,0,0], which is "black transparency." When the image is scaled, the bilinear or bicubic interpolators will cause a black halo to appear around the cropped area.

To reproduce the issue it is sufficient to add two white color strips overtop of each other, with the top one given the "alpha over" blend mode. If this top strip is cropped and scaled up by 2x, a black border will appear around it when rendered. Attached here is a particularly extreme example where a cropped color strip is scaled 16x:

Blender file:

black-border.blend611 KBDownload

Render:

To resolve this issue, this patch modifies the input_preprocess function in render.c to only replace the alpha channel of the cropped area. This preserves the original colour of those pixels from before the crop.

Diff Detail

Repository: rB Blender

Event Timeline

Isabelle Knott (blacklemori) requested review of this revision.Mar 24 2021, 4:59 AM

Isabelle Knott (blacklemori) created this revision.

Thanks for patch,
I think this is best solution really.
Not very happy about blurred edges, but since transform can rotate the image, I am not sure if this can be solved without affecting performance.

Adding @Sergey Sharybin (sergey) as reviewer as this is his area.

This revision is now accepted and ready to land.Mar 24 2021, 7:50 AM

Peter Fog (tintwotin) awarded a token.Mar 24 2021, 1:10 PM

Dalai Felinto (dfelinto) added 1 blocking reviewer(s): Sergey Sharybin (sergey).Mar 26 2021, 5:06 PM

This revision now requires review to proceed.Mar 26 2021, 5:06 PM

This doesn't really sound like a complete fix to me. It solves one specific case, but it does not prevent color bleed in general? From the description and code it seems that if you get an input which is a white square on black background and crop exactly around the square, the issue will still happen.

The other concern here is that this change breaks premultiplied alpha in float buffers.

So all this kind of suggests that the proper fix is somewhere else?

In D10798#276032, @Sergey Sharybin (sergey) wrote:

This doesn't really sound like a complete fix to me. It solves one specific case, but it does not prevent color bleed in general?

It doesn't prevent it in general. Haven't thought about premul, so this really needs better solution.

I have checked how does this work in compositor, and similar issue can be observed there but only with bilinear interpolation. Bicubic works fine. See sample below.

colorbleed.blend781 KBDownload

I would guess that compositor limits or masks "scanning area" and modifies only pixels that should be.
Sequencer calls scaling code for each pixel in destination image, even if it doesn't map on to original image.

@Isabelle Knott (blacklemori) will you be able to look at compositor node code and propose better solution for this problem?

This revision now requires changes to proceed.Mar 30 2021, 3:36 AM

I would have time mid-next week to look at it. I think if the default wrapping mode for the interpolators was clamp instead of black that might solve the problem, since I noticed this also happens if you translate a white color so its edge is on screen and scale it up. Indeed, my patch doesn't solve this case:

black-border-edge.blend607 KBDownload

The code that does the crop might also need changing to take advantage of the clamp, but I'm not familiar with the code that performs the image transform yet. Essentially, if an interpolator queries the value of a pixel out-of-image, it should take it to be the colour of the closest pixel in-image, but transparent.

How can one tell if the float buffer should use premultiplied alpha? I see premultiply and unpremultiply functions in imbuf, and some comments saying that certain code assumes premultiplied, is there a general principle?

In D10798#276432, @Isabelle Knott (blacklemori) wrote:

How can one tell if the float buffer should use premultiplied alpha? I see premultiply and unpremultiply functions in imbuf, and some comments saying that certain code assumes premultiplied, is there a general principle?

As far as I can tell this is really based on assumption and there is no way to check. Basically uchar images are assumed to be straight and float to be premultiplied.

I have created diff to fix this issue in D11058, So I will close this patch. Thanks for submission anyway.