1016982 - Async pannable scroll layers ignore scrollbox corner radius

Status: RESOLVED DUPLICATE of bug 1148582

(Core :: Layout, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

The clip wasn't using the border radius.

Comment 1

[Timothy Nikkel (:tnikkel)]

I think all our layers backends support mask layers for rounded rect clipping. But do we still want to ignore rounded rect clipping on b2g and/or android so we don't have a mask layer for perf reasons?

Comment 2

[Markus Stange [:mstange]]

Are mask layers that expensive? I was of the impression that they were the fastest way to get the desired effect (i.e. scrolling inside rounded clips). And I think rounded scrollboxes are uncommon enough that the affected pages can live with a small perf hit, and that that hit would be preferable to having the radius ignored completely.

Comment 3

[Timothy Nikkel (:tnikkel)]

I'm not sure how expensive they are, but they are surely more expensive then not doing them at all. :) They'd also increase our memory use.

Comment 4

[Markus Stange [:mstange]]

Fair enough. Who can make the decision here?

Comment 5

[Markus Stange [:mstange]]

Benoit, can you advise?

Comment 6

[Benoit Girard (:BenWa)]

Mask layers are expensive but in some cases they're the only way to have proper rendering with active layers. Aside from the memory usage increase they will also increase the time to build a transaction (latency). When compositing the mask layers will account for more memory bandwidth usage and dependent texture reads.

The question is if the active layer that is causing us to require a mask layer is saving more time then the mask layer is costing. I guess in most cases the answer might be a net save if we have async pannable content there.

IMO we should support them unless we take a clear and explicit decision to cut corners (that pun was by accident I swear).

Nevertheless we should do some profiling with and without mask layers to get some true numbers.

Comment 7

[Markus Stange [:mstange]]

Thank you.

(In reply to Benoit Girard (:BenWa) from comment #6)
> The question is if the active layer that is causing us to require a mask
> layer is saving more time then the mask layer is costing.

This patch does not change whether a scroll frame gets an active layer. At the moment, as soon as you start async panning a scroll frame with rounded corners, the corners become square, and this patch makes them respect the corner radius again (and triggers mask layer creation and so on).

> IMO we should support them unless we take a clear and explicit decision to
> cut corners (that pun was by accident I swear).

Heh, good. I agree. :)

> Nevertheless we should do some profiling with and without mask layers to get
> some true numbers.

Alright, I'll do some profiling tomorrow.

Comment 8

[Markus Stange [:mstange]]

Attachment: testcase

Comment 9

[Markus Stange [:mstange]]

With the patch applied, having border-radius makes the difference of 20fps vs 60fps on my Hamachi... this is much more expensive than I thought it would be. And the fill rate numbers go through the roof.

Comment 10

[Markus Stange [:mstange]]

(In reply to Markus Stange [:mstange] from comment #9)
> And the fill rate numbers go through the roof.

And that's because a mask layer on a container layer forces the container layer to use an intermediate surface, and the intermediate surface is sized to the whole display port. This needs to be fixed. I'm on it.

Comment 11

[Markus Stange [:mstange]]

Comment on attachment 8430048 [details] [diff] [review]
v1

I've filed bug 1016982 about the performance problem. Let's not land this patch until it's fixed.

Comment 12

[Markus Stange [:mstange]]

This was fixed by one of the patches in bug 1148582.