Bug 129154

Summary:

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

Product:

WebKit

Reporter:

Mihai Tica <mitica>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dino, esprehn+autocc, glenn, kondapallykalyan, mihnea, simon.fraser, WebkitBugTracker

Priority:

Keywords:

AdobeTracked

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

95614

Attachments:

Description	Flags
Patch V1	dino: review+
Patch for landing	commit-queue: commit-queue-
Patch for landing	none

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+	Details Formatted Diff Diff
Patch for landing (69.63 KB, patch) 2014-02-24 01:03 PST, Mihai Tica	commit-queue: commit-queue-	Details Formatted Diff Diff
Patch for landing (69.63 KB, patch) 2014-02-24 01:25 PST, Mihai Tica	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

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.