Closed Bug 1680128 Opened 10 months ago Closed 6 months ago

[webrender] no compositor flips/inverts y axis on macOS

Categories

(Core :: Graphics: WebRender, defect)

Unspecified
macOS
defect

Tracking

()

VERIFIED FIXED
89 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox87 --- unaffected
firefox88 + verified
firefox89 --- verified

People

(Reporter: bradwerth, Assigned: rmader)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Steps to Reproduce:

*** Warning ***
Following these instructions makes the browser very difficult to use. After these steps have been done, everything in the browser window is reflected about the Y axis, including mouse events.

  1. On macOS, ensure that the following prefs are set to these non-default values:
gfx.webrender.all true
gfx.webrender.compositor false
gfx.webrender.software true
  1. Re-launch the browser.

Expected Results:
Window is rendered normally.

Actual Results:
Window contents and mouse events are flipped about the Y axis.

Summary: {webrender] software fallback no compositor flips y axis on macOS → [webrender] software fallback no compositor flips y axis on macOS

This is not a complete/correct fix. It makes the window generally look correct
and makes it usable, but there is significant flickering (compositor is maybe
fighting with some other drawing mechanism?) and some UI elements still appear
inverted, namely XUL tooltips.

Blocks: gfx-triage
Blocks: sw-wr-correctness
No longer blocks: gfx-triage
Summary: [webrender] software fallback no compositor flips y axis on macOS → [webrender] software fallback no compositor flips/inverts y axis on macOS
Severity: -- → S4

Software WebRender does off-main-thread compositing. It should never be used for tooltips - nsChildView::ShouldUseOffMainThreadCompositing() returns false when called on a tooltip window.
Similarly, nsChildView::EnsureContentLayerForMainThreadPainting() should only be called on windows that don't use OMTC, and software webrender should never draw into those windows.

This is now happening for me even with hardware WebRender, with gfx.webrender.software false. If no compositor, I get the Y axis flip.

Mozregression shows that this began with https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=04ed910d0a2fa35e25c9d88a66cdead6777eacd6&tochange=e5cfc0f5d9c52a3ee66ff4a21deeea7e8bf606bc, which is likely just when the first swgl compositor landed.

Mozregression also shows that hardware WebRender began manifesting this bug when Bug 1697673 landed. Marking this as a regression of that bug.

Regressed by: 1697673
Summary: [webrender] software fallback no compositor flips/inverts y axis on macOS → [webrender] no compositor flips/inverts y axis on macOS

So we want to return true in SurfaceOriginIsTopLeft() if the native compositor is used and false if not, is that correct?

Attachment #9190672 - Attachment is obsolete: true

