1331538 - Fix some reftest failures when enabling WebRenderBorderLayer

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ethan Lin[:ethlin]]

There is a simple case: layout/reftests/position-dynamic-changes/multiple-changes.
The test has reftest-wait and remove the class after changing the div's left position. With WebRenderBorderLayer, the snapshot gets the image before changing the left property. I added some logs and found the transaction with new position is later than snapshot command. 
I think the mechanism of reftest-wait should be fine since for other tests with WebRenderPaintedLayer are all passed.

Comment 1

[Ethan Lin[:ethlin]]

The problem is when we use WRBorderLayer, the client rect[1] from MozAfterPaint is empty. So the reftest will not do DrawWindow to take the new snapshot. Currently FrameLayerBuilder has some mechanisms for paintedLayer[2] and imageLayer/colorLayer[3]. Maybe I should add something for borderLayer. Matt, do you have any idea for this problem?


[1] https://dxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-content.js#1135
[2] https://dxr.mozilla.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp?q=path%3AFrameLayerBuilder.cpp&redirect_type=single#4535
[3] https://dxr.mozilla.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp?q=path%3AFrameLayerBuilder.cpp&redirect_type=single#3183

Comment 2

[Matt Woodrow (:mattwoodrow)]

I think you need to add border (and text!) support within LayerTreeInvalidation.

http://searchfox.org/mozilla-central/source/gfx/layers/LayerTreeInvalidation.cpp#650

Comment 3

[Ethan Lin[:ethlin]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)
> I think you need to add border (and text!) support within
> LayerTreeInvalidation.
> 
> http://searchfox.org/mozilla-central/source/gfx/layers/LayerTreeInvalidation.
> cpp#650

Thanks! I should add one for border.

Comment 4

[Ethan Lin[:ethlin]]

Attachment: add BorderLayerProperties

I add BorderLayerProperties for border layer though I'm not very sure if the calculation is correct. At least the test multiple-changes.html is passed now.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8828219 [details] [diff] [review]
add BorderLayerProperties

Review of attachment 8828219 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

Do you want to do TextLayer while you're at it? :)

Comment 6

[Ethan Lin[:ethlin]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> Comment on attachment 8828219 [details] [diff] [review]
> add BorderLayerProperties
> 
> Review of attachment 8828219 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good!
> 
> Do you want to do TextLayer while you're at it? :)

Okay! I'll have another bug for it.

Comment 7

[Ethan Lin[:ethlin]]

Please push the attachment 8828219 [details] [diff] [review] to central.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d21bd74f1a20
Add BorderLayerProperties for border layer. r=mattwoodrow

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d21bd74f1a20