Closed Bug 1695318 Opened 1 year ago Closed 4 months ago

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

Categories

(Core :: Graphics: WebRender, defect)

Firefox 85
defect

Tracking

()

RESOLVED FIXED
98 Branch
Tracking Status
firefox-esr91 --- disabled
firefox96 --- wontfix
firefox97 --- wontfix
firefox98 --- fixed

People

(Reporter: ksenia, Assigned: kvark)

References

(Blocks 1 open bug, Regression, )

Details

(Keywords: regression)

Attachments

(5 files)

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.)

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

Regressed by: 1525641
Has Regression Range: --- → yes

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.

Assignee: nobody → dmalyshau
Severity: -- → S3

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).

(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.

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.

Flags: needinfo?(dmalyshau)

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

Flags: needinfo?(dmalyshau)
Attached file reduced.html

Reduced test case

Attached file story.md

Investigation story

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.

Pushed by dmalyshau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cb5768cec343
Check if the parent node is perspective when considering back faces r=gw
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 98 Branch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by dmalyshau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33a1ea9e3201
Pass the perspective association from CSS to WR r=gfx-reviewers,bradwerth
Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED

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:
Attachment #9259410 - Flags: approval-mozilla-beta?
Attachment #9259430 - Flags: approval-mozilla-beta?

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.

Attachment #9259410 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9259430 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.