Closed
Bug 1241332
Opened 8 years ago
Closed 8 years ago
Panning and zooming an input field when it gains focus has too much latency in Fennec
Categories
(Core :: Panning and Zooming, defect)
Core
Panning and Zooming
Tracking
()
RESOLVED
FIXED
mozilla47
People
(Reporter: rbarker, Assigned: rbarker)
References
(Depends on 1 open bug)
Details
(Keywords: polish, Whiteboard: [gfx-noted])
Attachments
(2 files, 7 obsolete files)
1.24 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
2.88 KB,
patch
|
Details | Diff | Splinter Review |
Fennec uses the APZC function ZoomToRect to ensure a focused input element is visible in the window (see bug 1231517). Currently it is using a timeout to wait for the window resize to propagate to the compositor thread. The attached patch was an attempt to flush APZ to ensure the updated frame metrics had propagated to the compositor thread. Unfortunately, it would intermittently fail. Examining the frame metrics in the APZC revealed the composition bounds had not yet been updated when the call failed.
Updated•8 years ago
|
Assignee: nobody → rbarker
Updated•8 years ago
|
Blocks: fennec-aboard-apz
Comment 1•8 years ago
|
||
Based on what we do in testing code I think what needs to happen here is that you need to wait for (1) the resize event, (2) any pending paints, and (3) the apz-flush. Once all three have completed the APZC should have the updated composition bounds. And actually if the zoomToRect call goes via the compositor thread then you don't need to wait for (3) because it will implicitly follow (2).
Keywords: polish
Whiteboard: [gfx-noted]
Comment 2•8 years ago
|
||
I tested this out today on my 6P, Nightly. The delay between A) user touches input field and B) input field is fully zoomed into view is really long right now. I think we need to fix this sooner rather than later because it feels kind of broken to have the feedback for a basic touch be so lagged behind. :)
Flags: needinfo?(rbarker)
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #2) > I tested this out today on my 6P, Nightly. > > The delay between A) user touches input field and B) input field is fully > zoomed into view is really long right now. I think we need to fix this > sooner rather than later because it feels kind of broken to have the > feedback for a basic touch be so lagged behind. :) I will attempt the fix outlined in comment #1 when I get a chance. Right now the feature works (not ideally) and we have a number features that are busted that need more urgent attention.
Flags: needinfo?(rbarker)
Assignee | ||
Updated•8 years ago
|
Summary: calling nsDomWindowUtils.flushApzRepaints() after a window resize does not guarantee the frame metrics will have finished propagating in C++APZ → Panning and zooming a input field when it gains focus has too much latency in Fennec
Assignee | ||
Updated•8 years ago
|
Summary: Panning and zooming a input field when it gains focus has too much latency in Fennec → Panning and zooming an input field when it gains focus has too much latency in Fennec
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8710187 -
Attachment is obsolete: true
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #8726421 -
Attachment is obsolete: true
Assignee | ||
Comment 7•8 years ago
|
||
Attachment #8726422 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8726423 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8726424 -
Attachment is obsolete: true
Updated•8 years ago
|
Attachment #8726423 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8726428 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8726498 -
Flags: review?(bugmail.mozilla)
Comment 10•8 years ago
|
||
Comment on attachment 8726498 [details] [diff] [review] 0002-Bug-1241332-part-2-Request-zoomToFocusedInput-after-resize-finishes-and-propagates-through-APZ-r-16030316-1e25225.patch Review of attachment 8726498 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/browser.js @@ +1564,5 @@ > + }; > + > + let apzFlushDone = function() { > + Services.obs.removeObserver(apzFlushDone, "apz-repaints-flushed", false); > + setTimeout(focusInput, 100); What's this setTimeout for? Can you just call dwu.zoomToFocusedInput() directly?
Assignee | ||
Comment 11•8 years ago
|
||
I think that 0 timeout was left over from the original patch when I was trying to figure out how to ensure the resize had completely propagated.
Attachment #8726498 -
Attachment is obsolete: true
Attachment #8726498 -
Flags: review?(bugmail.mozilla)
Attachment #8726768 -
Flags: review?(bugmail.mozilla)
Comment 12•8 years ago
|
||
Comment on attachment 8726768 [details] [diff] [review] 0002-Bug-1241332-part-2-Request-zoomToFocusedInput-after-resize-finishes-and-propagates-through-APZ-r-16030407-bf01cba.patch Review of attachment 8726768 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/chrome/content/browser.js @@ +1582,5 @@ > + paintDone(); > + } > + } > + > + let gotResizeWindow = false; I'd prefer moving this declaration up to before the resizeWindow declaration.
Attachment #8726768 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 13•8 years ago
|
||
Address, comments. Carry forward r+ from :kats
Attachment #8726768 -
Attachment is obsolete: true
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/409f7a0e990f https://hg.mozilla.org/integration/mozilla-inbound/rev/953545615bdc
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/409f7a0e990f https://hg.mozilla.org/mozilla-central/rev/953545615bdc
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•