mozilla :: #apz

17 May 2017
18:05botond|wfhkats: ping
18:06katsbotond: pon
18:06botond|wfhkats: i'm doing a bit of code archeology and came across
18:06firebotBug 1192919 FIXED, Need to schedule paints for scrolling inside inactive layers
18:07botond|wfhkats: the second paragraph there says, "The other main job of the TaskThrottler is to compute the estimated paint duration to use as part of the displayport heuristic. Botond and I talked about it and we think it makes sense to just disable this heuristic for now, and reimplement it later using a different method (passing around repaint request sequence numbers) if it will help."
18:07botond|wfhkats: do you, by any chance, recall why we didn't like that heuristic, and what the alternative method we had in mind was?
18:08katsbotond: i don't know if we discussed that explicitly, but i recall that the paint duration numbers were quite noisy
18:08katsand so it wouldn't give nice/linear responses in terms of repaint intervals
18:09katsi'm not sure what you mean by alternative method. do you mean alternative heuristic? or alternative way to reimplement the paint duration?
18:10botond|wfhkats: the alternative way to reimplement the paint duration heuristic, using repaint request sequence numbers
18:11katsbotond: i imagine we would have a sequence number attached to each repaint request, and that sequence number would be propagated through the paint back to APZ in NotifyLayersUpdated
18:11katsbotond: and then we could keep a map associating sequence number with start time
18:11katsor even just use the timestamp as the sequence number
18:11botond|wfhkats: ah. whereas previously, we would just assume the paint was caused by the most recent repaint request?
18:12botond|wfhkats: and thus paints caused by other things contributed to the noise?
18:12katsyeah i believe so
18:13katsi feel like we might have had some matching code to try and check that the paint corresponded to the repaint request but maybe that was in the java code for fennec
18:13katsi forget now
18:13botond|wfhkats: ok, that makes sense, thanks
18:13botond|wfhkats: i wonder if we should resurrect this heuristic using the sequence number strategy
18:14katsbotond: are there particular scenarios you think would benefit from this heuristic?
18:14botond|wfhkats: i feel like we could checkerboard less - possibly significantly less, though that's just a hunch - during scrollbar drags if we had better displayport predictions
18:15botond|wfhkats: like, suppose we can do a main thread paint + layer transaction in the space of 2 compositor frames
18:15katsbotond: it's possible
18:16botond|wfhkats: if we had perfect displayport prediction, we should be able to show content at least every other frame during even a fast drag
18:16katsbotond: the thing with scrollbar drag though is that velocity is user-controlled
18:16botond|wfhkats: but what i'm seeing is we basically checkerboard throughout
18:16katsit's not like an animation where we know what it will be ahead of time
18:17katsif we assume a constant drag velocity, then yeah, we would be able to do better
18:17katsi'm not sure if that assumption is valid
18:18katsmaybe hack up some code to report the drag velocity and run with it, and see how much variance there is in the velocity from one frame to the next?
18:18katsthat might give us an idea of "real world" velocity variance during scrollbar dragging
18:18botond|wfhkats: yeah, that's true. i guess in reality the drag velocity probably varies too much for predictions to be useful?
19:40rhuntbotond or kats: ping
19:40botond|wfhrhunt: pong
19:42rhuntcould either of you give me an overview of how async scrolling of a APZC is triggered when an input event is dispatched to it? after inputqueue and all that?
19:42rhuntI have the focus state transferring to the APZCTM along with the code for finding the associated APZC working, and I'm looking into getting something actually scrolling
19:43botond|wfhrhunt: sure, i can do that
19:44botond|wfhrhunt: so events the InputQueue eventually get processed by having InputBlockState::DispatchEvent(event) called
19:44botond|wfhor i guess that's CancelableBlockState::DispatchEvent
19:45botond|wfhrhunt: by that time, the input block has a confirmed target apzc
19:45botond|wfhstored in InputBlockState::mTargetApzc
19:45botond|wfhrhunt: and the evnet is passed to that APZC's HandleInputEvent() method
19:46botond|wfhrhunt: there, each event type is handled separately
19:46botond|wfhrhunt: let's take a mousewheel event as an example. that gets into AsyncPanZoomController::OnScrollWheel(event)
19:47botond|wfhrhunt: it's worth noting at this point that there are two ways APZC scrolls in response to events
19:47botond|wfhrhunt: one is scrolling right away in response to the event
19:48botond|wfhrhunt: the other is the event kicking off an animation (or updating an existing animation), which then ticks over time and does a bit of scrolling each tick
19:48botond|wfhrhunt: mousewheel does the latter
19:49botond|wfhrhunt: and we probably want keyboard to do the latter as well
19:50botond|wfhrhunt: actually, mousewheel can do either
19:50botond|wfhrhunt: it looks like it depends on whether the smooth scrolling preference is enabled
19:50botond|wfhrhunt: so that may be the case for keyboard too
19:51botond|wfhrhunt: anyways, so we'll go through both
19:51botond|wfhrhunt: so if a mousehweel event causes scrolling "right away", it gets into the "case ScrollWheelInput::SCROLLMODE_INSTANT" block of APZC::OnScrollWheel()
19:52botond|wfhrhunt: and then the function that actually does the scrolling in that block is CallDispatchScroll()
19:52botond|wfhrhunt: what this function does is handles scroll handoff
19:53botond|wfhrhunt: so that if the mousewheel was initially scrolling a subframe, and you've reached the end, you hand off the scroll to a parent scrollable element
19:53botond|wfhrhunt: i'm not sure if keyboard has that behaviour,,,
19:53rhuntbotond: okay that makes sense, do you know why this needs a start point and end point instead of just a delta? not sure how that fits with key events
19:54botond|wfhrhunt: :) good question
19:54botond|wfhrhunt: the answer is 3D transforms
19:54rhuntbotond: I don't know if it does handoff, I couldn't do it on one of my test pages so I dunno?
19:55botond|wfhrhunt: with certain 3D transforms, a given delta in one place on the page represents a larger distance than the same delta in another place
19:56rhuntbotond: oh interesting, I guess that makes sense!
19:56botond|wfhrhunt: like, imagine the scrollable element was rotated "into" the screen, so that parts near the top of the screen are "farhter away", and parts near the bottom are "closer"
19:57botond|wfhrhunt: then a delta of K screen pixels near the top represents more movement than a delta of K screen pixels near the bottom
19:58botond|wfhrhunt: anyways, this doesn't really apply to keyboard events
19:59botond|wfhrhunt: so we can probably do something like use 0 for the start point, and delta for the end point
20:01rhuntbotond: hmm, what coord space are start and end point in?
20:01botond|wfhrhunt: i can't get keyboard events to perform scroll handoff, either
20:01botond|wfhrhunt: chances are they don't do it, because it would require changing the focus, and it would be awkward for continued scrolling to cause a focus change
20:02botond|wfhrhunt: in which case, instead of calling CallDispatchScroll, we can just call AttemptScroll directly (that's what CallDispatchScroll does after deciding which APZC to hand the scroll to)
20:02rhuntbotond: the code calling nsIScrollableFrame for keyboard scrolling is here, I'd guess it has a parameter for handoff?
20:03botond|wfhrhunt: (and it's then AttemptScroll that actually changes the scroll offset by calling ScrollBy)
20:04botond|wfhrhunt: which parameter is that?
20:04rhuntbotond: oh I don't know if there was a parameter :) I was just guessing scroll handoff might be decided by the caller at that spot
20:05botond|wfhrhunt: the start and end point arguments to AttemptScroll (or CallDispatchScroll) are in the APZC's ParentLayer coordinates
20:05botond|wfhrhunt: which is a coordinate space where any transforms on the scrollable content are applied, but transforms on the scrollframe's "chrome" (think border, scrollbars) and surrounding content are not yet applied
20:08botond|wfhrhunt: it looks like nsIScrollableFrame::ScrollLine() takes an "overflow" out-parameter which, if passed, is populated with the amount of scroll that couldn't be consumed by that scroll frame
20:08botond|wfhrhunt: and it's then up to the caller to hand it to another scrollframe if they wish
20:08botond|wfhrhunt: the call site you linked to passes nullptr for that parameter, which means they're ignoring the overflow ==> no handoff
20:09botond|wfhrhunt: er, i meant nsIScrollableFrame::ScrollBy(), sorry
20:09botond|wfh(the function being called in the place you linked)
20:10rhuntbotond: ah so that solves that mystery! no handoff
20:10botond|wfhrhunt: yup. so anyways, that should cover the instant-scrolling codepath
20:11botond|wfhrhunt: the other codepath is the "case ScrollWheelInput::SCROLLMODE_SMOOTH" block in OnScrollWheel()
20:12botond|wfhrhunt: that creates a class derived from AsyncPanZoomAnimation (in this case, WheelScrollAnimation), and kicks it off by passing it to APZC::StartAnimation()
20:12rhuntbotond: does APZC::AttemptScroll do scroll handoff too potentially?
20:13botond|wfhrhunt: which causes the compositor to call DoSample() on the animation on each composite until DoSample() returns false
20:13botond|wfhrhunt: and it's WheelScrollAnimation::DoSample() that does the actual scrolling (in this case, by calling ScrollBy() directly)
20:14botond|wfhrhunt: we also have a codepath where, if there is already a WheelScrollAnimation in progress, we update it in response to the new scrollwheel tick, rather than discarding it and starting a new one
20:14botond|wfhrhunt: we may be able to reuse WheelScrollAnimation for at least some kinds of keyboard scrolling, since a wheel tick and an up/down keyboard arrow press basically do the same thing (they both get into PresShell::ScrollByLine, i think?)
20:15botond|wfhrhunt: yeah, so the way handoff works is you call CallDispatchScroll, which chooses an initial scrolling target, and calls AttemptScroll on that
20:16botond|wfhrhunt: and if there is overflow, AttemptScroll calls CallDispatchScroll again which chooses another scrolling target
20:16botond|wfhrhunt: the two functions are mutually recursive
20:16rhuntbotond: huh, WheelScrollAnimation might even be able to handle page scrolls too, if I'm reading it right
20:16botond|wfhrhunt: yup, because mice can be configured to scroll by a whole page on each tick
20:17botond|wfhrhunt: notice the call to CallDispatchScroll inside AttemptScroll is gated on AllowScrollHandoffInCurrentBlock()
20:17botond|wfhrhunt: we can arrange for that function to return false when the current block is a keybaord block
20:17botond|wfh(assuming we'll have such things as keyboard blocks)
20:20rhuntbotond: so I was wondering about that, it seems like we could have a KeyboardBlock, but keyboard events won't wait for a content response and so won't be cancelable so I'm not sure what that would really accomplish?
20:20botond|wfhrhunt: it may not accomplish much
20:20botond|wfhrhunt: but it may still be cleaner codewise to maintain the property that every event belongs to a block?
20:22rhuntbotond: yeah I could see that
20:23rhuntbotond: I'm trying to write up what state we'll need for processing key events
20:23rhuntbotond: right now I'm just storing the current focus target in APZCTM
20:24rhuntbotond: we'll also need the seq number associated with that focus target
20:24rhuntbotond: maybe someday with a flag about mouselisteners
20:25rhuntbotond: all of those don't really belong to a block though? they're constant between blocks?
20:26botond|wfhrhunt: yeah, sounds like those should be in APZCTM or InputQueue itself
20:28rhuntbotond: okay, so with both these paths of scrolling, they end up updating FrameMetrics::scrollOffset and how does that get synced back to content? RequestRepaint?
20:30botond|wfhrhunt: yeah
20:31botond|wfhrhunt: AttemptScroll calls ScheduleCompositeAndMaybeRepaint which calls RequestContentRepaint
20:31botond|wfhrhunt: for animations, AsyncPanZoomAnimation exposes a WantsRepaints() method that you can override to say whether your animation wants a repaint after each sample
20:32botond|wfhrhunt: and the function that calls DoSample (APZC::UpdateAnimation()) will call RequestContentRepaint in that case
20:32botond|wfhrhunt: RequestContentRepaint is forwarded to the GeckoContentController
20:33botond|wfhrhunt: and, after travelling over relevant IPC channels, it gets into APZCCallbackHelper::UpdateRootFrame() or UpdateSubFrame()
20:33botond|wfhrhunt: which eventually looks up the nsIScrollableFrame and calls ScrollToCSSPixelsApproximate() on it
20:46rhuntbotond: okay, that makes sense
20:47rhuntI'm going to see if I can get this prototyped
20:47botond|wfhrhunt: cool - thanks!
18 May 2017
No messages
Last message: 10 days and 4 hours ago