Bug 129307 - [CSS Blending] Blending operation is not isolated when z-index on parent changes dynamically
Summary: [CSS Blending] Blending operation is not isolated when z-index on parent chan...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: AdobeTracked
: 129305 (view as bug list)
Depends on:
Blocks: 129661
  Show dependency treegraph
 
Reported: 2014-02-25 06:35 PST by Mihai Tica
Modified: 2014-03-20 10:37 PDT (History)
7 users (show)

See Also:


Attachments
Test case (893 bytes, text/html)
2014-02-25 06:38 PST, Mihai Tica
no flags Details
Not for review: reimplemented isolation with logic added in RenderLayerCompositor (9.19 KB, patch)
2014-02-28 06:45 PST, Mihai Tica
no flags Details | Formatted Diff | Diff
First try at isolation with descedantDependentFlag (23.50 KB, patch)
2014-03-06 11:32 PST, Mihai Tica
no flags Details | Formatted Diff | Diff
Patch (23.51 KB, patch)
2014-03-06 13:42 PST, Mihai Tica
no flags Details | Formatted Diff | Diff
Add guards in RenderLayer.cpp (23.59 KB, patch)
2014-03-07 07:06 PST, Mihai Tica
no flags Details | Formatted Diff | Diff
Patch (22.60 KB, patch)
2014-03-18 10:31 PDT, Mihai Tica
no flags Details | Formatted Diff | Diff
Patch (22.50 KB, patch)
2014-03-19 00:40 PDT, Mihai Tica
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Tica 2014-02-25 06:35:37 PST
Consider the following scenario:

parent element and child element having -webkit-mix-blend-mode other than normal.
Set z-index and position for the parent from Javascript. Blending is not isolated, ie the blend operation is performed with all the backdrop.
Instead, blending should only be performed with the contents of the parent element.
Comment 1 Mihai Tica 2014-02-25 06:38:36 PST
Created attachment 225147 [details]
Test case

Repaint test. Place it it fast/repaint
Comment 2 Mihai Tica 2014-02-28 01:01:40 PST
*** Bug 129305 has been marked as a duplicate of this bug. ***
Comment 3 Mihai Tica 2014-02-28 06:45:42 PST
Created attachment 225457 [details]
Not for review: reimplemented isolation with logic added in RenderLayerCompositor

This is a proof of concept patch.

--Implementation--
The implementation correctly handles all cases, however, it resides in RenderLayerCompositor.
An alternative implies adding blending as a descendant dependent flag, similar to https://codereview.chromium.org/23503046/diff/59001/Source/core/rendering/RenderLayer.cpp, however, going upwards provides better performance.

--Tests--
The first test test validates that the background of the body is fully painted, and m_isolatesBlending doesn't affect the root renderer.
The second tests validates that isolation is correctly handled. Note that there the third test-case fails due to https://bugs.webkit.org/show_bug.cgi?id=129480. This dependency should be fixed for all cases where RenderLayer.paintsWithTransparency should return true.
Comment 4 Mihai Tica 2014-03-03 11:46:01 PST
@Simon: could you please have a look over the last patch? Would this approach be ok?
If not, do you have any suggestions?
Comment 5 Simon Fraser (smfr) 2014-03-03 11:49:21 PST
Comment on attachment 225457 [details]
Not for review: reimplemented isolation with logic added in RenderLayerCompositor

Did you see if this logic fits into RenderLayer::updateDescendantDependentFlags()?
Comment 6 Mihai Tica 2014-03-04 09:41:55 PST
(In reply to comment #5)
> (From update of attachment 225457 [details])
> Did you see if this logic fits into RenderLayer::updateDescendantDependentFlags()?

I've investigated this and it looks like it would be feasible to use the updateDescendantDependentFlags logic.

Would using this be ok? I'm guessing that hooking here implies no additional traversing, thus being more performant than the implementation from my last patch.
Comment 7 Mihai Tica 2014-03-06 11:32:10 PST
Created attachment 226015 [details]
First try at isolation with descedantDependentFlag
Comment 8 Mihai Tica 2014-03-06 13:42:45 PST
Created attachment 226033 [details]
Patch
Comment 9 Mihai Tica 2014-03-07 07:06:15 PST
Created attachment 226124 [details]
Add guards in RenderLayer.cpp
Comment 10 Mihai Tica 2014-03-12 01:38:22 PDT
@Simon: Could you please review this? Thanks
Comment 11 Mihai Tica 2014-03-18 10:31:21 PDT
Created attachment 227071 [details]
Patch
Comment 12 WebKit Commit Bot 2014-03-18 10:33:17 PDT
Attachment 227071 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.cpp:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Mihai Tica 2014-03-19 00:40:14 PDT
Created attachment 227167 [details]
Patch
Comment 14 WebKit Commit Bot 2014-03-20 10:37:47 PDT
Comment on attachment 227167 [details]
Patch

Clearing flags on attachment: 227167

Committed r165970: <http://trac.webkit.org/changeset/165970>
Comment 15 WebKit Commit Bot 2014-03-20 10:37:52 PDT
All reviewed patches have been landed.  Closing bug.