[Webkit-unassigned] [Bug 130891] [CSS Blending] Blending doesn't work if the parent stacking context is not a self painting layer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130891





--- Comment #11 from Dirk Schulze <krit at webkit.org>  2014-05-06 01:25:48 PST ---
(From update of attachment 230417)
View in context: https://bugs.webkit.org/attachment.cgi?id=230417&action=review

The final review should definitely be done by someone with more insight. In general, the patch looks good to me. Some snippets.

> Source/WebCore/ChangeLog:27
> +        When hasUnisolatedBlendingDescendants gets set we make sure that isSelfPaintingLayer flag is set too.

I think it looks better to replace Unisolated with NotIsolated . Seems easier to read.

> Source/WebCore/ChangeLog:34
> +        This change should not affect the logics behind the other flags.

I hope our test coverage is good enough to assume that.

> Source/WebCore/ChangeLog:39
> +        Changing isolatesBlending() to check isStackingContext() first, this will avoid
> +        the assertion in hasUnisolatedBlendingDescendants() for non-stacking context elements.

Wow, there should probably a comment in RenderLayer.h to warn others.

>> Source/WebCore/rendering/RenderLayer.cpp:1129
>>                  break;
> 
> We can optimize this descendant walk in updateDescendantDependentFlags by leaving the loop if just dirty flags are set, not all of them. Also, we can skip entering updateDescendantDependentFlags() on a child if some conditions are met. For example if we know that a child has blendMode, we are not interested if it has other descendants with blend modes; if a child is self painting layer, we are not interested if it has descendants with that property. I can address this in a follow up patch.

Can/Should it be done before this patch then? To be honest, I am not really into the code to judge.


After reading though the code again, I think it is fine to do this in a second patch afterwards.

> Source/WebCore/rendering/RenderLayer.h:805
> +    bool isolatesBlending() const { return isStackingContext() && hasUnisolatedBlendingDescendants(); }

Please put a comment why it is necessary to do that.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.