CTM-independent BackgroundBleedBackgroundOverBorder implementation

Source/core/paint/BoxDecorationData.cpp

Index: Source/core/paint/BoxDecorationData.cpp
diff --git a/Source/core/paint/BoxDecorationData.cpp b/Source/core/paint/BoxDecorationData.cpp
index 370311d2ceaaa95fa07f7340222ffa3a7895e294..66554ac46a3b4742e3e227ccefa1544b1a3b7004 100644
--- a/Source/core/paint/BoxDecorationData.cpp
+++ b/Source/core/paint/BoxDecorationData.cpp
@@ -38,13 +38,6 @@ BackgroundBleedAvoidance BoxDecorationData::determineBackgroundBleedAvoidance(co
         return BackgroundBleedNone;
     }
 
-    // If display lists are enabled (via Slimming Paint), then simply clip the background and do not
-    // perform advanced bleed-avoidance heuristics. These heuristics are not correct in the presence
-    // of impl-side rasterization or layerization, since the actual pixel-relative scaling and rotation
-    // of the content is not known to Blink.
-    if (RuntimeEnabledFeatures::slimmingPaintEnabled())
-        return BackgroundBleedClipBackground;
-
     // FIXME: See crbug.com/382491. getCTM does not accurately reflect the scale at the time content is
     // rasterized, and should not be relied on to make decisions about bleeding.
     AffineTransform ctm = context->getCTM();

[chrishtr, 2015/04/13 20:40:11]

Please factor this code into borderObscuresBackgroundEdge so it's not called if
RuntimeEnabledFeatures::slimmingPaintEnabled()

[f(malita), 2015/04/13 21:17:59]

Done.

@@ -64,8 +57,13 @@ BackgroundBleedAvoidance BoxDecorationData::determineBackgroundBleedAvoidance(co
     if (contextScaling.height() > 1)
         contextScaling.setHeight(1);
 
-    if (borderObscuresBackgroundEdge(*layoutBox.style(), contextScaling))
+    // If display lists are enabled (via Slimming Paint), then BackgroundBleedShrinkBackground is not
+    // usable as is relies on device-space heuristics. These heuristics are not correct in the presence
+    // of impl-side rasterization or layerization, since the actual pixel-relative scaling and rotation
+    // of the content is not known to Blink.
+    if (!RuntimeEnabledFeatures::slimmingPaintEnabled() && borderObscuresBackgroundEdge(*layoutBox.style(), contextScaling))

[chrishtr, 2015/04/13 20:40:11]

Why not just do it unconditionally? It's not slimming-paint specific.

[f(malita), 2015/04/13 21:17:59]

Not sure I follow here: BackgroundBleedShrinkBackground doesn't work well with
SP (and is also fragile with impl-side) because both the heuristic used to
determine whether it's usable and its impl are dependent on CTM knowledge.

If everything turns out as planned, I hope we can remove it when switching to SP
as there is no good way to make it work reliably.

         return BackgroundBleedShrinkBackground;
+
     if (!hasAppearance && layoutBox.style()->borderObscuresBackground() && layoutBox.backgroundHasOpaqueTopLayer())
         return BackgroundBleedBackgroundOverBorder;

[chrishtr, 2015/04/13 20:40:11]

I know it's not blocking slimming paint, but is it feasible to get rid of this
one too? If so add a TODO(fmalita)?

[f(malita), 2015/04/13 21:17:59]

(are you mixing these fine enum names maybe?)

This one will have to stay, because it's the only cheap bleed avoidance
workaround we have that is now SP/impl-side proof.

The remaining SP regressions are due to limitations of the
BackgroundBleedClipBackground fallback. I have a follow up to address that, but
it relies on using an extra layer - which is not cheap. The only way it can
stick is if BackgroundBleedClipBackground remains an uncommon fallback, with
most borders handled by BackgroundBleedBackgroundOverBorder.