Closed Bug 1354464 Opened 5 years ago Closed 5 years ago

Fix the size of webrender mask layer

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

(Keywords: correctness)

Attachments

(1 file)

Currently we apply layer's transform to stacking context in WRXXXLayer, but the size of mask layer is transformed already. So the mask layer will be transformed twice.
Blocks: 1354463
Keywords: correctness
Attached patch fix mask sizeSplinter Review
I apply the inverse layer transform to mask. The patch will fix these reftests when enabling 'layers.advanced.canvas-background-color' or 'layers.advanced.image-layers':
layout/reftests/border-radius/clipping-4-image.html
layout/reftests/border-radius/clipping-5-image.html
layout/reftests/border-radius/clipping-5-refi.html
layout/reftests/border-radius/intersecting-clipping-1-image.html
Attachment #8855703 - Flags: review?(mchang)
Attachment #8855703 - Attachment description: Part 1. fix mask size → fix mask size
(In reply to Ethan Lin[:ethlin] from comment #1)
> Created attachment 8855703 [details] [diff] [review]
> fix mask size
> 
> I apply the inverse layer transform to mask. The patch will fix these
> reftests when enabling 'layers.advanced.canvas-background-color' or
> 'layers.advanced.image-layers':
> layout/reftests/border-radius/clipping-4-image.html
> layout/reftests/border-radius/clipping-5-image.html
> layout/reftests/border-radius/clipping-5-refi.html
> layout/reftests/border-radius/intersecting-clipping-1-image.html

This is the original result of these reftests: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/T6UmOnqKQyq2Z5vJ9-YosA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
Comment on attachment 8855703 [details] [diff] [review]
fix mask size

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

::: gfx/layers/wr/WebRenderLayerManager.cpp
@@ +112,5 @@
>  {
>    if (GetLayer()->GetMaskLayer()) {
>      WebRenderLayer* maskLayer = ToWebRenderLayer(GetLayer()->GetMaskLayer());
> +    gfx::Matrix4x4 transform = GetWrBoundTransform();
> +    return maskLayer->RenderMaskLayer(transform.Inverse());

nit: Instead of passing in the transform here. Can't we just do this in RenderMaskLayer instead?

Also add a comment explaining why we have to inverse this.
Attachment #8855703 - Flags: review?(mchang) → review+
(In reply to Mason Chang [:mchang] from comment #3)
> Comment on attachment 8855703 [details] [diff] [review]
> fix mask size
> 
> Review of attachment 8855703 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/wr/WebRenderLayerManager.cpp
> @@ +112,5 @@
> >  {
> >    if (GetLayer()->GetMaskLayer()) {
> >      WebRenderLayer* maskLayer = ToWebRenderLayer(GetLayer()->GetMaskLayer());
> > +    gfx::Matrix4x4 transform = GetWrBoundTransform();
> > +    return maskLayer->RenderMaskLayer(transform.Inverse());
> 
> nit: Instead of passing in the transform here. Can't we just do this in
> RenderMaskLayer instead?
> 

Because I need the inverse transform of the original layer, but not the mask layer's transform. Or should I pass the original layer into RenderMaskLayer?

> Also add a comment explaining why we have to inverse this.

Okay, I will add a comment here.
Blocks: 1355012
try result looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6b9f0e2f4116b01994e7e2c57dfe233b0c6a507

I would like to push the patch first. Bug 1355012 is about mask layer in WebRenderDisplayItemLayer and I may change the interface of WebRenderLayer::BuildWrMaskLayer in the bug.
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/09534c0cda7c
Fix the size of webrender mask layer. r=mchang
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.