Per discussion in bug 1312103 I'm filing this bug to track disabling async scrolling in some scenarios. Specifically, we have various pieces of code (the touch carets, the new find-in-page code, and devtools highlighters) that all use anonymous content to "overlay" some stuff onto the content document. In some/all of these cases, because of async scrolling, the anonymous content jitters while the user is scrolling. We can minimize this effect by ensuring that the anonymous content elements are position:absolute, which will ensure that if APZ is used to scroll the document's root scrollframe it will behave fine. This is already the case for the touch carets, for example. However even in that case, putting touch carets inside an overflow:scroll element and then scrolling that element will result in the laggy updates/jitter, because the position:absolute is relative to the containing document and not the overflow:scroll element. (e.g. on Fennec if you go to https://people-mozilla.org/~kgupta/griddiv.html, long-press inside the grid to bring up selection carets, and then scroll it). We should figure out the minimal set of cases in which we need to disable async scrolling to eliminate this jittery behaviour. In bug 1312103 the devtools team agreed to try using position:absolute which will help some. However they will only have anonymous content on the root content document, not iframes (unlike the section carets, which go on the innermost content document). I'm not sure what the deal is with the new find-in-page, but that seems to jitter even scrolling the root content document and would presumably benefit from this change as well. In terms of actual code change, I believe the change should be pretty straightforward, localized to the nsLayoutUtils::ShouldDisableApzForElement function. In there we just need to determine if the content in question should have async scrolling based on whether or not it is the root scrolling document and/or the containing document has anonymous contents and/or the containing root content document has anonymous contents.

The carets issue described in the second paragraph of comment 0 is exactly bug 1273045. I'll be very happy to this bug fixed!

Thanks for filing this, Kartikaya!

Is the issue related to the top layer which renders the overlay? I think top layer now supports position: absolute, doesn't it?

Not really sure what you are asking. The anonymous content elements, which are in the "top layer" (IIUC) can be position:absolute, but that's relative to the document. If they are intended to be overlaid on something inside a scrollable div making them position:absolute doesn't help them stay in sync when the div is scrolled.

Oh, okay, I now understand what the issue is, and it seems to me this is hard to fix with async scrolling... Although it is probably not very hard to make top layer frame move relative to other elements, it may be nontrivial to have it painted with correct transform while keeping it at the topmost position in rendering order. So it seems disabling async scrolling in those cases is a reasonable solution for now.

Is there any update on this…?

Not in particular. I was waiting for bug 1312103 to be fixed before I tackled this.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7) > Not in particular. I was waiting for bug 1312103 to be fixed before I > tackled this. The patch in that bug is not going to help for the findbar highlighting, since it's implementing the optimizations the findbar code is already using.

Hi Kartikaya, bug 1312103 has landed; let me know when you can work on this. Also, there could be another scenario that is worth to mention, maybe you have some ideas how to fix this. As per bug 1312103, we're using position absolute where is possible to minimize the jitters; however it's not always possible: in some cases, especially the one where we're using <canvas> for our tools, we have to set the position to fixed and draw the content based on the scrolling. In that case, we still have issues. On the Grid Inspector we tried to have the canvas positioned absolutely but it doesn't work on pages that are bigger than 32767 pixels (see bug 1343217). So we could probably fall back to have position fixed here too, but that means we'll got back the jitters as well since the patch for bug 1312103 won't help. I'm open to any suggestion to mitigate the jitters in that case, otherwise I was wondering if we could have a way to disable the APZ by some API also for the root element, and use it only when we're dealing with <canvas> and position fixed.