(In reply to Robert Mader [:rmader] from comment #5)

So we want to return true in SurfaceOriginIsTopLeft() if the native compositor is used and false if not, is that correct?

I'm not sure what the right fix is. Since the patch works in most cases -- enough to pass on Treeherder -- it may be some specific part of my setup that's generating this problem. I'm happy to try out any patches to see if they fix the issue.

For the record, I'm currently working with the code in question[1] in bug 1699985 because I hope to be able to reuse RenderCompositorNative for Wayland. As it uses the NativeLayer abstraction, I think the correct fix would be to get the value from there, as tiles on Wayland would use GL coordinates (SurfaceOriginIsTopLeft() => false) while CoreAnimation appears to use top-left coordinates them. Unfortunately I have means to test that on MacOS, but Glenn might know better.

Glenn, do you know what we need here and do you have an idea where to declare it best?

1: https://searchfox.org/mozilla-central/source/gfx/webrender_bindings/RenderCompositorNative.h#49

Flags: needinfo?(gwatson)

I'm not sure exactly what we need here - it might make sense to include it in the CompositorCapabilities [1] struct, allowing native compositor implementation to query this from Gecko and/or change what is returned there based on platform?

[1] https://searchfox.org/mozilla-central/source/gfx/wr/webrender/src/composite.rs#932

Flags: needinfo?(gwatson)

If OS compositor is off, RenderCompositorNative uses mNativeLayerForEntireWindow. And it calls mNativeLayerForEntireWindow->SetSurfaceIsFlipped(true). It could just pass false there.

Assignee: nobody → robert.mader
Status: NEW → ASSIGNED
Flags: needinfo?(bwerth)

The test build works great with hardware WebRender and no compositor. When I try with software WebRender, I get flashing content in response to hover states and scrolling.

Flags: needinfo?(bwerth)

(In reply to Brad Werth [:bradwerth] from comment #12)

The test build works great with hardware WebRender and no compositor. When I try with software WebRender, I get flashing content in response to hover states and scrolling.

Just to make sure I understand you correctly: it does fix the issue as reported ("Window contents and mouse events are flipped about the Y axis.") but now there's some flickering, right? Because that sounds to me like a follow up bug that's possibly unrelated to the flipping.

In other words: the patch does improve the situation on both, HW-WR and SW-WR, right?

Flags: needinfo?(bwerth)

(In reply to Robert Mader [:rmader] from comment #13)

(In reply to Brad Werth [:bradwerth] from comment #12)

The test build works great with hardware WebRender and no compositor. When I try with software WebRender, I get flashing content in response to hover states and scrolling.

Just to make sure I understand you correctly: it does fix the issue as reported ("Window contents and mouse events are flipped about the Y axis.") but now there's some flickering, right? Because that sounds to me like a follow up bug that's possibly unrelated to the flipping.

In other words: the patch does improve the situation on both, HW-WR and SW-WR, right?

Yes, that's correct. I just confirmed that, without the patch, turning the compositor off for SW WebRender on macOS gives the flickering I showed in the video -- just upside-down. I had never tested that issue before because the browser is so difficult to use in that configuration.

Flags: needinfo?(bwerth)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cb4d40c5fad4
Do not flip surface when not using native compositor, r=mstange
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch

(In reply to Brad Werth [:bradwerth] from comment #12)

Created attachment 9212846 [details]
Test Build SW WR flashing.mov

The test build works great with hardware WebRender and no compositor. When I try with software WebRender, I get flashing content in response to hover states and scrolling.

That sounds like there's some issue with invalidation or dirty rects maybe needing to be flipped as well somewhere inside RenderCompositorNative. Someone should file a follow-up bug so we don't forget...

IIUC, the settings needed to trigger this bug aren't likely affect real-world users, so setting the status for 88 to wontfix. If there is a path for users to encounter this in the wild without going through about:config, please nominate this for Beta approval.

This issue is a major breaking change for us at Pixar Animation Studios. We set gfx.webrender.compositor false as part of our system for managing color accuracy in Firefox, which results in an inverted render on macOS in v88. Please re-consider fixing this bug.

It should be fixed in 89 and Nightly, isn't it?

Flags: needinfo?(kris.landes)

Comment on attachment 9212794 [details]
Bug 1680128 - Do not flip surface when not using native compositor, r=gw

Beta/Release Uplift Approval Request

  • User impact if declined: Some users apparently use this setting and can't use version 88 now / would need to switch to beta.
  • Is this code covered by automated tests?: No
  • 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): Not an officially supported config, code change itself is trivial.
  • String changes made/needed:
Attachment #9212794 - Flags: approval-mozilla-release?

That is super interesting! How does setting gfx.webrender.compositor to false help you manage color accuracy? I'm assuming this is about video playback - when "compositor" is on, videos get separate layers which are individually color-managed, and when "compositor" is off, videos are drawn into the window layer.
However, it was my understanding that neither approach is correctly color-managed in Firefox. In Firefox 88 and below, our layers are treated as if they were in the monitor color space. With compositor disabled, this means that video colors will be treated as being in the monitor space even if they're not. This is usually not accurate.
With compositor enabled, we have separate CALayers for videos. And we also set the monitor color space on those. (Bug 1697265 aims to change this.) I would have expected the same result - video colors treated in the monitor color space even if they are in a different color space. But if gfx.webrender.compositor makes a difference for video display, clearly this is not equivalent.

Furthermore, in Firefox 89, our non-video layers will be treated in the sRGB color space (bug 1696688). This will probably also break the workaround! So please test 89 and report how it works for you.

So, in summary, I do not see how the workaround results in correct video colors in Firefox, and I expect 89 to break this workaround even more.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)

It should be fixed in 89 and Nightly, isn't it?

Yes, it is seemingly fixed in 89 and Nightly, which is great.

Flags: needinfo?(kris.landes)

(In reply to Markus Stange [:mstange] from comment #22)

That is super interesting! How does setting gfx.webrender.compositor to false help you manage color accuracy? I'm assuming this is about video playback - when "compositor" is on, videos get separate layers which are individually color-managed, and when "compositor" is off, videos are drawn into the window layer.

That is correct. Without gfx.webrender.compositor set to false still images are rendered in the correct colorspace but video playback is not. Different colors are handled differently, e.g. blue never reaches full value, green blows out early and desaturates, etc. We want the hardware or OpenGL path to be used for "compositing", and on the Mac the "webrender" compositor causes discrepancies with how we would expect color to be represented. This started being a problem for us at v83, when "webrender" became opt-out instead of opt-in. Prior to that OpenGL was used which resulted in predictable colorspaces.

However, it was my understanding that neither approach is correctly color-managed in Firefox. In Firefox 88 and below, our layers are treated as if they were in the monitor color space. With compositor disabled, this means that video colors will be treated as being in the monitor space even if they're not. This is usually not accurate.

Interesting. So far this has not been an issue for us, but I'm not the person who could best speak to this topic. That would be our color lead Rick Sayre, and I'm working on getting him looped into the conversation.

Furthermore, in Firefox 89, our non-video layers will be treated in the sRGB color space (bug 1696688). This will probably also break the workaround! So please test 89 and report how it works for you.

This is very good to know! I'll make sure our team tests 89 and we'll report back with our findings.

Thanks for your prompt attention to this issue! It is much appreciated.

[Tracking Requested - why for this release]: Since we're doing a dot release anyway, can we take this (Yes, I know there will be more to worry about in 89)

Yeah, it's on the list to ride along.

Comment on attachment 9212794 [details]
Bug 1680128 - Do not flip surface when not using native compositor, r=gw

Approved for 88.0.1.

Attachment #9212794 - Flags: approval-mozilla-release? → approval-mozilla-release+

Hello! managed to reproduce the issue using macOS 11.3.1 M1 with Firefox 85.0a1 (20201209213504). After setting pref's from comment 0 and restarting the browser is displayed upside down.

Using Firefox 89.0b8 (20210504185920) and 88.0.1 (20210504152106) on macOS 11.3.1 M1 and Intel and macOS 15 and the same pref's from comment 0 the browser is no longer flipped and it's displayed as expected. However, the flickering issue is still present and also we encounter a hang or a crash when visiting webpages and scrolling (e.g on cnn.com).

Given the above comments, I just want to confirm that these remaining issues are expected when using these preferences and this issue only fixes the flipping part. Is this correct? Thank you!

Flags: needinfo?(robert.mader)

Yes, that's correct. The flickering issue is unrelated.

Quoting from comment 17:

Someone should file a follow-up bug so we don't forget...

Will do so now :)

Flags: needinfo?(robert.mader)
See Also: → 1709590

Thank you, Robert!
Closing this as verified per comment 30 and comment 29.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.