Diff
Modified: trunk/LayoutTests/ChangeLog (153722 => 153723)
--- trunk/LayoutTests/ChangeLog 2013-08-05 21:17:41 UTC (rev 153722)
+++ trunk/LayoutTests/ChangeLog 2013-08-05 21:20:19 UTC (rev 153723)
@@ -1,5 +1,19 @@
2013-08-05 Morten Stenshorne <msten...@opera.com>
+ Region based columns not painted correctly in non-default writing-modes
+ https://bugs.webkit.org/show_bug.cgi?id=118506
+
+ Reviewed by David Hyatt.
+
+ * fast/multicol/newmulticol/hide-box-horizontal-bt-expected.html: Added.
+ * fast/multicol/newmulticol/hide-box-horizontal-bt.html: Added.
+ * fast/multicol/newmulticol/hide-box-vertical-lr-expected.html: Added.
+ * fast/multicol/newmulticol/hide-box-vertical-lr.html: Added.
+ * fast/multicol/newmulticol/hide-box-vertical-rl-expected.html: Added.
+ * fast/multicol/newmulticol/hide-box-vertical-rl.html: Added.
+
+2013-08-05 Morten Stenshorne <msten...@opera.com>
+
Region based columns not clipped properly
https://bugs.webkit.org/show_bug.cgi?id=118499
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt-expected.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt-expected.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt-expected.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, horizontal-bt</title>
+ <script>
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ </script>
+ </head>
+ <body>
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; background:lime;">
+ <div style="position:absolute; z-index:1; left:0; right:0; top:0; height:6em; background:white;"></div> <!-- make it fail if writing-mode isn't supported -->
+ <div style="width:20em; height:20em; -webkit-writing-mode:horizontal-bt;">
+ <div style="-webkit-columns:4; -webkit-column-fill:auto; -webkit-column-gap:0; columns:4; column-fill:auto; column-gap:0; orphans:1; widows:1; height:6em; line-height:2em; background:cyan;">
+ <br>
+ <br>
+ <br>
+ <br>
+ PASS
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-horizontal-bt.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, horizontal-bt</title>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ function scheduleHide() {
+ document.body.offsetLeft;
+ setTimeout("hideIt()", 100);
+ }
+ function hideIt() {
+ document.getElementById('hider').style.visibility = 'hidden';
+ document.body.offsetLeft;
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+ </head>
+ <body _onload_="scheduleHide()">
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; background:lime;">
+ <div id="hider" style="position:absolute; z-index:1; left:5.1em; bottom:2em; width:3em; height:3em; background:red;"></div>
+ <div style="position:absolute; z-index:1; left:0; right:0; top:0; height:6em; background:white;"></div> <!-- make it fail if writing-mode isn't supported -->
+ <div style="width:20em; height:20em; -webkit-writing-mode:horizontal-bt;">
+ <div style="-webkit-columns:4; -webkit-column-fill:auto; -webkit-column-gap:0; columns:4; column-fill:auto; column-gap:0; orphans:1; widows:1; height:6em; line-height:2em; background:cyan;">
+ <br>
+ <br>
+ <br>
+ <br>
+ <div style="position:relative;"> PASS</div>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr-expected.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr-expected.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr-expected.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, vertical-lr</title>
+ <script>
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ </script>
+ </head>
+ <body style="color:black; background:white;">
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; width:30em; background:black;">
+ <div style="width:20em; height:20em; -webkit-writing-mode:vertical-lr;">
+ <div style="-webkit-logical-height:6em; line-height:2em; background:cyan;">
+ <br>
+ <span style="-webkit-margin-start:5em;"> PASS</span>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-lr.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, vertical-lr</title>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ function scheduleHide() {
+ document.body.offsetLeft;
+ setTimeout("hideIt()", 100);
+ }
+ function hideIt() {
+ document.getElementById('hider').style.visibility = 'hidden';
+ document.body.offsetLeft;
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+ </head>
+ <body _onload_="scheduleHide()" style="color:black; background:white;">
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; width:30em; background:black;">
+ <div id="hider" style="position:absolute; z-index:1; top:5.1em; left:2em; width:3em; height:3em; background:red;"></div>
+ <div style="width:20em; height:20em; -webkit-writing-mode:vertical-lr;">
+ <div style="-webkit-columns:4; -webkit-column-fill:auto; -webkit-column-gap:0; columns:4; column-fill:auto; column-gap:0; orphans:1; widows:1; -webkit-logical-height:6em; line-height:2em; background:cyan;">
+ <br>
+ <br>
+ <br>
+ <br>
+ <div style="position:relative;"> PASS</div>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl-expected.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl-expected.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl-expected.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, vertical-rl</title>
+ <script>
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ </script>
+ </head>
+ <body style="color:black; background:white;">
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; width:30em; background:black;">
+ <div style="width:20em; height:20em; -webkit-writing-mode:vertical-rl;">
+ <div style="-webkit-logical-height:6em; line-height:2em; background:cyan;">
+ <br>
+ <span style="-webkit-margin-start:5em;"> PASS</span>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl.html (0 => 153723)
--- trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/hide-box-vertical-rl.html 2013-08-05 21:20:19 UTC (rev 153723)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <title>hiding a box covering a multicol, vertical-rl</title>
+ <script>
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+ if (window.internals)
+ internals.settings.setRegionBasedColumnsEnabled(true);
+ function scheduleHide() {
+ document.body.offsetLeft;
+ setTimeout("hideIt()", 100);
+ }
+ function hideIt() {
+ document.getElementById('hider').style.visibility = 'hidden';
+ document.body.offsetLeft;
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+ </head>
+ <body _onload_="scheduleHide()" style="color:black; background:white;">
+ <p>The word PASS should be seen below.</p>
+ <div style="position:relative; width:30em; background:black;">
+ <div id="hider" style="position:absolute; z-index:1; top:5.1em; right:12em; width:3em; height:3em; background:red;"></div>
+ <div style="width:20em; height:20em; -webkit-writing-mode:vertical-rl;">
+ <div style="-webkit-columns:4; -webkit-column-fill:auto; -webkit-column-gap:0; columns:4; column-fill:auto; column-gap:0; orphans:1; widows:1; -webkit-logical-height:6em; line-height:2em; background:cyan;">
+ <br>
+ <br>
+ <br>
+ <br>
+ <div style="position:relative;"> PASS</div>
+ </div>
+ </div>
+ </div>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (153722 => 153723)
--- trunk/Source/WebCore/ChangeLog 2013-08-05 21:17:41 UTC (rev 153722)
+++ trunk/Source/WebCore/ChangeLog 2013-08-05 21:20:19 UTC (rev 153723)
@@ -1,5 +1,24 @@
2013-08-05 Morten Stenshorne <msten...@opera.com>
+ Region based columns not painted correctly in non-default writing-modes
+ https://bugs.webkit.org/show_bug.cgi?id=118506
+
+ Reviewed by David Hyatt.
+
+ The column translation offset was calculated incorrectly.
+ The dirty rect intersection check was also wrong.
+
+ Added some documentation, to make it clear what's going on.
+
+ Tests: fast/multicol/newmulticol/hide-box-horizontal-bt.html
+ fast/multicol/newmulticol/hide-box-vertical-lr.html
+ fast/multicol/newmulticol/hide-box-vertical-rl.html
+
+ * rendering/RenderMultiColumnSet.cpp:
+ (WebCore::RenderMultiColumnSet::collectLayerFragments):
+
+2013-08-05 Morten Stenshorne <msten...@opera.com>
+
Region based columns not clipped properly
https://bugs.webkit.org/show_bug.cgi?id=118499
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp (153722 => 153723)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2013-08-05 21:17:41 UTC (rev 153722)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2013-08-05 21:20:19 UTC (rev 153723)
@@ -413,14 +413,36 @@
void RenderMultiColumnSet::collectLayerFragments(LayerFragments& fragments, const LayoutRect& layerBoundingBox, const LayoutRect& dirtyRect)
{
- // Put the layer bounds into flow thread-local coordinates by flipping it first.
+ // Let's start by introducing the different coordinate systems involved here. They are different
+ // in how they deal with writing modes and columns. RenderLayer rectangles tend to be more
+ // physical than the rectangles used in RenderObject & co.
+ //
+ // The two rectangles passed to this method are physical, except that we pretend that there's
+ // only one long column (that's the flow thread). They are relative to the top left corner of
+ // the flow thread. All rectangles being compared to the dirty rect also need to be in this
+ // coordinate system.
+ //
+ // Then there's the output from this method - the stuff we put into the list of fragments. The
+ // translationOffset point is the actual physical translation required to get from a location in
+ // the flow thread to a location in some column. The paginationClip rectangle is in the same
+ // coordinate system as the two rectangles passed to this method (i.e. physical, in flow thread
+ // coordinates, pretending that there's only one long column).
+ //
+ // All other rectangles in this method are slightly less physical, when it comes to how they are
+ // used with different writing modes, but they aren't really logical either. They are just like
+ // RenderBox::frameRect(). More precisely, the sizes are physical, and the inline direction
+ // coordinate is too, but the block direction coordinate is always "logical top". These
+ // rectangles also pretend that there's only one long column, i.e. they are for the flow thread.
+ //
+ // To sum up: input and output from this method are "physical" RenderLayer-style rectangles and
+ // points, while inside this method we mostly use the RenderObject-style rectangles (with the
+ // block direction coordinate always being logical top).
+
+ // Put the layer bounds into flow thread-local coordinates by flipping it first. Since we're in
+ // a renderer, most rectangles are represented this way.
LayoutRect layerBoundsInFlowThread(layerBoundingBox);
flowThread()->flipForWritingMode(layerBoundsInFlowThread);
- // Do the same for the dirty rect.
- LayoutRect dirtyRectInFlowThread(dirtyRect);
- flowThread()->flipForWritingMode(dirtyRectInFlowThread);
-
// Now we can compare with the flow thread portions owned by each column. First let's
// see if the rect intersects our flow thread portion at all.
LayoutRect clippedRect(layerBoundsInFlowThread);
@@ -472,11 +494,11 @@
// multicolumn block as well. This won't be an issue until we start creating multiple multicolumn sets.
// Shift the dirty rect to be in flow thread coordinates with this translation applied.
- LayoutRect translatedDirtyRect(dirtyRectInFlowThread);
+ LayoutRect translatedDirtyRect(dirtyRect);
translatedDirtyRect.moveBy(-translationOffset);
// See if we intersect the dirty rect.
- clippedRect = layerBoundsInFlowThread;
+ clippedRect = layerBoundingBox;
clippedRect.intersect(translatedDirtyRect);
if (clippedRect.isEmpty())
continue;
@@ -485,9 +507,10 @@
// offset and the clip rect for the column with that offset applied.
LayerFragment fragment;
fragment.paginationOffset = translationOffset;
-
+
LayoutRect flippedFlowThreadOverflowPortion(flowThreadOverflowPortion);
- flipForWritingMode(flippedFlowThreadOverflowPortion);
+ // Flip it into more a physical (RenderLayer-style) rectangle.
+ flowThread()->flipForWritingMode(flippedFlowThreadOverflowPortion);
fragment.paginationClip = flippedFlowThreadOverflowPortion;
fragments.append(fragment);
}
