RESOLVED FIXED 129154
An element having -webkit-mix-blend mode should only blend with the contents of the parent stacking context
https://bugs.webkit.org/show_bug.cgi?id=129154
Summary An element having -webkit-mix-blend mode should only blend with the contents ...
Mihai Tica
Reported 2014-02-21 08:32:30 PST
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.
Attachments
Patch V1 (69.42 KB, patch)
2014-02-21 09:11 PST, Mihai Tica
dino: review+
Patch for landing (69.63 KB, patch)
2014-02-24 01:03 PST, Mihai Tica
commit-queue: commit-queue-
Patch for landing (69.63 KB, patch)
2014-02-24 01:25 PST, Mihai Tica
no flags
Mihai Tica
Comment 1 2014-02-21 09:11:10 PST
Created attachment 224877 [details] Patch V1
Dean Jackson
Comment 2 2014-02-22 16:06:03 PST
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
Comment 3 2014-02-24 01:03:15 PST
Created attachment 225034 [details] Patch for landing
WebKit Commit Bot
Comment 4 2014-02-24 01:15:23 PST
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
Comment 5 2014-02-24 01:25:13 PST
Created attachment 225042 [details] Patch for landing
WebKit Commit Bot
Comment 6 2014-02-24 01:56:49 PST
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 7 2014-02-24 10:57:04 PST
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
Comment 8 2014-02-25 01:10:37 PST
(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
Comment 9 2014-03-03 23:54:51 PST
(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
Comment 10 2014-03-21 08:02:50 PDT
Close this one as the follow up bug landed.
Note You need to log in before you can comment on or make changes to this bug.