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

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Brad Werth [:bradwerth]]

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

2. Re-launch the browser.

Expected Results:
Window is rendered normally.

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

Comment 1

[Brad Werth [:bradwerth]]

Attachment: Bug 1680128: Partially fix the inverted Y axis with webrender software no compositor.

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.

Comment 2

[Markus Stange [:mstange]]

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.

Comment 3

[Brad Werth [:bradwerth]]

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

Comment 4

[Brad Werth [:bradwerth]]

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.

Comment 5

[Robert Mader [:rmader]]

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

Comment 6

[Brad Werth [:bradwerth]]

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

Comment 7

[Robert Mader [:rmader]]

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

Comment 8

[Glenn Watson [:gw]]

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

Comment 9

[Markus Stange [:mstange]]

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

Comment 10

[Robert Mader [:rmader]]

Attachment: Bug 1680128 - Do not flip surface when not using native compositor, r=gw

Comment 11

[Robert Mader [:rmader]]

Brad, can you test this try build? https://treeherder.mozilla.org/jobs?repo=try&revision=573c6c2db4b8e3e39ea48383b7233ed76437695f

Comment 12

[Brad Werth [:bradwerth]]

Attachment: 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.

Comment 13

[Robert Mader [:rmader]]

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

Comment 14

[Brad Werth [:bradwerth]]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb4d40c5fad4
Do not flip surface when not using native compositor, r=mstange

Comment 16

[Alexandru Michis [:malexandru]]

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

Comment 17

[Lee Salzman [:lsalzman]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 19

[Kristoffer Landes]

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.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

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

Comment 21

[Robert Mader [:rmader]]

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:

Comment 22

[Markus Stange [:mstange]]

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.

Comment 23

[Kristoffer Landes]

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

Comment 24

[Kristoffer 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.

Comment 25

[Mike Kaply [:mkaply]]

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

Comment 26

[Ryan VanderMeulen [:RyanVM]]

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

Comment 27

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 88.0.1.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/47adb3bed3a4

Comment 29

[Alexandru Trif, Desktop QA [:atrif]]

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!

Comment 30

[Robert Mader [:rmader]]

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

Comment 31

[Alexandru Trif, Desktop QA [:atrif]]

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