-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid flash of background color when scrolling in safari #32747
Conversation
Size Change: -12 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
Thank you for so patiently improving the Safari experience. It will make a lot of people happy. In this case, I do appreciate seeing a white flash rather than the very dark flash present before. However it's still a bit of a bandaid, and introduces some other issues:
I suspect to land this we'll want to compare exactly which bits scroll and how, to make sure that's 1:1 — I know that's something Jay spent a lot of time getting just right. |
I noticed this while testing another PR, but in trunk with the same content, we see the background color flashing only in some themes. For example Armando vs Twenty Fourteen. Let me see if I can isolate that armando.mp42014.mp4 |
Apparently changing the font-size is enough to do it? fontsize.mp4So I tested this out at various font-sizes, and mapped where the background flash doesn't appear. The one thing I did notice is that the compositing layer for
To see compositing layers, click on the layers tab in safari devtools: |
Nice, I created a base case outside of Gutenberg see https://gist.github.com/gwwar/4d674b0ef9618fd0a5df0e25db969b19 I'll try whittling that down further but we can see what happens when the compositing layer is too large, we'll get a pink background bleed: scroll-bleed.mp4This doesn't happen when the compositing layer is smaller (say we decrease the font-size): scrolling.mp4 |
I couldn't seem to manage to replicate this on my laptop screen. One thing I recall from back in the day was that something like https://davidwalsh.name/translate3d Not sure if that works as I can't replicate but worth considering? |
This is wild. I'm going to dig in deeply on this and see what I can learn, if anything.
Thanks for looking, yes — translate hacks also including |
I'm super impressed that you managed to isolate this. But I'm afraid I can't replicate the tearing: Screen.Recording.2021-06-17.at.11.01.05.movThis is Safari 14.1.1 on an iMac. I'll try on a laptop now, but it seems regardless of font size or otherwise, I can't see it. But let's say you've isolated it, and it's related to compositing layer size — where do we go from there? As best I can get the next step is a webkit bug report and potentially #32575 as a bandaid? |
I tested on my macbook, and for the briefest moment I was able to see the flash of pink. It was only once, and it happened ever so briefly when scrolling past an image, but notably also scrolling down the page to where I hadn't been before. That last bit could be a coincidence, but mentioning in case there's some off-screen caching going on. |
If y'all check the layers tab what we're interested in is the content div layer getting larger in area. I suspect the trigger point is going to be different for different machines and what GPU they have. I'll see if I can try and force the compositing layer to be larger in the test case. This is also possible locally by using a monitor with a high resolution and maximizing the page. |
Do memory/gpu throttling tools exist that we could leverage? |
🤔 I'm not sure there are tools to simulate reduced GPU memory. I think I can come up with a test case using a bunch of large rectangles that should probably do it. The |
Ooh while refining the test case I think I uncovered something else. Ideally the compositing layer should only be the size of the scrollable pane. For example, just the visible bits we see in red: However in Gutenberg, there are other compositing layers that intersect, which might explain how this content layer got so large. I'll try to replicate this in the test base case. |
For the compositing layer size tangent I went ahead and wrote up my findings here: |
08a2bcb
to
febc370
Compare
I've updated the approach to move the background color to a different div that isn't a compositing layer. So this is ready for another round of reviews. Note this fixes the background flashing only, I plan on fixing the flickering text in #32824 |
I took this for a spin. The "tearing" of the background seems very much fixed for me, very nice! As seen in the GIF there's still the flickering with the fixed position block toolbar, but you're aware of that (#32824). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well for me, fixes the tearing, still shows the dark gray background in preview and template editing modes, and is a mighty fine small fix. It would be nice with a small look by @ellatrix if she has time, otherwise let's land this. Very nice work!
Looks like Android E2Es are also broken in trunk from "adb: no devices/emulators found", so the red run there is unrelated to this change. |
In unrelated testing today I noticed a dark scroll bar in the main canvas in Firefox, in trunk. I believe this PR might fix that as well. |
I'll merge once I can get the other E2Es green. Will try rebasing with trunk |
…ement that is not a compositing layer
febc370
to
d939812
Compare
Thanks for the reviews @jasmussen @getdave ! |
Is this something we want to include in WP 5.8? |
@youknowriad Sure, if the the window is still open for that 👍 I think this should be relatively safe to try. |
…ement that is not a compositing layer (#32747)
…ement that is not a compositing layer (#32747)
I've filed a bug in webkit https://bugs.webkit.org/show_bug.cgi?id=227532 and added a much smaller test case here. I think there's a combination of needing a fast repaint rate (which is easier to trigger with a mousewheel) plus a large compositing layer to slow down the speed of repaints. https://codepen.io/gwwar/pen/BaRawoz or https://gist.github.com/gwwar/0bbe359ea9830cb959774f6ee6e94b75 It should look like this if folks manage to trigger the glitch: scrolling.mp4If folks have trouble seeing the pink, try triggering a lot of scrolls in dev console like: for ( i =0; i<100000; i++) { document.querySelector('.scrollable').scroll( { top: i } ); } And then trigger a scroll from the user (via touchpad or mouse). devconsole.mp4 |
I also isolated the scrollbar drawing that layer instead of drawing the scrollbar control in: It should look like this with the bug: scrollbarcolor.mp4
This one also requires scrollbars to be shown: scrolltoalways.mp4Filed this one to: https://bugs.webkit.org/show_bug.cgi?id=227545 |
In Safari when we scroll we can see a flash of background color that's set on
.edit-post-layout .interface-interface-skeleton__content
. I set it to pink in the before video to make this more apparent. This background color is intended to be used while in device preview/template mode.Changes in this PR move this background color rule to a different div that is not a compositing layer. It looks like there may be a Safari browser bug around ordering (or general slowness) with mixing hardware accelerated elements and non-accelerated elements.
This fix targets the background flashing behavior only in Safari. There's a separate exploratory PR to remove the text flashing issues in #32824
Extra Debugging Notes:
Before:
beforeinpink.mp4
After:
noflash.mp4
Testing Instructions