1360127 - Support SVG image for wr background image layer

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Ethan Lin[:ethlin]]

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

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Support non-imagecontainer images

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

Comment 2

[Mason Chang [Inactive] [: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.

Comment 3

[Ethan Lin[:ethlin]]

(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

Comment 4

[Mason Chang [Inactive] [:mchang]]

(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

Comment 5

[Ethan Lin[: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

Comment 6

[Ethan Lin[:ethlin]]

Attachment: check if the image container available

Comment 7

[Ethan Lin[:ethlin]]

Attachment: check if the image container available

Fix some nits.

Comment 8

[Mason Chang [Inactive] [: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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/1ba304cfe390