1695318 - Black patches appear in a 3d tour on youriguide.com

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ksenia Berezina [:ksenia]]

Attachment: Screen Shot 2021-02-25 at 3.03.08 PM.png

Following up on the comment 6. There seems to be a regression causing black patches appear in a 3d tour.

Steps to reproduce:

1. Open https://youriguide.com/api-docs-sample in Firefox release on Mac.
2. Select "Laundry" on the map
3. Look around the room, and observe left and right sides of the screen

Expected:
No black patches

Actual:
Flickering black patches. The problem is better observed when rotating slowly and looking down.

I run mozregression and it points to https://bugzilla.mozilla.org/show_bug.cgi?id=1648601

 4:58.79 INFO: Narrowed integration regression window from [d4d45635, d25dfe67] (3 builds) to [d4d45635, a1336e13] (2 builds) (~1 steps left)
 4:58.79 INFO: No more integration revisions, bisection finished.
 4:58.79 INFO: Last good revision: d4d4563523acc5f8975b086d46fc382749dd5a7b
 4:58.79 INFO: First bad revision: a1336e13d8ae8947a7b1f87a94cb37c74b32a3e9
 4:58.79 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d4d4563523acc5f8975b086d46fc382749dd5a7b&tochange=a1336e13d8ae8947a7b1f87a94cb37c74b32a3e9

(tried the 3d tour with and without webrender enabled and still see the black patches.)

Comment 1

[Timothy Nikkel (:tnikkel)]

Regression range with webrender enabled (command line ./mach mozregression --pref gfx.webrender.all:true -a "https://youriguide.com/api-docs-sample" --good 2019-01-01)

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=588535ba7633dee062173d41059b403fd3939e4e&tochange=f6766ba4ac77c6757c4e4db7598a2e685f23fcf6

Most likely bug 1525641

Comment 2

[Dzmitry Malyshau [:kvark]]

Thank you for finding the regression range! I'm curious as to why this would be mac-specific: the changes in bug 1515641 are not mac-specific.
Actually, testing on a mac right now, I don't see any black quads either...
Notably, the interpolation appears visually broken (all the images are skewed), but this is a different issue, possibly.

Comment 3

[Ksenia Berezina [:ksenia]]

Notably, the interpolation appears visually broken (all the images are skewed), but this is a different issue, possibly.

This is probably https://bugzilla.mozilla.org/show_bug.cgi?id=1694750 (the patch making images distorted was reverted a couple days ago).

Comment 4

[Timothy Nikkel (:tnikkel)]

(In reply to Dzmitry Malyshau [:kvark] from comment #2)

Thank you for finding the regression range! I'm curious as to why this would be mac-specific: the changes in bug 1515641 are not mac-specific.

I'm not sure if it is mac specific, the original reporter and myself just happened to use mac. I haven't tested anywhere else.

Actually, testing on a mac right now, I don't see any black quads either...

Not sure why, it was really easy to reproduce for me.

Comment 5

[Ksenia Berezina [:ksenia]]

Hi Dzmitry, wonder if it's possible to prioritize this as it seems to be the only remaining issue to make Firefox "fully supported" on youriguide.com?

I'm curious as to why this would be mac-specific: the changes in bug 1515641 are not mac-specific.

Just to add to this, I've tested it on Windows and it's also reproducible (with or without webrender enabled), so it's not mac-specific.

Comment 6

[Dzmitry Malyshau [:kvark]]

Thank you for pinging! I"ll take another look at it now.

Comment 7

[Dzmitry Malyshau [:kvark]]

Attachment: reduced.html

Reduced test case

Comment 8

[Dzmitry Malyshau [:kvark]]

Attachment: story.md

Investigation story

Comment 9

[Dzmitry Malyshau [:kvark]]

What I'm seeing is simply that one of the faces "face2" has a transformation that makes it back visible and front invisible. Here is the transform as seen by WR:

         m11: -0.017051794,
         m12: -0.7714886,
         m13: -0.6360145,
         m14: 0,
         m21: 0,
         m22: 0.6361071,
         m23: -0.7716009,
         m24: 0,
         m31: 0.99985456,
         m32: -0.01315718,
         m33: -0.010846766,
         m34: 0,
         m41: 11.068237,
         m42: 689.11334,
         m43: 1211.0691,
         m44: 1,

Our backface algorithm considers the sign of transform.inverse().m33, which is -0.0108467666 (making its back face visible instead of the front face). This is exactly non-inverse m33 for some reason... I suppose it may happen if the transform doesn't rotate Z but only scales it.

If we remove the backface test in spatial_tree.rs the image shows up correctly.

Interestingly, Gecko's inspector shows the following computed transform:

: matrix3d(-0.0170518, -0.771489, -0.636015, 0, 0, 0.636107, -0.771601, 0, 0.999855, -0.0131572, -0.0108468, 0, -399.442, 5.25629, 244.333, 1)

It's the same rotation but with a different translation (which makes sense, because this one is not for the immediate parent of the image), so it should still be invisible. Chrome produces the same transformation fwiw, but it considers it visible.

I need to check if our understanding of the backface semantics is right. The algorithm doesn't appear to be doing anything strange.

Comment 10

[Dzmitry Malyshau [:kvark]]

Attachment: Bug 1695318 - Check if the parent node is perspective when considering back faces r=gw

Comment 11

[Dzmitry Malyshau [:kvark]]

Attachment: Bug 1695318 - Pass the perspective association from CSS to WR r=gw

Depends on D136163

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb5768cec343
Check if the parent node is perspective when considering back faces r=gw

Comment 13

[Cristian Tuns]

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33a1ea9e3201
Pass the perspective association from CSS to WR r=gfx-reviewers,bradwerth

Comment 15

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/33a1ea9e3201

Comment 16

[Dzmitry Malyshau [:kvark]]

Comment on attachment 9259410 [details]
Bug 1695318 - Check if the parent node is perspective when considering back faces r=gw

Beta/Release Uplift Approval Request

• User impact if declined: Some websites using 3D transformations are not rendered correctly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): There is no security risk here. In the worst case this change breaks something unexpected. But the benefit of fixing it seems to overweight that risk for me.
• String changes made/needed:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9259410 [details]
Bug 1695318 - Check if the parent node is perspective when considering back faces r=gw

This has been an issue with WR since we started rolling it out. I think we can let this fix ride 98 to release instead.