hasDescendantWithBlendMode should not be propagated outside the current stacking context.

hasDescendantWithBlendMode should not be propagated outside
the current stacking context.

If a stacking context element has descendants with blend mode, it should
create a new transparency layer, so that blending descendants will blend
only with the content within this stacking context (stacking context
will isolate blending).

With this CL, hasDescendantWithBlendMode will be set only if
the descendant is right inside the current stacking context, without
looking into nested stacking context elements. It also renames
hasDescendantWithBlendMode to hasNonIsolatedDescendantWithBlendMode,
which is more self-explanatory.

This change is necessary after https://codereview.chromium.org/466193002
(Disentangle blend mode from updateDescendantDependentFlags).

The "isolating" state used to be tested with DRT, but this property has
been removed with https://codereview.chromium.org/466943002/
(Remove isolatesBlending from render tree dumps).

We cannot test this change using layout tests. The effect is that
RenderLayer doesn't create any aditional and unnecessary skia
transparency layers.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183806

[abarth-chromium, 2014-08-18 17:54:40]

> We cannot test this change using layout tests. The effect is that
> RenderLayer doesn't create any aditional and unnecessary skia
> transparency layers.

Can we test this with a performance test?

We shouldn't have code that can't be tested.

[rosca, 2014-08-18 18:08:31]

On 2014/08/18 17:54:40, abarth wrote:
> > We cannot test this change using layout tests. The effect is that
> > RenderLayer doesn't create any aditional and unnecessary skia
> > transparency layers.
> 
> Can we test this with a performance test?
> 
> We shouldn't have code that can't be tested.

I agree, this is why I added the flag in DRT. However, creating unnecessary
transparency layers is not ok even if it brings no performance penalty. I'll
look more into what kind of tests we have in blink and how this could be tested.
If you have any hints, please let me know.

[abarth-chromium, 2014-08-18 18:10:59]

On 2014/08/18 at 18:08:31, rosca wrote:
> However, creating unnecessary transparency layers is not ok even if it brings
no performance penalty.

Why is that?

> I'll look more into what kind of tests we have in blink and how this could be
tested. If you have any hints, please let me know.

You first need to identify what problem it causes to create unnecessary
transparency layers.  Then we can test that the problem does not occur.

[rosca, 2014-09-24 11:18:08]

On 2014/08/18 18:10:59, abarth wrote:
> On 2014/08/18 at 18:08:31, rosca wrote:
> > I'll look more into what kind of tests we have in blink and how this could
be
> tested. If you have any hints, please let me know.
> 
> You first need to identify what problem it causes to create unnecessary
> transparency layers.  Then we can test that the problem does not occur.

Right. Creating many skia transparency layers will cause painting to be a lot
slower for deep trees.
I added a test in the PeformanceTests folders. This test creates a chain of
elements in DOM tree and only the leaf
has blending. The incorrect propagation of "hasDescendantWithBlendMode" flag
causes each element in this chain
to get its own transparency layer. Slow repaints for software animations
translates into a low fps rate.
The results are 7 fps without this patch vs 58 fps with this patch applied.

I created a new folder called Blending in PerformanceTests. I intend to add more
performance tests involving blending to this folder. 
After landing this patch, I plan to add blink_perf.blending target to
tools/perf/benchmarks/blink_perf.py in Chromium.

This is the first perftest I'm adding to blink and I'm not sure I did the right
thing. Please let me know if it's the case.

[rosca, 2014-09-30 17:48:54]

@abarth, please have a look at this when you have time. Thanks!

[rosca, 2014-10-14 20:16:09]

rosca@adobe.com changed reviewers:
+ eseidel@chromium.org

[rosca, 2014-10-15 19:38:21]

eseidel, could you review this patch? I couldn't catch abarth the last couple of
weeks and I would like to get it done.

[abarth-chromium, 2014-10-16 00:40:54]

The CQ bit was checked by abarth@chromium.org

[abarth-chromium, 2014-10-16 00:40:54]

lgtm

[commit-bot: I haz the power, 2014-10-16 00:41:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/478333002/20001

[commit-bot: I haz the power, 2014-10-16 00:41:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-16 00:41:50]

Failed to apply patch for Source/core/rendering/RenderLayer.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/rendering/RenderLayer.cpp
  Hunk #1 FAILED at 1169.
  Hunk #2 FAILED at 1771.
  2 out of 2 hunks FAILED -- saving rejects to file
Source/core/rendering/RenderLayer.cpp.rej

Patch:       Source/core/rendering/RenderLayer.cpp
Index: Source/core/rendering/RenderLayer.cpp
diff --git a/Source/core/rendering/RenderLayer.cpp
b/Source/core/rendering/RenderLayer.cpp
index
7628d593e95e929803d4a0be18b5258ac60935fe..60843e93830c1392ffd787fe7d5af29cd40a97b3
100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -1169,7 +1169,7 @@ LayoutRect RenderLayer::paintingExtent(const RenderLayer*
rootLayer, const Layou
 
 void RenderLayer::beginTransparencyLayers(GraphicsContext* context, const
RenderLayer* rootLayer, const LayoutRect& paintDirtyRect, const LayoutSize&
subPixelAccumulation, PaintBehavior paintBehavior)
 {
-    bool createTransparencyLayerForBlendMode =
m_stackingNode->isStackingContext() && hasDescendantWithBlendMode();
+    bool createTransparencyLayerForBlendMode =
m_stackingNode->isStackingContext() && hasNonIsolatedDescendantWithBlendMode();
     if ((paintsWithTransparency(paintBehavior) || paintsWithBlendMode() ||
createTransparencyLayerForBlendMode) && m_usedTransparency)
         return;
 
@@ -1771,7 +1771,7 @@ void RenderLayer::paintLayerContents(GraphicsContext*
context, const LayerPainti
 
     // Blending operations must be performed only with the nearest ancestor
stacking context.
     // Note that there is no need to create a transparency layer if we're
painting the root.
-    bool createTransparencyLayerForBlendMode = !renderer()->isDocumentElement()
&& m_stackingNode->isStackingContext() && hasDescendantWithBlendMode();
+    bool createTransparencyLayerForBlendMode = !renderer()->isDocumentElement()
&& m_stackingNode->isStackingContext() &&
hasNonIsolatedDescendantWithBlendMode();
 
     if (createTransparencyLayerForBlendMode)
         beginTransparencyLayers(context, paintingInfo.rootLayer,
paintingInfo.paintDirtyRect, paintingInfo.subPixelAccumulation,
paintingInfo.paintBehavior);

[rosca, 2014-10-16 09:57:41]

The CQ bit was checked by rosca@adobe.com

[commit-bot: I haz the power, 2014-10-16 09:58:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/478333002/40001

[commit-bot: I haz the power, 2014-10-16 12:04:02]

Committed patchset #3 (id:40001) as 183806