982596 - APZC needs to base displayport calculation on visible rect of composition bounds

Status: RESOLVED INVALID

(Core :: Layout, defect)

Description

[Timothy Nikkel (:tnikkel)]

Let H be the height of the scroll port of a scrollable element. Let V be the visible height. APZC will calculate a displayport that is basically some multiple, say k, of H tall. Making it k*H tall.

If we then translate that display port rect into layer margins (like bug 957668) and apply those margins to the visible rect we will get a resulting display port that is (k-1)*H + V. In other words we only subtract the non-visible height, all of the extra multiples of H remain.

So just using layer margins like bug 957668 won't be enough to solve our memory woes.  We'll need APZC to consider the visible rect when calculating the display port.

Comment 1

[Timothy Nikkel (:tnikkel)]

Attachment: Part 1. Add a 'seeable' rect to frame metrics.

Probably didn't need all the java plumbing in retrospect.

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Part 2. Make APZC use the seeable rect for calculating display port and other places.

Comment 3

[Timothy Nikkel (:tnikkel)]

Attachment: Part 3. Add the actual calculation of the seeable rect to layout and populate it to the frame metrics.

Comment 4

[Markus Stange [:mstange]]

(In reply to Timothy Nikkel (:tn) from comment #0)
> Let H be the height of the scroll port of a scrollable element. Let V be the
> visible height. APZC will calculate a displayport that is basically some
> multiple, say k, of H tall. Making it k*H tall.
> 
> If we then translate that display port rect into layer margins

I don't understand this part. Isn't the point of the translation to margins to stop using a multiple of the scrollport height as the start of the calculation? I thought in the world with displayport margins there would no longer be a k, only a margin in screen pixels that's based on the current scroll velocity.

Comment 5

[Timothy Nikkel (:tnikkel)]

The patches in bug 957668 very shallowly convert the display port to layer margins. In fact APZC isn't changed at all by those patches. Instead when content gets a new display port from APZC it then translates it into layer margins. Does that make it clearer?

Basically APZC needs to modify it's display port calculation and I don't think there is any way it can intelligently do so without knowing the visible rect. Whether we convert to layer margins there or not doesn't matter, the math will work out the same.

The reason that layer margins are important is the following case: we zoom (increase the resolution) of a document, APZC will send this zoom change for layout to update to before APZC changes any displayports on non-root scrollable frames. So until APZC sends us new display ports for the child scrollables that takes into account the new zoom layout needs to do something to keep those displayports reasonable. This is when we convert the existing display port to the _old_ layer pixels (before zoom), convert that to layer margins, apply those layer margins to the new visible rect (this time we are in the new layer pixels). This way we expand the display port from the visible rect by roughly the same number of layer pixels.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Markus Stange [:mstange] from comment #4)
> calculation? I thought in the world with displayport margins there would no
> longer be a k, only a margin in screen pixels that's based on the current
> scroll velocity.

So we could calculate the exact same rect, then convert it into margins expressed in screen pixels. But these will end up being too big in some cases (imagine a 100000px tall iframe). I'm saying we need to change the way APZC calculates the rect in the first place. And I'm further saying that it needs the visible rect in order to calculate it in a reasonable way. How else could APZC calculate a displayport for a 10000px tall iframe unless it knows what part of the iframe is visible? Currently it says the entire iframe is visible.

Comment 7

[Markus Stange [:mstange]]

Hmm, looks like I had a different understanding of what the plan was. I understand how APZC would need to know the visible rect in order to calculate a display port rect to request. But I thought the idea was to stop having APZC request a *rect* in the first place - instead, it should only request margins, without knowing or caring about the exact size of the rect that those would apply to. Did I misunderstand the proposal?

Also, do you agree that APZC wouldn't need to know the visible rect in order to request sensible margins?
The way I imagined it is that there's a certain number of pixels that can be scrolled into view in a short time, which we want to prerender, and that this number of pixels doesn't depend on the size (visible or not) of the scrollable element.

Comment 8

[Botond Ballo [:botond]]

(In reply to Timothy Nikkel (:tn) from comment #0)
> Let H be the height of the scroll port of a scrollable element. Let V be the
> visible height. APZC will calculate a displayport that is basically some
> multiple, say k, of H tall. Making it k*H tall.
> 
> If we then translate that display port rect into layer margins (like bug
> 957668) and apply those margins to the visible rect we will get a resulting
> display port that is (k-1)*H + V. In other words we only subtract the
> non-visible height, all of the extra multiples of H remain.
> 
> So just using layer margins like bug 957668 won't be enough to solve our
> memory woes.  We'll need APZC to consider the visible rect when calculating
> the display port.

Have we considered representing the displayport as just the "k" or something similar, and having layout fill in pieces like "V"?

Comment 9

[Timothy Nikkel (:tnikkel)]

(In reply to Markus Stange [:mstange] from comment #7)
> Hmm, looks like I had a different understanding of what the plan was. I
> understand how APZC would need to know the visible rect in order to
> calculate a display port rect to request. But I thought the idea was to stop
> having APZC request a *rect* in the first place - instead, it should only
> request margins, without knowing or caring about the exact size of the rect
> that those would apply to. Did I misunderstand the proposal?

I'm not sure, I guess it depends what everyone was thinking exactly. But what you describe is different from the patches that kats posted. Those patches don't change how or even what APZC calculates; they just translate the same rect to margins (which can be applied to a rect different from what APZC used to calculate them). That approach alone isn't enough as I outlined above. This may or may not differ from what was understood as the proposal.

> Also, do you agree that APZC wouldn't need to know the visible rect in order
> to request sensible margins?

Sure, to request sensible margins. But we want to do better than just sensible, and I'm not convinced that the height should not come into play here.

> The way I imagined it is that there's a certain number of pixels that can be
> scrolled into view in a short time, which we want to prerender, and that
> this number of pixels doesn't depend on the size (visible or not) of the
> scrollable element.

If we are not talking about a fling (a separate issue) then the amount of content that can be scrolled into view in a short time is via the user putting their finger on the phone, and dragging the entire height of the visible scrolled content (and then stopping their finger). Which pretty clearly depends on the height of the scrollable element.

And if we consider faster scrolling I think that the amount of content that is visible does have an effect on how quickly a user will want to scroll through it.

Comment 10

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #8)
> Have we considered representing the displayport as just the "k" or something
> similar, and having layout fill in pieces like "V"?

That might work.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

(In reply to Timothy Nikkel (:tn) from comment #9)
> (In reply to Markus Stange [:mstange] from comment #7)
> > Hmm, looks like I had a different understanding of what the plan was. I
> > understand how APZC would need to know the visible rect in order to
> > calculate a display port rect to request. But I thought the idea was to stop
> > having APZC request a *rect* in the first place - instead, it should only
> > request margins, without knowing or caring about the exact size of the rect
> > that those would apply to. Did I misunderstand the proposal?

That was the original proposal, yeah.

> I'm not sure, I guess it depends what everyone was thinking exactly. But
> what you describe is different from the patches that kats posted. Those
> patches don't change how or even what APZC calculates; they just translate
> the same rect to margins (which can be applied to a rect different from what
> APZC used to calculate them). That approach alone isn't enough as I outlined
> above. This may or may not differ from what was understood as the proposal.

The patches I posted were very incomplete. That may have been the source of misunderstanding. For the most part the changes I made to the APZCCallbackHelper were made to get it to compile with the new signature and to verify that the changes on the layout side worked as intended. I probably forgot this and/or misrepresented it to you when I handed off the bug, sorry.

> > Also, do you agree that APZC wouldn't need to know the visible rect in order
> > to request sensible margins?
> 
> Sure, to request sensible margins. But we want to do better than just
> sensible, and I'm not convinced that the height should not come into play
> here.
> 
> > The way I imagined it is that there's a certain number of pixels that can be
> > scrolled into view in a short time, which we want to prerender, and that
> > this number of pixels doesn't depend on the size (visible or not) of the
> > scrollable element.
> 
> If we are not talking about a fling (a separate issue) then the amount of
> content that can be scrolled into view in a short time is via the user
> putting their finger on the phone, and dragging the entire height of the
> visible scrolled content (and then stopping their finger). Which pretty
> clearly depends on the height of the scrollable element.
> 
> And if we consider faster scrolling I think that the amount of content that
> is visible does have an effect on how quickly a user will want to scroll
> through it.

Timothy and I discussed this aspect a bit last Friday; and I've been thinking about it a bit more since then. Timothy's argument, which I agreed with on Friday, is that when you're panning with your finger on the screen, you can at most scroll one seeable-rect-height before you have to lift your finger and reposition it. So from that point of view you want to pre-render at least that much content as part of the displayport. However what I realized was missing from this view is that you are ALSO bounded by how fast you can move your finger. We send repaint requests every 16ms now while panning (throttled down if the previous request isn't finished yet) so if you can only move your finger 1 cm on the screen in that time, there's no reason we need to repaint the entire seeable-rect-height rather than just the 1 cm. Therefore I think the amount you want pre-render in the "panning" case is min(seeable-rect-height, max-finger-velocity-based-value).

When flinging (finger comes off the screen) the max fling velocity is also dependent on how much of a "runway" you have to accelerate your finger through - however note that once you put your finger down on a subframe, all events are sent to that subframe so the runway pretty much always extends to the end of the screen. So we can ignore the runway length argument, and assume the user can always generate the same max velocity regardless of the size of the visible rect. To completely avoid checkerboarding, we always to have content pre-rendered based on the max possible fling velocity.

So I think in both of these cases there is a strong argument to just use a max-velocity based displayport margin rather than making it dependent on the seeable-rect-height. The max-velocity is going to be roughly constant in cm/second; this can be converted to layer pixels/second and multiplied by the expected round-trip time for a paint request (in seconds), giving us the layer pixel margin value we want to use.

Comment 12

[Markus Stange [:mstange]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> however note that once you put your finger down on a subframe, all
> events are sent to that subframe so the runway pretty much always extends to
> the end of the screen.

Isn't this also the case when you're not flinging? So the argument that the range of a non-fling-scroll is bounded by the seeable rect size isn't actually true, is it?

> So I think in both of these cases there is a strong argument to just use a
> max-velocity based displayport margin rather than making it dependent on the
> seeable-rect-height.

Sounds convincing to me :-)

Comment 13

[Kartikaya Gupta (email:[e-mail])]

(In reply to Markus Stange [:mstange] from comment #12)
> (In reply to Kartikaya Gupta (email:[e-mail]) from comment #11)
> > however note that once you put your finger down on a subframe, all
> > events are sent to that subframe so the runway pretty much always extends to
> > the end of the screen.
> 
> Isn't this also the case when you're not flinging? So the argument that the
> range of a non-fling-scroll is bounded by the seeable rect size isn't
> actually true, is it?

You're right, I totally missed that aspect of it.

Comment 14

[Timothy Nikkel (:tnikkel)]

Okay, that does give me new info and changes some of my assumptions. I'll go back to the drawing board.