Closed Bug 1441743 Opened 8 years ago Closed 8 years ago

[Wayland] UI elements on youtube flicker when playing videos

Categories

(Core :: Widget: Gtk, defect, P3)

Product:
Core
Component:
Widget: Gtk
Version:
59 Branch
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox59 --- wontfix
firefox62 --- fixed

People

(Reporter: mo.mashi69, Assigned: stransky)

References

(Blocks 1 open bug,
URL
)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates,
59 bytes, text/x-review-board-request
jhorak
: review+
Details
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0 Build ID: 20180227130131 Steps to reproduce: Build FF 59b (at 59b13 as of today). Use ac_add_options --enable-default-toolkit=cairo-gtk3-wayland switch in mozconfig Start build. Actual results: The UI elements of youtube flicker drastically while viewing videos. System become slow while viewing videos (not sure if it's related). Video itself displays perfectly but youtube's UI elements such as time line, volume, pause play buttons, annotations flicker violently. Flickering looks pretty much like this: https://bug1335488.bmoattachments.org/attachment.cgi?id=8832141 Expected results: UI should not flicker. Additional Notes: ---------------------- The only similar issue I've seen was here: https://bugzilla.mozilla.org/show_bug.cgi?id=1335488 - but it is not the same because the user in that thread was on MacOS X and I am on Wayland in Linux (also, the bug in that thread disappeared by itself). Things I've tried: - building same code without the Wayland switch (aka X11 native FF). Result: Problem Absent Under X11. - using a brand new profile with no extensions or changed settings. Result: Problem persist with new profile. - disabling hardware acceleration, Result: Problem persists. - runnning FF in safemode. Result: Problem persists.
Fadi R, do you know/could you please check if the problem reproduces with using a FF60/FF58 wayland build? It would be most useful to figure if this is a regression or not? Meanwhile, moving this issue to Graphics, since IMO that would be a step forward.
status-firefox58: --- → ?
status-firefox59: --- → affected
status-firefox60: --- → ?
Component: Untriaged → Graphics
OS: Unspecified → Linux
Product: Firefox → Core
Hardware: Unspecified → x86_64
Flags: needinfo?(mo.mashi69)
Sure. It won't be a problem testing 60 but I haven't built firefox-wayland for 58 before. Does it have wayland support built into it or do I need apply a patch from somewhere? if it has wayland support, do I just use the ac_add_options --enable-default-toolkit=cairo-gtk3-wayland switch as I did with 59?
Flags: needinfo?(mo.mashi69)
I've tested 60 off of central. Glitch is still there. Just to be clear, the glitch was in 59 (before and after I applied the EGL patch suggested here: so it's not related to it). 58 pending...
(In reply to Adrian Florinescu [:AdrianSV] from comment #1) > Fadi R, do you know/could you please check if the problem reproduces with > using a FF60/FF58 wayland build? It would be most useful to figure if this > is a regression or not? > Meanwhile, moving this issue to Graphics, since IMO that would be a step > forward. > I've tested 60 off of central. Glitch is still there. Adrian, you can mark status-firefox60: --- → affected I've also tested the last stransky/fedora commit before firefox-wayland was merged with Mozilla upstream: https://codeload.github.com/stransky/gecko-dev/zip/143196035b95ad6e3f806291cb54f2205030e952 It was a very early version of FF 60.0a (alpha1). It ran as slow as molasses and my CPU temperature went up by 20oC at idle but the video glitch was no longer present. So it seems that you are correct, this issue is a regression. It must have been introduced somewhere along the line when ff-wayland's performance was improved. As far as FF 58 is concerned, from my understanding (and I could be wrong about this) there is no FF 58 wayland support in the main mozilla code. I can still test stransky's FF-wayland 58 but I don't see what that will tell us beyond what I've already found. Let me know if you need something else.
Based on comment 4, marking 60 as affected and as a regression. As for a follow up, I'm not that familiar with Wayland and what is the best/shortest path for following with investigation on this. Milan, could you advise on how to proceed further with this issue?
status-firefox60: ? → affected
Flags: needinfo?(milan)
Keywords: regression
Blocks: wayland
Whiteboard: [gfx-noted]
I don't know if this would help but the wasn't there when the merge with upstream happened on Jan 25. I can build a few of 59 betas until I see at which point the regression happened. If we know in which revision the bug happened, the people at gfx could use diff to find the most likely candidate.
Sorry, I was just re-reading the comment and that first sentence didn't make too much sense. I was just saying that the bug wasn't there on Jan 25 when stansky's code merged with upstream, so I can probably try to build a few of the betas that happened in that months to try to find the point at which the bug occurred.
(In reply to Fadi R from comment #7) > Sorry, I was just re-reading the comment and that first sentence didn't make > too much sense. I was just saying that the bug wasn't there on Jan 25 when > stansky's code merged with upstream, so I can probably try to build a few of > the betas that happened in that months to try to find the point at which the > bug occurred. If you could help find a regression range, that would be most helpful. Thanks.
No problem. By the way, I decided to build straight out of central. Tracing the problem will be a lot easier there sinece I know exactly what date stransky sent his commits upstream. I'll let you guys know.
I was positive that I've tried using a new profile when this whole thing started but I decided to give that another go. This time around, testing using a new profile, the glitch didn't persist. A bunch of testing later, I ended up tracking the glitch to a maybe corrupt content-prefs.sqlite. I'm going to test for a few days, if the glitch doesn't return, we can mark this as closed.
It's back. The glitching seems to happen when the youtube page is not at 100% zoom. All that deleting cont-prefs.sqlite does is reset the page zoom. When you zoom into 120% (that's the zoom I usually use firefox at) and hit refresh, the glitching comes back. When it's back at 100% (and you hit refresh), it goes away.
More testing to follow.
OK, I'm done testing and I have a few updates and corrections: ------------------------------------------------------------- - Today I found out that glitching occurs when youtube page is not at 100% zoom. Additionally, even if you have page at 100% zoom, full screen results in glitching. So does theater mode. - Since I didn't account for zoom when testing youtube videos, there's been a descrepency in my testing as some builds were tested at 100% and others were tested at 120% zoom. I had to go back and re-examine some previous conclusions: The first thing I had tested was: was this bug introduced by mozilla (a regression) or was it already present in the redhat/stransky code. I downloaded the last code to be committed to stransky's repo before it was retired, built it and ran youtube videos on it. I had initially tested this build at zoom 100% a week ago and found no glitches and had concluded that it was unaffected. Retesting that same build today at 120% zoom, I did find glitches at that zoom level. So my initial conclusion that this bug was a regression was false. Sorry guys. This bug seems to be native to the ff-wayland support. Perhaps Martin Stránský might have some more insight as to what could be causing this bug or at least where the people in gfx could start looking. I've attached an info request from him to this comment. If you need any more tests made, let me know.
Flags: needinfo?(stransky)
(In reply to Fadi R from comment #13) > OK, I'm done testing and I have a few updates and corrections: > ------------------------------------------------------------- > > - Today I found out that glitching occurs when youtube page is not at 100% > zoom. Additionally, even if you have page at 100% zoom, full screen results > in glitching. So does theater mode. > > - Since I didn't account for zoom when testing youtube videos, there's been > a descrepency in my testing as some builds were tested at 100% and others > were tested at 120% zoom. I had to go back and re-examine some previous > conclusions: > > The first thing I had tested was: was this bug introduced by mozilla (a > regression) or was it already present in the redhat/stransky code. I > downloaded the last code to be committed to stransky's repo before it was > retired, built it and ran youtube videos on it. I had initially tested this > build at zoom 100% a week ago and found no glitches and had concluded that > it was unaffected. Retesting that same build today at 120% zoom, I did find > glitches at that zoom level. So my initial conclusion that this bug was a > regression was false. Sorry guys. This bug seems to be native to the > ff-wayland support. > > Perhaps Martin Stránský might have some more insight as to what could be > causing this bug or at least where the people in gfx could start looking. > I've attached an info request from him to this comment. Yes I'm aware of that.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(stransky)
Summary: Firefox Wayland 59.0b13: UI elements on youtube flicker when playing videos → [Wayland] UI elements on youtube flicker when playing videos
Thanks Martin.
Too late to fix in 59 but we can still take a patch in Nightly.
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16) > Too late to fix in 59 but we can still take a patch in Nightly. Mozilla does not ship wayland enabled builds so this bug does not affect any product mozilla ships right now.
Clearing the regression flag as this is not a regression.
Keywords: regression
Flags: needinfo?(milan)
This is not an issue with EGL patches and Webrender enabled.
See Also: → 1467128
This is caused by direct (unbuffered) rendering to wayland surface where we don't clip the drawing.
Assignee: nobody → stransky
Component: Graphics → Widget: Gtk
Comment on attachment 8985568 [details] Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates, https://reviewboard.mozilla.org/r/251178/#review257454 ::: widget/gtk/WindowSurfaceWayland.h:87 (Diff revision 1) > } > bool IsMatchingSize(class WindowBackBuffer *aBuffer) > { > return aBuffer->mWidth == mWidth && aBuffer->mHeight == mHeight; > } > + gfx::IntSize GetSize() Hm, looks like this can be removed.
Comment on attachment 8985568 [details] Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates, https://reviewboard.mozilla.org/r/251178/#review257696 ::: widget/gtk/WindowSurfaceWayland.cpp:727 (Diff revision 1) > WindowSurfaceWayland::Lock(const LayoutDeviceIntRegion& aRegion) > { > MOZ_ASSERT(mIsMainThread == NS_IsMainThread()); > > - // We allocate back buffer to widget size but return only > - // portion requested by aRegion. > + LayoutDeviceIntRect screenRect = mWindow->GetBounds(); > + gfx::IntRect bounds = aRegion.GetBounds().ToUnknownRect(); We need to check numbers of invalid rects here, to make sure we're requested to draw entire screen.
Comment on attachment 8985568 [details] Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates, https://reviewboard.mozilla.org/r/251178/#review257702 ::: widget/gtk/WindowSurfaceWayland.h:123 (Diff revision 1) > + already_AddRefed<gfx::DrawTarget> LockFrontBuffer(int aWidth, int aHeight); > + already_AddRefed<gfx::DrawTarget> LockImageSurface(const gfx::IntSize& aLockSize); Hm, consider to change name to something else, because content of the methods does not lock anything, maybe GetDrawTargetForFrontBuffer and GetDrawTargetForImageSurface. ::: widget/gtk/WindowSurfaceWayland.h:136 (Diff revision 1) > WindowBackBuffer* mBackBuffer; > + RefPtr<gfxImageSurface> mImageSurface; > wl_callback* mFrameCallback; > wl_surface* mFrameCallbackSurface; > MessageLoop* mDisplayThreadMessageLoop; > + bool mFrontBufferDrawing; Please add brief explanation to where we're drawing when this is false (it's not a backbuffer but extra images surface because of drawing partial of the window) for example // if false, drawing to mImageSurface
Attachment #8985568 - Flags: review?(jhorak) → review+
Comment on attachment 8985568 [details] Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates, https://reviewboard.mozilla.org/r/251178/#review257702 > Please add brief explanation to where we're drawing when this is false (it's not a backbuffer but extra images surface because of drawing partial of the window) > > for example > // if false, drawing to mImageSurface I renamed it to mDirectWlBufferDraw to make it clear.
Comment on attachment 8985568 [details] Bug 1441743 - [Wayland] Don't draw directly to frame buffer for partial window updates, https://reviewboard.mozilla.org/r/251178/#review257702 > Hm, consider to change name to something else, because content of the methods does not lock anything, maybe GetDrawTargetForFrontBuffer and GetDrawTargetForImageSurface. I'd like to follow the compositor widget terminology here.
Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/6ac15a2e6914 [Wayland] Don't draw directly to frame buffer for partial window updates, r=jhorak
https://hg.mozilla.org/mozilla-central/rev/6ac15a2e6914
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62

I am running version 69 on Manjaro and am having this issue. Is it a regression?

(In reply to Scott Palmer from comment #31)

I am running version 69 on Manjaro and am having this issue. Is it a regression?

Yes, there's a regression which was fixed in Firefox 70. Please try latest nightly/beta or wait to next FF update.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: