Bug 129154 – An element having -webkit-mix-blend mode should only blend with the contents of the parent stacking context

• Component: CSS
• Status: RESOLVED FIXED

[Mihai Tica]

As stated in the Blending and Compositing spec (http://dev.w3.org/fxtf/compositing-1/):
An element that has blending applied, must blend with all the underlying content of the stacking context [CSS21] that that element belongs to.

Currently, the element blends with every content drawn below. This issue implies that blending should be isolated and shouldn't blend with content outside the parent stacking context.

[Mihai Tica]

Created attachment 224877 [details]
Patch V1

[Dean Jackson]

Comment on attachment 224877 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review

> Source/WebCore/ChangeLog:9
> +        This change isolates blending, preventing it to blend with other underlying elements besides the parent stacking context.

Typo "to" -> "from"

> Source/WebCore/rendering/RenderLayer.cpp:814
> +    if (!oldStyle && renderer().style().hasBlendMode())
> +        m_updateParentStackingContextShouldIsolateBlendingDirty = true;
> +
> +    if (oldStyle && oldStyle->hasBlendMode() != renderer().style().hasBlendMode())
> +        m_updateParentStackingContextShouldIsolateBlendingDirty = true;

Make this one test, even though it will be long.

> Source/WebCore/rendering/RenderLayer.h:785
>  #if ENABLE(CSS_COMPOSITING)
>      bool hasBlendMode() const { return renderer().hasBlendMode(); }
> +    bool isolatesBlending() const { return m_isolatesBlending; }
>  #else
>      bool hasBlendMode() const { return false; }
> +    bool isolatesBlending() const { return false; }
>  #endif

I think it would be better to write these the other way around:

bool isolatesBlending() const
{
#if ENABLE(CSS_COMPOSITING)
  return m_isolatesBlending;
#else
  return false;
#endif
}


Same for hasBlendMode

[Mihai Tica]

Created attachment 225034 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 225034 [details]
Patch for landing

Rejecting attachment 225034 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 225034, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/5120381560029184

[Mihai Tica]

Created attachment 225042 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 225042 [details]
Patch for landing

Clearing flags on attachment: 225042

Committed r164579: <http://trac.webkit.org/changeset/164579>

[Simon Fraser (smfr)]

Comment on attachment 224877 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review

> Source/WebCore/rendering/RenderLayer.cpp:836
> +            ancestor->m_isolatesBlending = renderer().style().hasBlendMode();

Would have been nice to call setIsolatesBlending() here.

What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag.

[Mihai Tica]

(In reply to comment #7)
> (From update of attachment 224877 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:836
> > +            ancestor->m_isolatesBlending = renderer().style().hasBlendMode();
> 
> Would have been nice to call setIsolatesBlending() here.
> 
> What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag.

You are right, I've mistakenly omitted this case, it should be corrected in a follow up patch.

[Mihnea Ovidenie]

(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 224877 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review
> > 
> > > Source/WebCore/rendering/RenderLayer.cpp:836
> > > +            ancestor->m_isolatesBlending = renderer().style().hasBlendMode();
> > 
> > Would have been nice to call setIsolatesBlending() here.
> > 
> > What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag.
> 
> You are right, I've mistakenly omitted this case, it should be corrected in a follow up patch.

Filled follow-up bug: https://bugs.webkit.org/show_bug.cgi?id=129307

[Mihnea Ovidenie]

Close this one as the follow up bug landed.