Closed Bug 1376538 Opened 2 years ago Closed 2 years ago
APZ: Transmit focus updates more frequently
Async keyboard scrolling can only happen when APZ has the current focus target. We currently transmit a focus update in nsDisplayList::PaintRoot, but we should be able to transmit more frequently than that. (see Botond's comment in bug 1351783 comment 168)
The description of this bug made me realize that I actually made a mistake in bug 1351783 comment 168: (In reply to Botond Ballo [:botond] from comment #168) > (In reply to Botond Ballo [:botond] from comment #55) > > > +Bug 1351783 part 10 - Create and sync the current FocusTarget on each layer transaction r?kats > > > + > > > +This commit modifies PresShell and nsDisplayList to send a FocusTarget update on > > > +every layer transaction. Ideally we would like to send updates as often as possible, > > > +but this seems like it works well. This can be iterated on later, if necessary. > > > > As a lead for future iteration on this: look into "empty transactions" (bug > > 1257641 is a good place to look), and make sure a focus update is sent with > > empty transactions too. > > It looks like empty transactions still go through PresShell::Paint(), so > they will already pick up the focus update with the code as written. The focus update is sent in nsDisplayList::PaintRoot(), not PresShell::Paint(), is nsDisplayList::PaintRoot() is *not* called for empty transactions (because for empty transactions we early-exit PresShell::Paint() here  or here , while PaintRoot() is called from PaintFrame() ). So we will need two code changes in this bug: 1) Sending the focus target as part of an empty transaction 2) Scheduling an empty transaction when the focus target changes  http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6356  http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6401  http://searchfox.org/mozilla-central/rev/b425854d9bbd49d5caf9baef3686e49ec91c17ec/layout/base/PresShell.cpp#6426
I don't really know why we were calling layerManager->SetFocusTarget in nsDisplayList. There isn't any reason for it anymore (there was before), so this patch fixes that, which simplifies some things. This patch then requests an empty transaction whenever we get a new focus sequence number so that we get focus updates quicker. This can potentially happen frequently, so it's important that we don't spam IPC with updates. I'm told by Botond that using an empty transaction could be a good fit for this.
Attachment #8882001 - Flags: review?(bugmail)
Comment on attachment 8882001 [details] [diff] [review] focus-send.patch Review of attachment 8882001 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Might be good to amend the commit message to specifically mention empty transactions.
Attachment #8882001 - Flags: review?(bugmail) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec5e890bcbd Transmit APZ focus target updates on empty transactions. r=kats
Priority: -- → P3
2 years ago
No longer depends on: 1378006
You need to log in before you can comment on or make changes to this bug.