[New Multicolumn] Make rounded corners clipping multicol-aware.

[New Multicolumn] Make rounded corners clipping multicol-aware.

R=chrishtr@chromium.org

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

[chrishtr, 2014-11-13 23:07:39]

https://codereview.chromium.org/721363002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.h:33: static void
applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&, GraphicsContext*,
const LayoutPoint& paginationOffset, PaintLayerFlags, ClipRecorder&,
BorderRadiusClippingRule = IncludeSelfForBorderRadius);
fragmentOffset instead of paginationOffset? Same comment applies to the naming
in LayerFragment.

[mstensho (USE GERRIT), 2014-11-14 08:18:48]

https://codereview.chromium.org/721363002/diff/1/Source/core/paint/LayerPaint...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/1/Source/core/paint/LayerPaint...
Source/core/paint/LayerPainter.h:33: static void
applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&, GraphicsContext*,
const LayoutPoint& paginationOffset, PaintLayerFlags, ClipRecorder&,
BorderRadiusClippingRule = IncludeSelfForBorderRadius);
On 2014/11/13 23:07:38, chrishtr wrote:
> fragmentOffset instead of paginationOffset? Same comment applies to the naming
> in LayerFragment.

Done.

Yeah, paginationOffset isn't a great name. I'd be tempting to call it
fragmentTranslation, or at least something with translation. Because that's what
it is. It's the translation required to get from flow thread coordinates to
visual coordinates. I'll use fragmentOffset here for now, and get back to the
big rename at a later time.

[chrishtr, 2014-11-14 20:18:21]

https://codereview.chromium.org/721363002/diff/20001/LayoutTests/fast/multico...
File LayoutTests/fast/multicol/border-radius-clipped-layer-expected.html
(right):

https://codereview.chromium.org/721363002/diff/20001/LayoutTests/fast/multico...
LayoutTests/fast/multicol/border-radius-clipped-layer-expected.html:8: first
column<br>
Can you avoid the br tag?

https://codereview.chromium.org/721363002/diff/20001/Source/core/paint/LayerP...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/20001/Source/core/paint/LayerP...
Source/core/paint/LayerPainter.h:33: static void
applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&, GraphicsContext*,
const LayoutPoint& fragmentOffset, PaintLayerFlags, ClipRecorder&,
BorderRadiusClippingRule = IncludeSelfForBorderRadius);
Please document what the fragmentOffset is from.

[mstensho (USE GERRIT), 2014-11-14 23:16:20]

https://codereview.chromium.org/721363002/diff/20001/LayoutTests/fast/multico...
File LayoutTests/fast/multicol/border-radius-clipped-layer-expected.html
(right):

https://codereview.chromium.org/721363002/diff/20001/LayoutTests/fast/multico...
LayoutTests/fast/multicol/border-radius-clipped-layer-expected.html:8: first
column<br>
On 2014/11/14 20:18:21, chrishtr wrote:
> Can you avoid the br tag?

I'd rather not. I want to control how many lines I get.

https://codereview.chromium.org/721363002/diff/20001/Source/core/paint/LayerP...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/20001/Source/core/paint/LayerP...
Source/core/paint/LayerPainter.h:33: static void
applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&, GraphicsContext*,
const LayoutPoint& fragmentOffset, PaintLayerFlags, ClipRecorder&,
BorderRadiusClippingRule = IncludeSelfForBorderRadius);
On 2014/11/14 20:18:21, chrishtr wrote:
> Please document what the fragmentOffset is from.

Done. Made me realize that the BorderRadiusClippingRule parameter is useless
now, but we can deal with that later (added FIXME).

[chrishtr, 2014-11-14 23:22:26]

https://codereview.chromium.org/721363002/diff/40001/Source/core/paint/LayerP...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/40001/Source/core/paint/LayerP...
Source/core/paint/LayerPainter.h:34: // Set rounded clip rectangles defined by
border radii all the way from the LayerPaintingInfo
Aside, unrelated to your CL: why are we setting it all the way from the root?
Why is the root not painting its own rounded corners before recursion? Don't
quite get it, do you?

