Bug 44380 - [Qt] Layer approach to support generic shadow handling
Summary: [Qt] Layer approach to support generic shadow handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ariya Hidayat
URL:
Keywords: HTML5, Qt, QtTriaged
Depends on:
Blocks: 34479
  Show dependency treegraph
 
Reported: 2010-08-21 07:21 PDT by Ariya Hidayat
Modified: 2010-08-22 01:37 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.94 KB, patch)
2010-08-21 07:55 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff
Patch (10.14 KB, patch)
2010-08-21 16:57 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff
Patch (10.12 KB, patch)
2010-08-21 18:19 PDT, Ariya Hidayat
no flags Details | Formatted Diff | Diff
Patch (10.12 KB, patch)
2010-08-22 00:14 PDT, Ariya Hidayat
krit: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ariya Hidayat 2010-08-21 07:21:31 PDT
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.
Comment 1 Ariya Hidayat 2010-08-21 07:55:49 PDT
Created attachment 65025 [details]
Patch
Comment 2 Ariya Hidayat 2010-08-21 16:57:04 PDT
Created attachment 65037 [details]
Patch
Comment 3 Ariya Hidayat 2010-08-21 18:19:19 PDT
Created attachment 65038 [details]
Patch
Comment 4 Dirk Schulze 2010-08-21 22:17:50 PDT
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.
Comment 5 Ariya Hidayat 2010-08-21 23:21:11 PDT
> 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.
Comment 6 Dirk Schulze 2010-08-21 23:53:13 PDT
(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.
Comment 7 Ariya Hidayat 2010-08-22 00:06:04 PDT
> 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".
Comment 8 Dirk Schulze 2010-08-22 00:13:51 PDT
Comment on attachment 65038 [details]
Patch

Indeed. Please fix the typo. r=me.
Comment 9 Ariya Hidayat 2010-08-22 00:14:31 PDT
Created attachment 65050 [details]
Patch
Comment 10 Dirk Schulze 2010-08-22 00:16:00 PDT
Comment on attachment 65050 [details]
Patch

r=me, again ;-)
Comment 11 Ariya Hidayat 2010-08-22 01:37:26 PDT
Committed r65782: <http://trac.webkit.org/changeset/65782>