874302 - DrawWindow is used by browser-gestureSupport

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This will be sad.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

I'm not sure but it looks like we are using drawWindow during overscroll, this is likely to cause a noticeable amount of jank.

Comment 2

[Stephen A Pohl [:spohl]]

Could you help me understand this a bit better? What do you mean by overscroll? Is drawWindow known to cause jank and if so why? Is there a good alternative that's preferred?

fwiw, drawWindow is called when we take a snapshot of the current page, which will be the background of the box element for the current page during animations. During animations, one box element (previous or next page) slides over a box element of the current page.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Stephen Pohl [:spohl] from comment #2)
> Could you help me understand this a bit better? What do you mean by
> overscroll? Is drawWindow known to cause jank and if so why? Is there a good
> alternative that's preferred?

Yes, calling drawWindow requires us to redraw the entire page. You should time how long it takes, but it can easily be more than 15ms. Unfortunately, there are no great alternatives at this point. Implementing the animation in the compositor with the asynchronous scrolling is the better way of implementing this.

> fwiw, drawWindow is called when we take a snapshot of the current page,
> which will be the background of the box element for the current page during
> animations. During animations, one box element (previous or next page)
> slides over a box element of the current page.

When do we all take a snapshot? Does it happening during scrolling? It looked like it was happening when you scroll up beyond the bounds of the page.

Comment 4

[Stephen A Pohl [:spohl]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> (In reply to Stephen Pohl [:spohl] from comment #2)
> When do we all take a snapshot? Does it happening during scrolling? It
> looked like it was happening when you scroll up beyond the bounds of the
> page.

Ah, right: We take snapshots for history swipe animations (bug 678392) and we currently still take snapshots for the bounce behavior when scrolling (bug 673875). In bug 868166, we'll stop taking snapshots for the bounce effect.

Is my understanding correct that drawWindow is okay in the history swipe animation use case, or are you suggesting that we change it there too?

Comment 5

[Markus Stange [:mstange]]

(In reply to Stephen Pohl [:spohl] from comment #4)
> Is my understanding correct that drawWindow is okay in the history swipe
> animation use case, or are you suggesting that we change it there too?

For the currently shown page we should be able to do without drawWindow, for example by showing -moz-element(#theactualbrowserelement) in the animated box for the current page. But for those pages that one can swipe back / forward to, we definitely need a way to store their snapshots in memory somehow. If there were a way to get the snapshot asynchronously from the compositor, that would be perfect.

Comment 6

[Matt Woodrow (:mattwoodrow)]

-moz-element is also really slow unfortunately. Async readback from the compositor would be a much better choice there.

Comment 7

[Markus Stange [:mstange]]

What time frame are we talking about? Would there be a chance to make -moz-element fast (by making the compositor handle simple cases) first?

Comment 8

[Matt Woodrow (:mattwoodrow)]

I'm not sure about the time frame, but we definitely should fix -moz-element at some point.

The problem isn't really the compositor, but more that we don't support this with display lists. Currently we break out of normal display list / layerization, and instead render the entire -moz-element contents into a software buffer.

I think we'd want to be able to have the same display item in multiple places in the display list. Then we could use the same layer for both the original rendering, and the -moz-element reproduction.

Comment 9

[Markus Stange [:mstange]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> I think we'd want to be able to have the same display item in multiple
> places in the display list.

Bug 644362 has what seems to me a simpler approach.

Comment 10

[Stephen A Pohl [:spohl]]

We have switched to arrows instead of page snapshots for history swiping in bug 860493 and removed the drawWindow call in the process.