Closed Bug 1360127 Opened 4 years ago Closed 4 years ago

Support SVG image for wr background image 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

Attachments

(1 file, 2 obsolete files)

We don't support a image without ImageContainer. But currently we don't do the check in the nsDisplayBackgroundImage::GetLayerState. So reftests with svg will not pass. For instance, image/test/reftest/downscaling/downscale-svg-1a.html[1].


[1] https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/bcFFpERLQM2S-w27Qjiqog/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
Currently we prepare the image when we are going to build wr commands[1]. I think it's better but it's hard to check if the container exists when getting layer state. So I think we can use 'PushItemAsImage' for this case. 


[1] https://dxr.mozilla.org/mozilla-central/source/layout/painting/nsCSSRendering.cpp#2754
Attachment #8862319 - Flags: review?(mchang)
Comment on attachment 8862319 [details] [diff] [review]
Support non-imagecontainer images

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

Is this patch complete? I don't see PushItemAsImage defined anywhere.
(In reply to Mason Chang [:mchang] from comment #2)
> Comment on attachment 8862319 [details] [diff] [review]
> Support non-imagecontainer images
> 
> Review of attachment 8862319 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is this patch complete? I don't see PushItemAsImage defined anywhere.

We already have this function in WebRenderDisplayItemLayer[1].

[1] https://hg.mozilla.org/projects/graphics/file/tip/gfx/layers/wr/WebRenderDisplayItemLayer.cpp#l116
(In reply to Ethan Lin[:ethlin] from comment #3)
> (In reply to Mason Chang [:mchang] from comment #2)
> > Comment on attachment 8862319 [details] [diff] [review]
> > Support non-imagecontainer images
> > 
> > Review of attachment 8862319 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Is this patch complete? I don't see PushItemAsImage defined anywhere.
> 
> We already have this function in WebRenderDisplayItemLayer[1].
> 
> [1]
> https://hg.mozilla.org/projects/graphics/file/tip/gfx/layers/wr/
> WebRenderDisplayItemLayer.cpp#l116

Ahh sorry this wasn't showing up in DXR.

Can't you just check what type of image container this is [1], and if it's TYPE_VECTOR, then we know it's SVG? In that case, don't create a layer for it.

[1] http://searchfox.org/mozilla-central/source/__GENERATED__/dist/include/imgIContainer.h#78
Flags: needinfo?(ethlin)
Okay, if we don't want to another layer, maybe we could check imgContainer's 'IsImageContainerAvailable', like nsBulletFrame[1].

[1] https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsBulletFrame.cpp#624
Flags: needinfo?(ethlin)
Attachment #8862319 - Attachment is obsolete: true
Attachment #8862319 - Flags: review?(mchang)
Attachment #8862772 - Flags: review?(mchang)
Fix some nits.
Attachment #8862772 - Attachment is obsolete: true
Attachment #8862772 - Flags: review?(mchang)
Attachment #8862774 - Flags: review?(mchang)
Comment on attachment 8862774 [details] [diff] [review]
check if the image container available

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

::: layout/painting/nsCSSRendering.cpp
@@ +1939,5 @@
>    return PaintStyleImageLayerWithSC(aParams, aRenderingCtx, sc, *aParams.frame->StyleBorder());
>  }
>  
>  bool
> +nsCSSRendering::CanBuildWebRenderDisplayItemsForStyleImageLayer(mozilla::layers::LayerManager* aManager,

nit: fix aManager line spacing

::: layout/painting/nsCSSRendering.h
@@ +490,5 @@
>                                                 nsRenderingContext& aRenderingCtx,
>                                                 nsStyleContext *mBackgroundSC,
>                                                 const nsStyleBorder& aBorder);
>  
> +  static bool CanBuildWebRenderDisplayItemsForStyleImageLayer(mozilla::layers::LayerManager* aManager,

fix line spacing
Attachment #8862774 - Flags: review?(mchang) → review+
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/1ba304cfe390
If the background image doesn't have a image container, we shouldn't create a layer for it. r=mchang
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.