Bug 44380 – [Qt] Layer approach to support generic shadow handling

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ariya Hidayat]

Class ContextShadow needs to have layer support, i.e via "begin layer" and "end layer" function which creates temporary surface to draw onto. For complex shapes (and also text) which cast shadow, with more complicated fill than just a solid color, this extra redirection is needed. Furthermore, blur filter has to operate on an extra surface anyway.

[Ariya Hidayat]

Created attachment 65025 [details]
Patch

[Ariya Hidayat]

Created attachment 65037 [details]
Patch

[Ariya Hidayat]

Created attachment 65038 [details]
Patch

[Dirk Schulze]

Comment on attachment 65038 [details]
Patch

WebCore/platform/graphics/qt/ContextShadow.cpp:270
 +  QPainter* ContextShadow::beginShadowLayer(QPainter* p, const QRectF &rect)
I wounder if we always know the size? It's not just SVG and CSS shadows for text and boxs in HTML that use shadow. We use it as well on Canvas, and it can transform the context multiple times between beginShadowLayer and endShadowLayer.

WebCore/platform/graphics/qt/ContextShadow.cpp:292
 +          // No need to have the buffer larger that the clip.
Typo: larger that the clip.

WebCore/platform/graphics/qt/ContextShadow.h:66
 +      // Note: multiple/nested shadow layer is NOT allowed.
Thats bad. In SVG we can have multiple shadows:
<g style="-webkit-svg-shadow:...">
  <rect style="-webkit-svg-shadow:..."/>
  <rect />
</g>

Can't Canvas have multiple shadows?
You may want to work more like transparencyLayer.

[Ariya Hidayat]

> I wounder if we always know the size?

Yes, since the use case of the layer is inside each primitive drawing in GraphicsContext.

>  +          // No need to have the buffer larger that the clip.
> Typo: larger that the clip.

Good catch.

> WebCore/platform/graphics/qt/ContextShadow.h:66
>  +      // Note: multiple/nested shadow layer is NOT allowed.
> Thats bad. In SVG we can have multiple shadows:
> <g style="-webkit-svg-shadow:...">
>   <rect style="-webkit-svg-shadow:..."/>
>   <rect />
> </g>
> 
> Can't Canvas have multiple shadows?
> You may want to work more like transparencyLayer.

Not sure I need to tackle these case, I may miss something here.
The plan is to use begin and end *only* inside each primitive drawing in GraphicsContext, thus there is no need to support nested calls.

[Dirk Schulze]

(In reply to comment #5)
> Not sure I need to tackle these case, I may miss something here.
> The plan is to use begin and end *only* inside each primitive drawing in GraphicsContext, thus there is no need to support nested calls.

Hm, if I understand you correctly, you just call beginShadowLayer and endShadowLayer for every primitive inside GC? That would mean, you can't make a groupe shadow of more than one primitive, right? At least SVG needs the ability to apply groupe shadows. And IIRC Canvas too?
I know that groupe shadow soesn't work right now, neither for Cairo, nor Skia, nor Qt. But a nested, common implementation of beginShadowLayer and endShadowLayer, that can be called from outside of GC could solve this problem. That's why I asked if it is possible to make it more like transparencyLayer.

[Ariya Hidayat]

> Hm, if I understand you correctly, you just call beginShadowLayer and endShadowLayer for every
> primitive inside GC? That would mean, you can't make a groupe shadow of more than one primitive, right? 

While I agree GraphicsContext::beginShadowLayer (and endShadowLayer) is a nice thing to have, unfortunately it is outside the scope of this patch.

Even if in the end we have such functionality in GraphicsContext, we still need something similar (ContextShadow) since Qt does not have native support for creating drop shadow on the fly.

IOW, don't think this like a layer but more like "give me some surface for you to filter+blur, as the shadow for my shape".

[Dirk Schulze]

Comment on attachment 65038 [details]
Patch

Indeed. Please fix the typo. r=me.

[Ariya Hidayat]

Created attachment 65050 [details]
Patch

[Dirk Schulze]

Comment on attachment 65050 [details]
Patch

r=me, again ;-)

[Ariya Hidayat]

Committed r65782: <http://trac.webkit.org/changeset/65782>