[New Multicolumn] Handle flow thread overflow correctly in flipped writing modes.

[New Multicolumn] Handle flow thread overflow correctly in flipped writing modes.

LayoutFlowThread::fragmentsBoundingBox() works on physical rectangles. Document
this fact and make sure that the callers do use physical rectangles, instead of
having the block progression coordinate flipped in vertical-rl and
horizontal-bt writing modes.

BUG=457185
R=dsinclair@chromium.org,jchaffraix@chromium.org

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

[dsinclair, 2015-02-18 21:00:29]

https://codereview.chromium.org/933043005/diff/1/Source/core/layout/LayoutFlo...
File Source/core/layout/LayoutFlowThread.cpp (right):

https://codereview.chromium.org/933043005/diff/1/Source/core/layout/LayoutFlo...
Source/core/layout/LayoutFlowThread.cpp:104: flipForWritingMode(rect);
I'm not sure I understand why this works. When we call flipWritingMode I believe
we end up in RenderBox.h::flipWritingMode(LayoutRect& rect). Looking at the
code, it will modify the X or Y value of the rect, but it doesn't change the
setting of the writing mode. So, when we call back in, won't we just adjust the
value a second time in exactly the same way, or, more likely, what did I miss?

[mstensho (USE GERRIT), 2015-02-18 22:25:41]

https://codereview.chromium.org/933043005/diff/1/Source/core/layout/LayoutFlo...
File Source/core/layout/LayoutFlowThread.cpp (right):

https://codereview.chromium.org/933043005/diff/1/Source/core/layout/LayoutFlo...
Source/core/layout/LayoutFlowThread.cpp:104: flipForWritingMode(rect);
On 2015/02/18 21:00:29, dsinclair wrote:
> I'm not sure I understand why this works. When we call flipWritingMode I
believe
> we end up in RenderBox.h::flipWritingMode(LayoutRect& rect). Looking at the
> code, it will modify the X or Y value of the rect, but it doesn't change the
> setting of the writing mode. So, when we call back in, won't we just adjust
the
> value a second time in exactly the same way, or, more likely, what did I miss?

Yes, except that you have a fragmentsBoundingBox() call in the middle, which
does something to the rectangle. :)

flipForWritingMode() converts between layout rectangles and physical rectangles.
If the input is physical, it becomes a layout rectangle. If the input is a
layout rectangle, it becomes physical. It doesn't really care what the input is,
since in either case, all it needs to do is flip one coordinate (and even that
only if writing mode is vertical-rl or horizontal-bt).

In most of the layout code (e.g. RenderBox::m_frameRect or the rectangles in
RenderOverflow) the rectangles are _almost_ physical, but not quite (so we don't
call them physical): the block progression coordinate (y in horizontal writing
modes, x in vertical writing modes) is always the "logical top" - i.e. the block
coordinate where it starts in its container. If e.g. blocks go from right to
left, the x coordinate will represent the right edge of the box, relatively to
the right edge of its container. This is what we need to flip (to be physical,
we need x to represent the left edge of the box relatively to the left edge of
its container).

In other parts of the code (e.g. most of Layer, and when you get close to
actually painting something), rectangles are physical, so that they represent
what you're actually going to see on the screen.

So we use flipForWritingMode() to get from one rectangle type to the other.

mapRectToPaintInvalidationBacking() works with layout rectangles, while
fragmentsBoundingBox() works with physical ones, so we need to:

1. Convert the rectangle to physical
2. Let fragmentsBoundingBox() do its job on the physical rectangle (if you care
to know, it will convert from a physical flow thread rectangle to a physical
visual multicol rectangle. [1])
3. Convert the resulting rectangle to a layout rectangle, since that's what
mapRectToPaintInvalidationBacking() uses.

[1] I've attempted to explain the difference between flow thread and visual
coordinates/rectangles here:
http://www.chromium.org/developers/design-documents/multi-column-layout

[dsinclair, 2015-02-19 15:48:05]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2015-02-19 15:48:06]

lgtm. Thanks a lot for the explanation.

[commit-bot: I haz the power, 2015-02-19 15:51:31]

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

[commit-bot: I haz the power, 2015-02-19 16:06:00]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190482