1399043 - Should multiply border width by the stacking context's scale

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Description

[Ethan Lin[:ethlin]]

We store the scale value to stacking context. But currently we only use it to calculate the bounds and clipping. Border width is another thing we should take care. A related test is "layout/reftests/bugs/586683-1.html".

Comment 1

[Ethan Lin[:ethlin]]

I think only border width has this problem.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Bug 1399043 - Turn on layers-free mode for the test about border with scale.

Review commit: https://reviewboard.mozilla.org/r/178700/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178700/

Comment 3

[Ethan Lin[:ethlin]]

Attachment: Bug 1399043 - Part2. Turn on layers-free mode for the test about border with scale.

Review commit: https://reviewboard.mozilla.org/r/178702/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178702/

Comment 4

[Ethan Lin[:ethlin]]

Sorry, I didn't think it clearly. Per discussion with Markus, border radius, box shadow, and text may also need to be scaled.

Comment 5

[Ethan Lin[:ethlin]]

Comment on attachment 8906956 [details]
Bug 1399043 - Turn on layers-free mode for the test about border with scale.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178700/diff/1-2/

Comment 6

[Ethan Lin[:ethlin]]

Comment on attachment 8906957 [details]
Bug 1399043 - Part2. Turn on layers-free mode for the test about border with scale.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178702/diff/1-2/

Comment 7

[Ethan Lin[:ethlin]]

I haven't figured out a very good way to apply the scale to all items. My current approach is to let stacking context have the ability to scale different wr properties. The current patches are for border renderer. If this way is acceptable, then I can use the same way to apply other wr commands in the following patches or bugs.

Comment 8

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

I'm not really convinced that this is the right approach to be taking. If we go down this road and scale everything on the gecko side things are going to get very complicated. It's still not clear to me why the scaling doesn't happen inside WebRender. I think we should investigate that in more detail and see if there's something we can change in WebRender to make this work. If not, we should at least properly document the problem so that it's easier to understand what things need to be scaled on the Gecko side.

Comment 9

[Ethan Lin[:ethlin]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> I'm not really convinced that this is the right approach to be taking. If we
> go down this road and scale everything on the gecko side things are going to
> get very complicated. It's still not clear to me why the scaling doesn't
> happen inside WebRender. I think we should investigate that in more detail
> and see if there's something we can change in WebRender to make this work.
> If not, we should at least properly document the problem so that it's easier
> to understand what things need to be scaled on the Gecko side.

Originally we set the scale to the stacking context's transform. So everything of the item will be scaled. In bug 1391499, we retrieve the scale from the transform for reordering the transform, and then manually apply the scale to the bounds (StackingContextHelper::ToRelativeLayoutRectRounded). So the problem is we should apply the scale to the whole item, not just bounds. An alternative way is that the reordering things probably can be done in Webrender. Then we don't need to handle the inherited scale on the gecko site.

Comment 10

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

Yeah I'm starting to think what we did in bug 1391499 was not the correct approach. I'll spend some more time investigating this as well. I'm going to back out those patches locally and see if we can make it work a different way.

Comment 11

[Ethan Lin[:ethlin]]

Comment on attachment 8906956 [details]
Bug 1399043 - Turn on layers-free mode for the test about border with scale.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/178700/diff/2-3/

Comment 12

[Ethan Lin[:ethlin]]

After bug 1400034 landed, the test is passed. So I just turn on layers-free for the test in this bug.

Comment 13

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

Comment on attachment 8906956 [details]
Bug 1399043 - Turn on layers-free mode for the test about border with scale.

https://reviewboard.mozilla.org/r/178700/#review187062

Comment 14

[Ethan Lin[:ethlin]]

"layers-free" is going to default on. I think I don't need to land this patch anymore.