(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #9) > Hi Kartikaya, bug 1312103 has landed; let me know when you can work on this. I'll look at it this week. Leaving needinfo on me so I don't forget. > I'm open to any suggestion to mitigate the jitters in that case, The only thing I can think of right now is keeping the canvas position:absolute but sizing the canvas a little larger than the visible area (we can expose the displayport size via some JS API if that helps - bug 1232491). Then as the user scrolls you shift the canvas' position and content simultaneously. It's basically the same as the "virtual list" idea, but applied to canvas. That is, you always make sure the visible part of the canvas is part of the DOM and the part that's way offscreen is not part of the DOM. > otherwise I > was wondering if we could have a way to disable the APZ by some API also for > the root element, and use it only when we're dealing with <canvas> and > position fixed. I would really like to avoid falling back to janky scrolling if at all possible.

Here's a WIP, needs more testing.

So the v2 WIP I posted, in combination with the patch I posted to bug 1273045 seems to at least fix the case on Fennec where the carets are in a subframe and the user scrolls the subframe. Without the patch in bug 1273045 the carets don't reposition, and the content also doesn't actually repaint. The content not repainting is probably a separate bug. I'll test the patch more on desktop and such.

Comment on attachment 8845954 [details] [diff] [review] WIP v2 (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #13) > The content not repainting is probably a separate bug. The problem here was that the paint-skipping code wasn't taking into account elements for which we had force-disabled APZ. I'll fix that as well.

So I have patches that seem to reliably disable APZ when there is anonymous content inserted into the root content document. With the patches on bug 1273045 it seems to work well for the accessible caret. I also tested the new find-in-page [1] and the CSS grid highlighter [2] in subframes, and both of those behave much better (less laggy) with these patches - but they are still a little laggy behind the scrolling. I'm not entirely sure why - Matteo/Mike, can you point me to the code that the devtools/find-in-page features use to reposition their content when a subframe scrolls? I'm wondering if it's a front-end issue or a platform issue. [1] Load https://people-mozilla.org/~kgupta/griddiv.html, ctrl-f search for "100", then scroll the subframe [2] Load https://people-mozilla.org/~kgupta/bug/1316318/cssgrid.html, inspect the .wrapper div, and turn on the grid highlighter, then scroll the subframe

I'll put the patches up anyway to get some review feedback and also in case anybody wants to try them out.

Comment on attachment 8846712 [details] Bug 1316318 - Disable APZ scrolling in subframes if the root document has visible anonymous content. https://reviewboard.mozilla.org/r/119716/#review121808 ::: layout/base/nsLayoutUtils.cpp:1191 (Diff revision 1) > + if (rootShell) { > + if (nsIDocument* rootDoc = rootShell->GetDocument()) { > + nsIContent* rootContent = rootShell->GetRootScrollFrame() > + ? rootShell->GetRootScrollFrame()->GetContent() > + : rootDoc->GetDocumentElement(); > + // For the selection carets: disable APZ on any scrollable subframes that Nit: s/selection carets/AccessibleCaret/, please. Selection carets was deprecated long time ago.

Comment on attachment 8846712 [details] Bug 1316318 - Disable APZ scrolling in subframes if the root document has visible anonymous content. https://reviewboard.mozilla.org/r/119716/#review122120

Comment on attachment 8846713 [details] Bug 1316318 - Disable paint-skipping for elements where we disable APZ. https://reviewboard.mozilla.org/r/119718/#review122122

For comment 15

Landing these patches anyway since they fix the AccessibleCaret issue perfectly. If additional work is needed we can do it in a follow-up bug.

Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c20e214d6486 Disable APZ scrolling in subframes if the root document has visible anonymous content. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/0e09b0ea4873 Disable paint-skipping for elements where we disable APZ. r=mstange

https://hg.mozilla.org/mozilla-central/rev/c20e214d6486 https://hg.mozilla.org/mozilla-central/rev/0e09b0ea4873

Sorry for the late response kats! It slipped under the radar. (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15) > So I have patches that seem to reliably disable APZ when there is anonymous > content inserted into the root content document. With the patches on bug > 1273045 it seems to work well for the accessible caret. I also tested the > new find-in-page [1] and the CSS grid highlighter [2] in subframes, and both > of those behave much better (less laggy) with these patches - but they are > still a little laggy behind the scrolling. I'm not entirely sure why - > Matteo/Mike, can you point me to the code that the devtools/find-in-page > features use to reposition their content when a subframe scrolls? I'm > wondering if it's a front-end issue or a platform issue. So, for the grid highlighter it seems we still have big issues (see the attachment) [1]. How we position the grid inspector for scrolled elements is basically the same for any highlighters, for the grid is a bit more complex but you should have the same issue with regular inspector as well (at least, I have). Basically we get the "adjusted" quads for the element inspected (https://dxr.mozilla.org/mozilla-central/source/devtools/shared/layout/utils.js#191) then we compared with the previous ones (https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/highlighters/auto-refresh.js#188) if it has moved we update the highlighter position. This check happens during a requestAnimationFrame callback. Let me know if I you need more info about it. [1] The page is: zer0.github.io/devtools/highlighters-test-page.html Notice that here we've nested scrollable elements / frames with fixed position; so it's a sort of stress test.

I finally got around to looking at your test page again. I'm pretty sure that the problem you're seeing on that page is because the grid in a fixed-position element. Note that if you scroll the "playground" panel, the highlighter remains in the right place. This happens because APZ is disabled on that scrollable element. It's only when you scroll the document body/root scrollframe that the jitter happens. That's because APZ is still enabled on the root scrollframe of documents with anonymous content. I did that intentionally because normally when you scroll the root scrollframe, all subframes will scroll along with it, and so will the highlighter. So everything will stay in the right spot relative to everything else. I didn't consider this case, where you have a highlighter that's supposed to stay stuck to a fixed-position scrollframe. The only way I can see out out of this is to disable APZ on the root scrollframe as well, which I don't really want to do. I'm not convinced that this scenario will occur very often in the wild (where you a scrollable subframe inside a fixed-pos layer, and then you have anonymous content over top that needs to stay in sync with the content of the scrollable subframe). If it turns out this is a frequently occurring scenario I'm happy to reconsider.