1642549 - Incorrect rendering for scaled mix-blend pictures with non-zero content offset

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Connor Brewster [:cbrewster]]

Attachment: test-case.html

While looking into bug 1642072 I noticed some textures were rendering incorrectly on the demo at https://keithclark.co.uk/labs/css-fps/nojs/

I was able to narrow it down to smaller test-case, we incorrectly render elements that have a perspective transform, background-blend-mode, and are partially clipped by the viewport bounds.

This is a regression from bug 1559861 which changes surface device pixel scaling on the fly.

Comment 1

[Connor Brewster [:cbrewster]]

This also occurs when using a filter effect or opacity instead of background-blend-mode. The intermediate targets appear to be rendering correctly, so I suspect this means that the brush_opacity, brush_blend, and brush_mix_blend shaders don't handle the perspective transform + scaling factor case quite right.

Edit: This happens with any of the brush shaders rendering a perspective transformed picture with a different device pixel scale than the surface being rendered into.

Comment 2

[Connor Brewster [:cbrewster]]

I've narrowed this down a bit more. The incorrect rendering happens when (1) a picture task has a different device pixel scale than the parent surface it will be composited into and (2) the picture task has a non-zero content offset (due to viewport clipping or inflation from blur/drop-shadow).

I think part of the problem is that when we composite a picture into a parent picture, we assume that both pictures share the same "device space", but this is not always the case (either due to my scaling patch, or the 4096x4096 max picture size check).

Comment 3

[Bert Peers [:bpeers]]

Thanks for investigating! Yes we do need to explicitly handle the different scaling factors of different "inputs".
Here is an example diff where the code used to assume that source, backdrop and output are all in the same scale and so offsets and UVs can just be shared. Unfortunately that assumption is silent so that can be hard to find -- it's a bug by omission :)

There may be other shaders that need to be fixed, and/or maybe I didn't get that patch quite right, and/or you've hit another subtlety :) -- but hopefully this gives some context. Thanks.

Comment 4

[Tim Spurway [:tspurway]]

S1 or S2 bugs need an assignee - could you find someone for this bug?

Comment 5

[Connor Brewster [:cbrewster]]

I'm working on a fix for this.

Comment 6

[Glenn Watson [:gw]]

Sounds like you have the info needed from Bert, ping me again if there is more to follow up here.

Comment 7

[Connor Brewster [:cbrewster]]

Attachment: Bug 1642549: Fix incorrect offset scaling when handling mix-blend readback r=gw,Bert

When handling mix-blend readbacks, we need to calculate the origin of the source picture in the backdrop's space.
Before this patch, this was done by scaling the source picture origin by the backdrop's device pixel scale.
However, this is the incorrect scaling; We really want the relative scale between source and backdrop instead.
This patch computes the relative scaling from the device pixel scale of both the source and backdrop pictures.

This bug likely went unnoticed because the device pixel scale used to be 1 in most cases, but now we sometimes
increase the device pixel scale for raster roots.

Comment 8

[Connor Brewster [:cbrewster]]

Thanks for the info Bert, that was very helpful!

Looks like I have been battling two different bugs (which led to a very confusing day). The bug introduced by bug 1559861 only applies to mix-blend pictures. (Which is fixed by the patch I just attached)

The other bug causes the stretching seen in the test case, but it is unrelated to the mix-blend offset bug. Also, mozregression shows that it is not a regression from bug 1559861. I'll open a new issue so we can track that separate from this bug. (Bug 1643236)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4db2b3b6b8e
Fix incorrect offset scaling when handling mix-blend readback r=gw

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/b4db2b3b6b8e

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:cbrewster, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Connor Brewster [:cbrewster]]

Attachment: incorrect-rendering.png

Comment 13

[Connor Brewster [:cbrewster]]

Comment on attachment 9154075 [details]
Bug 1642549: Fix incorrect offset scaling when handling mix-blend readback r=gw,Bert

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect rendering of elements with mix-blend modes and perspective transforms. For example, https://keithclark.co.uk/labs/css-fps/nojs/ renders incorrectly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Go to https://keithclark.co.uk/labs/css-fps/nojs/ and watch the animation for 10-20 seconds.
  Some wall textures are incorrectly offset. See the attached incorrect-rendering.png for an example.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very small patch with test coverage. It only affects rendering in rare cases (mix-blend + perspective). If this causes any problems, it will be easy to back out.
• String changes made/needed: none

Comment 14

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest build 79.0a1 (2020-06-08) on Windows 10.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9154075 [details]
Bug 1642549: Fix incorrect offset scaling when handling mix-blend readback r=gw,Bert

approved for 78.0b5

Comment 16

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/af3142edcff8

Comment 17

[Rares Doghi, Desktop QA]

This issue is Verified as fixed in our latest Beta build 78.0b7 on Windows 10.