CTM-independent BackgroundBleedBackgroundOverBorder implementation

Source/core/paint/BoxDecorationData.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/paint/BoxDecorationData.h"
 
 #include "core/layout/LayoutBox.h"
 #include "core/style/BorderEdge.h"
 #include "core/style/ComputedStyle.h"
@@ skipping 20 matching lines @@
 
     if (!hasBackground)
         return BackgroundBleedNone;
 
     if (!hasBorder || !layoutBox.style()->hasBorderRadius() || layoutBox.canRenderBorderImage()) {
         if (layoutBox.backgroundShouldAlwaysBeClipped())
             return BackgroundBleedClipBackground;
         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.

     FloatSize contextScaling(static_cast<float>(ctm.xScale()), static_cast<float>(ctm.yScale()));
 
     // Because RoundedRect uses IntRect internally the inset applied by the
     // BackgroundBleedShrinkBackground strategy cannot be less than one integer
     // layout coordinate, even with subpixel layout enabled. To take that into
     // account, we clamp the contextScaling to 1.0 for the following test so
     // that borderObscuresBackgroundEdge can only return true if the border
     // widths are greater than 2 in both layout coordinates and screen
     // coordinates.
     // This precaution will become obsolete if RoundedRect is ever promoted to
     // a sub-pixel representation.
     if (contextScaling.width() > 1)
         contextScaling.setWidth(1);
     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.

 
     return BackgroundBleedClipBackground;
 }
 
 bool BoxDecorationData::borderObscuresBackgroundEdge(const ComputedStyle& style, const FloatSize& contextScale) const
 {
     BorderEdge edges[4];
     style.getBorderEdgeInfo(edges);
 
     for (int i = BSTop; i <= BSLeft; ++i) {
         const BorderEdge& currEdge = edges[i];
         // FIXME: for vertical text
         float axisScale = (i == BSTop || i == BSBottom) ? contextScale.height() : contextScale.width();
         if (!currEdge.obscuresBackgroundEdge(axisScale))
             return false;
     }
 
     return true;
 }
 
 } // namespace blink