[webkit-reviews] review denied: [Bug 95258] Add support for blendmode to webkit rendering engine : [Attachment 161330] renamed function so it's shorter

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Rik Cabanier
<cabanier at adobe.com>'s request for review:
Bug 95258: Add support for blendmode to webkit rendering engine
https://bugs.webkit.org/show_bug.cgi?id=95258

Attachment 161330: renamed function so it's shorter
https://bugs.webkit.org/attachment.cgi?id=161330&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=161330&action=review


> Source/WebCore/rendering/RenderBoxModelObject.h:90
> +    virtual bool requiresLayer() const { return isRoot() || isPositioned()
|| isTransparent() || hasTransform() || hasHiddenBackface() || hasReflection()
|| createsGroup() || style()->specifiesColumns(); }

You also need to change RenderBox::requiresLayer() and
RenderTableRow::requiresLayer().

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1780
> +    if (hasCompositedDescendants && (layer->transform() ||
renderer->isTransparent() || renderer->hasMask() || renderer->hasReflection()
|| renderer->hasFilter() || renderer->hasBlendMode())) {
>	   reason = RenderLayer::IndirectCompositingForGraphicalEffect;

Why not use createsGroup() here?

> Source/WebCore/rendering/RenderObject.h:943
> +    bool createsGroup() const { return hasMask() || hasFilter() ||
hasBlendMode(); } 

I think this deserves a comment explaining the usage of "group". Should
isTransparent() also be included?