1657312 - [Youtube] Masthead flickers when scrolling while a video is playing in theater mode

Status: VERIFIED FIXED

(Core :: Graphics, defect, P3)

Description

[Emil Ghitta, QA [:emilghitta]]

Affected versions

• 81.0a1 (BuildId:20200804215246)

Unaffected versions

• 80.0b3 (BuildId:20200803045446)
• 79.0 (BuildId:20200720193547)
• 78.1.0esr (BuildId:20200722151235)
• 68.11.0esr (BuildId:20200720181548)

Affected platforms

• Windows 10 64bit

Unaffected platforms

• macOS 10.14
• Ubuntu 18.04 64bit.

Steps to reproduce

1. Launch Firefox.
2. Access the following link or access any youtube video.
3. Maximize the browser window.
4. Enter in theater mode and play the video.
5. Scroll down and up.

Expected result

• No UI issues are encountered.

Actual result

• A flicker can be observed at the top portion of the masthead.

Regression Range
This seems to be a regression:

• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4ed6ec0c8220321b13c108ffaa7ab6cfc8d7698f&tochange=4558b3cfd6fe49647780a0ec86dfa97f0dd3adcb
• Potential regressor: Bug 1653409

Notes

• For further information regarding this issue please access the following link to see the screenshot.
• [Suggested Severity] Since this is a cosmetic issue I think that marking this as an S4 issue fits here.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Matt!

It seems that mozregression pointed out Bug 1653409 - Set ImageBitmap's picture rect using the picture rect of layers::Image, not the size. r=mstange for causing this regression (I'm not sure about this since your code targeted macOS ? and this issue doesn't seem reproducible on macOS machines).

Could you please take a look?

Thank you!

Comment 2

[Markus Stange [:mstange]]

This was likely caused by bug 1653166 which made us use a scaled DirectComposition visual for scaled video, and thus affects Windows.

Comment 3

[Markus Stange [:mstange]]

Probably due to the way the clip and the transform combines. It seems the rectangle that DC computes for the transformed clip doesn't quite match the rectangle that we intended.
https://searchfox.org/mozilla-central/rev/a3b25e347e2c22207c4b369b99246e4aebf861a7/gfx/webrender_bindings/DCLayerTree.cpp#362-377

And indeed, a pixel-aligned rectangle C isn't necessarily equal to transformed(rounded(untransformed(C))).

Maybe we should adjust the transform to match the rounded rects, like so:
B = rounded(untransformed(C))
T = gfxUtils::TransformRectToRect(B, C)

Comment 4

[Matt Woodrow (:mattwoodrow)]

I don't know why I rounded the clip, I think we can just not do that.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1657312 - Don't round clip rect. r?mstange

Depends on D86088

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e06ab2926d5
Don't round clip rect. r=mstange

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/8e06ab2926d5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9169944 [details]
Bug 1657312 - Don't round clip rect. r?mstange

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect clipping of videos on youtube, 1px bar visible
• 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): Super simple, just changes clip rounding, so can't have a bigger impact than 1px
• String changes made/needed: None

Comment 10

[Cristian Fogel [:cfogel]]

Verified patch with 82.0a1 (2020-08-26).
Waiting for 81 before updating the rest of the status-flags.

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9169944 [details]
Bug 1657312 - Don't round clip rect. r?mstange

approved for 81.0b3

Comment 12

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d6ac877c7f3

Comment 13

[Cristian Fogel [:cfogel]]

Verified with 81.0b3