Closed Bug 1331538 Opened 3 years ago Closed 3 years ago

Fix some reftest failures when enabling WebRenderBorderLayer

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

Attachments

(1 file)

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.
See Also: → 1329609
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
Flags: needinfo?(matt.woodrow)
I think you need to add border (and text!) support within LayerTreeInvalidation.

http://searchfox.org/mozilla-central/source/gfx/layers/LayerTreeInvalidation.cpp#650
Flags: needinfo?(matt.woodrow)
(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.
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.
Attachment #8828219 - Flags: review?(matt.woodrow)
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? :)
Attachment #8828219 - Flags: review?(matt.woodrow) → review+
(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.
Keywords: checkin-needed
Please push the attachment 8828219 [details] [diff] [review] to central.
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d21bd74f1a20
Add BorderLayerProperties for border layer. r=mattwoodrow
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d21bd74f1a20
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.