[webkit-reviews] review denied: [Bug 50422] [Qt] No shadow offset should be affected by any transformation : [Attachment 76006] Refactored, more comments, added new test

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at gmail.com> has denied Helder Correia
<helder at sencha.com>'s request for review:
Bug 50422: [Qt] No shadow offset should be affected by any transformation
https://bugs.webkit.org/show_bug.cgi?id=50422

Attachment 76006: Refactored, more comments, added new test
https://bugs.webkit.org/attachment.cgi?id=76006&action=review

------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=76006&action=review

> WebCore/platform/graphics/ContextShadow.cpp:165
> +    QTransform transform = p->transform();

Use const ref.

> WebCore/platform/graphics/ContextShadow.cpp:167
> +	   QPolygonF transDestPolygon = transform.map(QPolygonF(m_layerArea));

transformedPolygon? I don't think Dest is necessary here (there is no Source
and Dest per se, for transformation)

> WebCore/platform/graphics/ContextShadow.h:110
> +    bool shadowsIgnoreTransforms() { return m_shadowsIgnoreTransforms; }

Make it a const.

> WebCore/platform/graphics/ContextShadow.h:127
> +    // Enclosing int rect where shadow needs to be drawn to using the layer
context.
>      IntRect m_layerRect;
> +    // Same as m_layerRec but has full precision (useful for lossless
transformations).
> +    FloatRect m_layerFloatRect;
> +    // Rect occupied by the original shape or image.
> +    FloatRect m_layerArea;
> +    // Buffer to where the temporary shadow will be drawn to.
>      PlatformImage m_layerImage;

Instead of m_layerArea, m_layerFloatRect and m_layerInflation, I propose using
m_layerOrigin (top left corner for endShadowLayer) and m_sourceRect (for the
scratch image). These variables are potentially easier to understand.