| Summary: | [GTK] Tearing when entering AC mode | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | agomez, berto, bugs-noreply, cgarcia, commit-queue, gustavo, mcatanzaro, mrobinson, yoon | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | Other | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=155446 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Michael Catanzaro
2015-11-05 14:53:01 PST
Created attachment 264887 [details]
Screenshot #2
Created attachment 264889 [details]
Screenshot #3
It's not 100% reproducible; it happens perhaps 50% of the time.
Wow, there is some weird issue with these screenshots. They display with a bunch of transparency in WebKit, but what I saw on my screen was a bunch of black. You have to open the screenshots in *sushi*, which replaces all the transparency with black, in order to see the tearing that I regularly see. The black is what I really see when opening these pages. I can't reproduce it. I think could have fixed this in r192052, since we were returning early from DrawingAreaProxyImpl::willEnterAcceleratedCompositingMode incorrectly in some cases. (In reply to comment #4) > I can't reproduce it. I think could have fixed this in r192052, since we > were returning early from > DrawingAreaProxyImpl::willEnterAcceleratedCompositingMode incorrectly in > some cases. Unfortunately no, it's still broken in 2.10.4, which includes that commit. Still broken in 2.10.7. Can you still reproduce this in trunk? Yes, on the first try, with the reproducer in comment #0. A new web view is a requirement to reproduce this, I don't think it ever happens for existing web views. I'm doing exactly what first comments says, opening https://github.com/WebKit/webkit/commits/master, middle clicking all the commits and then visiting the new tabs. I've never been able to reproduce this. Yoon, any chance you are able to reproduce this? (In reply to comment #0) > Created attachment 264886 [details] > Example screenshot > > Even with 2.10.3, we are still getting bad tearing when entering AC mode. To > reproduce, open https://github.com/WebKit/webkit/commits/master in one tab > and then middle-click on a bunch of commit links to open them in new tabs. A > bunch of the new tabs will be screwed up until you scroll the page. Once the > first scroll event occurs, the pages fix themselves. Thanks for reporting this, Michael. I've been suffering this very often and with many web pages in the last releases. I can confirm in 2.10.7 and will pay closer attention from now on. I didn't report sooner because I've been seeing "similar problems" for a long, long time and with several series of WKGTK and I didn't know how to describe/report/check if the problem was in WKGTK or my general desktop. Another "similar" issue that I'm seeing is that, when swapping between tabs, the new tab makes a flickering tearing a "screenshot" from something that has been shown in my display in a very different moment and with a very different application. As commented, I hit similar problems while browsing but we wanted to check what would happen when forcing AC compositing. Therefore, we can do that by launching epiphany like: $ WEBKIT_FORCE_COMPOSITING_MODE=1 epipahny So, steps that I followed: 1. Launch a new and fresh epiphany instance 2. Open this bug directly from the location bar 3. Open in a background tab the link https://github.com/WebKit/webkit/commits/master by using the secondary button of the mouse and clicking in the link in comment 0. 4. Move to that new tab The outcome when forcing or not forcing the compositing mode is quite different. Without forcing, there is nothing remarkable to comment. With forcing: 1. We experience some tearing straight away after launching epiphany, upon the creation of the window. It is a quick flickering that is promptly redrawn by epiphany's content. 2. On moving to the second tab opened in the background we experience again some tearing. Its severity changes from running session to running session. Sometimes is just a quick flickering but, others, we can see quickly a complete screenshot from some other previously opened epiphany tab or other application. Also, sometimes, the tearing is complete garbage that it is not redrawn until we force it by scrolling in the new tab. I'm trying hard to reproduce this with no success at all. The thing is that here it never enters AC mode when visiting the github pages. (In reply to comment #13) > I'm trying hard to reproduce this with no success at all. The thing is that > here it never enters AC mode when visiting the github pages. Really? GitHub has used AC at for me since at least 2.6.x. Not every page, but the commits history page, always. Any luck using WEBKIT_FORCE_COMPOSITING_MODE=1? Thanks for trying to reproduce. Worst-case, I can leave you with my OLPI laptop next time I'm in Spain. But maybe it's a Fedora-specific issue? Andres, are you using Ubuntu or Fedora? I've seen this bug on three out of the three computers I tested on (all using Fedora). Maybe I should try reproducing on Debian testing, to maybe rule that out. WEBKIT_FORCE_COMPOSITING_MODE doesn't help here, because things happen differently when AC is forced, there's more tearing as Andrés reported, but they are for different issues that I'm already working on. But in this case the problem is when switching from non AC to AC mode. By the way, when I say 2.6.x, I mean I think this issue did not exist in 2.6.0, but was introduced in some commit backported to the 2.6 branch. If you think it would help, I could try building old releases to see? No, I know that this problem started when we moved the redirected window creation to the realize method. When we enter AC mode we keep rendering the non AC contents until we have an AC frame in the UI process. When you open a new tab, the view is not realized until it's mapped when you visit the tab. At that point, we still don't have an AC frame in the UI process, and we didn't render any non AC contents yet to show until we receive the frame. Sometimes, we receive an update from the web process with just the background filled white, but sometimes we don't (or it's transparent if the website uses transparent bg), and we end up rendering an uninitialized surface for a while. What I don't understand is why you are not receiving the damage event once we have an AC frame to repaint the view. In that case, you still would see a flicker effect, but the AC contents will be rendering eventually. But instead, we need to scroll to receive the update. Created attachment 272003 [details]
Patch
I hope this patch fixes the issue because I haven't been able to reproduce it. What it should fix for sure is the flickering I've seen sometimes when visiting a tab with AC enabled for the first time.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Well the good news is, I don't see any screen tearing with this patch, but... (In reply to comment #8) > Yes, on the first try, with the reproducer in comment #0. A new web view is > a requirement to reproduce this, I don't think it ever happens for existing > web views. ...a week and a half later, I can no longer reproduce this with latest trunk inside GNOME jhbuild, with or without your patch. I... guess that's good? Except I don't think we changed anything relevant here. I suspect it's just a fluke, even though I tried many times today, or it could be because of some change in some dependency in GNOME jhbuild.... It's still happening with 2.10.7 in F23... maybe I'll try building 2.10.7 in jhbuild to confirm it's not due to any change in WebKit.... (In reply to comment #20) > Well the good news is, I don't see any screen tearing with this patch, but... > > (In reply to comment #8) > > Yes, on the first try, with the reproducer in comment #0. A new web view is > > a requirement to reproduce this, I don't think it ever happens for existing > > web views. > > ...a week and a half later, I can no longer reproduce this with latest trunk > inside GNOME jhbuild, with or without your patch. I... guess that's good? > Except I don't think we changed anything relevant here. I suspect it's just > a fluke, even though I tried many times today, or it could be because of > some change in some dependency in GNOME jhbuild.... > > It's still happening with 2.10.7 in F23... maybe I'll try building 2.10.7 in > jhbuild to confirm it's not due to any change in WebKit.... I would help a lot if you could test this patch applied on top of 2.10.7, because I'm currently preparing a new 2.10 release, and I would love to fix this bug for the next release. OK, the tearing does not occur with Epiphany 3.18.3, WebKitGTK+ 2.10.6, built in the latest GNOME JHBuild. So I am not sure why the tearing has gone away, but it is definitely not due to any change on our end. Next, I will build a 2.10.7 Fedora package with your patch applied to see if that makes a difference, as I am still able to reproduce the tearing outside of JHBuild. (In reply to comment #21) > I would help a lot if you could test this patch applied on top of 2.10.7, > because I'm currently preparing a new 2.10 release, and I would love to fix > this bug for the next release. Consider waiting for Yoon's patch in the cairo matrix bug. (In reply to comment #22) > OK, the tearing does not occur with Epiphany 3.18.3, WebKitGTK+ 2.10.6, > built in the latest GNOME JHBuild. So I am not sure why the tearing has gone > away, but it is definitely not due to any change on our end. But does it happen with the fedora package? Maybe it's a change in github that doesn't use AC anymore. > Next, I will build a 2.10.7 Fedora package with your patch applied to see if > that makes a difference, as I am still able to reproduce the tearing outside > of JHBuild. Ah, weird, any downstream patch? maybe the user agent is making github behave differently? > (In reply to comment #21) > > I would help a lot if you could test this patch applied on top of 2.10.7, > > because I'm currently preparing a new 2.10 release, and I would love to fix > > this bug for the next release. > > Consider waiting for Yoon's patch in the cairo matrix bug. Yes, indeed I'm waiting for that one too, the one currently submitted didn't work unfortunately :-( (In reply to comment #23) > But does it happen with the fedora package? Maybe it's a change in github > that doesn't use AC anymore. To be clear, the tearing still occurs with webkitgtk4-2.10.7-1.fc23. I am now going to build a test package now with your patch applied that should otherwise be identical to the Fedora package. > Ah, weird, any downstream patch? maybe the user agent is making github > behave differently? I don't think so, it was occurring in my JHBuild environment just a week and a half ago. But yes, we have two downstream patches remaining, one for the Fedora user agent (still meaning to update that patch), one to make YouTube videos work. > > (In reply to comment #21) > Yes, indeed I'm waiting for that one too, the one currently submitted didn't > work unfortunately :-( Alas (In reply to comment #24) > To be clear, the tearing still occurs with webkitgtk4-2.10.7-1.fc23. I am > now going to build a test package now with your patch applied that should > otherwise be identical to the Fedora package. I don't think I'll get to this today, it's not a simple commit to backport. Do you have instructions anywhere for checking out a stable branch with git? I think it would be easier to backport the patch with git than with editing the patch manually, but I've never understood how to use git-svn and I'd rather not spend time learning it just to get at your stable branches. Comment on attachment 272003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272003&action=review Great changelog entry, I mostly understand what you are doing! > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1098 > + if (widgetIsOnscreenToplevelWindow(oldToplevel) && GTK_WINDOW(oldToplevel) == priv->toplevelOnScreenWindow) { Surely GTK_WINDOW(oldToplevel) == oldToplevel so you don't need this cast? > Source/WebKit2/UIProcess/cairo/BackingStoreCairo.cpp:-71 > - gtk_widget_realize(viewWidget); OK, I tested and there are no regressions related to bug #147453. (In reply to comment #26) > Comment on attachment 272003 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272003&action=review > > Great changelog entry, I mostly understand what you are doing! > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1098 > > + if (widgetIsOnscreenToplevelWindow(oldToplevel) && GTK_WINDOW(oldToplevel) == priv->toplevelOnScreenWindow) { > > Surely GTK_WINDOW(oldToplevel) == oldToplevel so you don't need this cast? It's not needed in C, but this is C++ and the compiler doesn't like comparing GtkWidget and GtkWindow pointers. > > Source/WebKit2/UIProcess/cairo/BackingStoreCairo.cpp:-71 > > - gtk_widget_realize(viewWidget); > > OK, I tested and there are no regressions related to bug #147453. Great, thanks (In reply to comment #25) > (In reply to comment #24) > > To be clear, the tearing still occurs with webkitgtk4-2.10.7-1.fc23. I am > > now going to build a test package now with your patch applied that should > > otherwise be identical to the Fedora package. > > I don't think I'll get to this today, it's not a simple commit to backport. > > Do you have instructions anywhere for checking out a stable branch with git? > I think it would be easier to backport the patch with git than with editing > the patch manually, but I've never understood how to use git-svn and I'd > rather not spend time learning it just to get at your stable branches. The instructions are in the wiki page, but anyway, I'll backport the patch to the branch and submit the patches here (I think I'll need to backport the one that reduces view state IPC traffic too). Created attachment 272111 [details]
Patched for 2.10
I had to also backport the view state patch, and fix some conflicts. This contains the two patches without changelogs (to avoid conflicts) to be applied in 2.10
Thanks, I will test this patch then. I did check the wiki looking for instructions, but didn't find any. (In reply to comment #30) > Thanks, I will test this patch then. > > I did check the wiki looking for instructions, but didn't find any. How to add a webkit-2.12 branch to existing git-svn clone If you have already cloned git://git.webkit.org/WebKit.git, it only contains a git-svn clone of the svn trunk. To add webkit-2.12 branch to there, add a new remote to .git/config: [svn-remote "webkit-2.12"] url = http://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.12 fetch = :refs/remotes/git-svn-webkit-2.12 and run the following commands: git svn fetch webkit-2.12 -r 196806 git branch webkit-2.12 git-svn-webkit-2.12 git checkout webkit-2.12 git svn rebase copy pasted from the wiki (https://trac.webkit.org/wiki/WebKitGTK/2.12.x) Comment on attachment 272003 [details]
Patch
It works!
:D
I figure nobody will use this, but RPMs with the fix here: http://koji.fedoraproject.org/koji/taskinfo?taskID=13118326 I won't start a Fedora update for this, but rather wait for your 2.10.8 release. (In reply to comment #33) > I figure nobody will use this, but RPMs with the fix here: > http://koji.fedoraproject.org/koji/taskinfo?taskID=13118326 > > I won't start a Fedora update for this, but rather wait for your 2.10.8 > release. I'm now only waiting for bug #154283 to make 2.10.8 Committed r197062: <http://trac.webkit.org/changeset/197062> |