[chrishtr, 2014-11-14 23:22:29]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2014-11-14 23:22:29]

lgtm

[commit-bot: I haz the power, 2014-11-14 23:23:49]

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

[commit-bot: I haz the power, 2014-11-14 23:24:02]

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

[commit-bot: I haz the power, 2014-11-14 23:24:03]

Failed to apply patch for Source/core/paint/LayerPainter.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/paint/LayerPainter.h
  Hunk #1 FAILED at 30.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/paint/LayerPainter.h.rej

Patch:       Source/core/paint/LayerPainter.h
Index: Source/core/paint/LayerPainter.h
diff --git a/Source/core/paint/LayerPainter.h b/Source/core/paint/LayerPainter.h
index
fae2c89d0581be795d36203ec145b04166aca746..efd63ed56f00f5c8e4371c33f7fb2ea49320b292
100644
--- a/Source/core/paint/LayerPainter.h
+++ b/Source/core/paint/LayerPainter.h
@@ -30,7 +30,16 @@ public:
     void paintOverlayScrollbars(GraphicsContext*, const LayoutRect& damageRect,
PaintBehavior, RenderObject* paintingRoot = 0);
 
     enum BorderRadiusClippingRule { IncludeSelfForBorderRadius,
DoNotIncludeSelfForBorderRadius };
-    static void applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&,
GraphicsContext*, PaintLayerFlags, ClipRecorder&, BorderRadiusClippingRule =
IncludeSelfForBorderRadius);
+
+    // Set rounded clip rectangles defined by border radii all the way from the
LayerPaintingInfo
+    // "root" layer down to the specified layer (or the parent of said layer,
in case
+    // BorderRadiusClippingRule says to skip self). fragmentOffset is used for
multicol, to specify
+    // the translation required to get from flow thread coordinates to visual
coordinates for a
+    // certain column.
+    // FIXME: The BorderRadiusClippingRule parameter is really useless now. If
we want to skip self,
+    // why not just supply the parent layer as the first parameter instead?
+    static void applyRoundedRectClips(RenderLayer&, const LayerPaintingInfo&,
GraphicsContext*, const LayoutPoint& fragmentOffset, PaintLayerFlags,
ClipRecorder&, BorderRadiusClippingRule = IncludeSelfForBorderRadius);
+
     static void beginTransparencyLayers(GraphicsContext*, RenderLayer&, const
RenderLayer* rootLayer, const LayoutRect& paintDirtyRect, const LayoutSize&
subPixelAccumulation, PaintBehavior);
 
 private:

[mstensho (USE GERRIT), 2014-11-14 23:29:22]

The CQ bit was checked by mstensho@opera.com

[commit-bot: I haz the power, 2014-11-14 23:30:43]

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

[mstensho (USE GERRIT), 2014-11-14 23:36:58]

https://codereview.chromium.org/721363002/diff/40001/Source/core/paint/LayerP...
File Source/core/paint/LayerPainter.h (right):

https://codereview.chromium.org/721363002/diff/40001/Source/core/paint/LayerP...
Source/core/paint/LayerPainter.h:34: // Set rounded clip rectangles defined by
border radii all the way from the LayerPaintingInfo
On 2014/11/14 23:22:26, chrishtr wrote:
> Aside, unrelated to your CL: why are we setting it all the way from the root?
> Why is the root not painting its own rounded corners before recursion? Don't
> quite get it, do you?

Both rectangular and rounded clip rectangles are applied all the way from the
"root" [1] for all layers, I think. The reason has to be that different
descendant layers are affected differently by ancestors' clipping. E.g. an
absolutely positioned box "CHILD1" inside a statically positioned box "PARENT"
with overflow:hidden isn't clipped by "PARENT", while a relatively positioned
"CHILD2" of the same "PARENT" will get clipped by it.

[1] "root" is not root when we transform. Then "root" becomes the transformed
layer.

Note that this it not about actually painting the borders. That's done locally
on every layer. This is only about clipping along the inner edges of the borders
when painting child content.

[commit-bot: I haz the power, 2014-11-15 00:42:26]

Committed patchset #4 (id:60001) as 185399