1659764 - Youtube video will overlap video controls if the resolution is switched to 144p while in default or theater view mode

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Timea Cernea [:tbabos][inactive]]

Affected platforms:
MacOS 10.13/10.14
Not reproducible on Windows

Affected versions:
Latest Nightly 81 (2020-08-18)

Steps to reproduce:

1. Launch Firefox
2. Go to youtube.com and click on any suggested video (in default view mode or theater)
3. Change the video resolution to 144p

Expected:
Video controls should be visible at all times.

Actual:
Video controls will be overlapped by the video itself but are still functional behind it. Recording: https://streamable.com/du6q20

Regression-Range:
Mozregression revealed Bug 1653409 to be the regressor.
2020-08-18T19:29:25.072000: DEBUG : Found commit message:
Bug 1653409 - Set ImageBitmap's picture rect using the picture rect of layers::Image, not the size. r=mstange

Differential Revision: https://phabricator.services.mozilla.com/D85356

Note:
This issue permanently affects the default/theater view on youtube.
Restarting the browser or refreshing the page does not restore the visibility of the media controls on the videos
Full-Screen view mode is not affected.

Severity Suggestion:
Due to the reasons from "notes" would suggest S2 severity since it permanently breaks the default/theater view on Youtube if the user will switch to 144p resolution on a firefox profile.

Comment 1

[Markus Stange [:mstange]]

Looks like the order of the native layers is wrong. Maybe WR is making a mistake when calculating whether the video and video controls overlap?

Comment 2

[Timea Cernea [:tbabos][inactive]]

If Webrender is force-disabled, this issue can no longer be reproduced. Also, it fixes FX profiles which already got affected by this issue.

Comment 3

[Nicolas Silva [:nical]]

Also ni? Glenn in case it is something in the picture caching layerization heuristics. I can't reproduce this on linux, I suspect it is only happening with native OS surfaces.

Comment 4

[Glenn Watson [:gw]]

Attachment: Bug 1659764 - Fix compositor surface overlap with native compositors.

This patch fixes a regression introduced when support was added
for transforms with native OS compositor surfaces.

We need to store both the true device rect and also the pre-transform
device rect for the external surface descriptor. The true device
rect is used for overlap detection to order compositor surfaces
correctly, while the pre-transform rect is sent to the native OS
compositor, since the transform is applied at that level.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3d572eae359
Fix compositor surface overlap with native compositors. r=mattwoodrow

Comment 6

[Alexandru Michis [:malexandru]]

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

Comment 7

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on latest Nightly 82.0a1 (2020-08-24) on MacOS 10.13/10.14/10.15. Waiting for Uplift to Fx81.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

AFAIK, WR is disabled by default on macOS so I don't think we need to uplift this. Feel free to correct me if that's wrong.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

This is reproducible on Windows, see duplicate bug 1659747.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Thanks for pointing that out. Glenn, please nominate this for Beta uplift when you get a chance.

Comment 12

[Glenn Watson [:gw]]

Comment on attachment 9171103 [details]
Bug 1659764 - Fix compositor surface overlap with native compositors.

Beta/Release Uplift Approval Request

• User impact if declined: Artifacts when playing video at low quality + WR enabled.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Per comments in bug report.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small patch, affects WR path only, only affects video so unlikely to cause regressions elsewhere.
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9171103 [details]
Bug 1659764 - Fix compositor surface overlap with native compositors.

approved for 81.0b3

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5a948819bc0c

Comment 15

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 81.0b3 (BuildId:20200827203325) on macOS 10.14