Setting width of a flexitem causes the adjacent flex item to be displayed poorly.
authortony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2012 20:46:45 +0000 (20:46 +0000)
committertony@chromium.org <tony@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Oct 2012 20:46:45 +0000 (20:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=99925

Reviewed by Ojan Vafai.

Source/WebCore:

Make sure that we always repaint when moving a child. This is similar to what RenderDeprecatedFlexibleBox does.

Test: css3/flexbox/repaint-during-resize-no-flex.html

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::setFlowAwareLocationForChild): Move logic for repaining into the helper method
for setting the location of a child.
(WebCore::RenderFlexibleBox::layoutColumnReverse): Remove code for repaint since it's now in setFlowAwareLocationForChild.
(WebCore::RenderFlexibleBox::adjustAlignmentForChild): Remove code for repaint since it's now in setFlowAwareLocationForChild.

LayoutTests:

Add a repaint test.  The render tree should be cross platform, but due to
slight color differences in the grey overlay, the png can't be shared.

* css3/flexbox/repaint-during-resize-no-flex-expected.txt: Added.
* css3/flexbox/repaint-during-resize-no-flex.html: Added.
* platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png: Added.
* platform/chromium/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@132395 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/css3/flexbox/repaint-during-resize-no-flex-expected.txt [new file with mode: 0644]
LayoutTests/css3/flexbox/repaint-during-resize-no-flex.html [new file with mode: 0644]
LayoutTests/platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png [new file with mode: 0644]
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderFlexibleBox.cpp

index 4cf4759..ed44e43 100644 (file)
@@ -1,3 +1,18 @@
+2012-10-24  Tony Chang  <tony@chromium.org>
+
+        Setting width of a flexitem causes the adjacent flex item to be displayed poorly.
+        https://bugs.webkit.org/show_bug.cgi?id=99925
+
+        Reviewed by Ojan Vafai.
+
+        Add a repaint test.  The render tree should be cross platform, but due to
+        slight color differences in the grey overlay, the png can't be shared.
+
+        * css3/flexbox/repaint-during-resize-no-flex-expected.txt: Added.
+        * css3/flexbox/repaint-during-resize-no-flex.html: Added.
+        * platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png: Added.
+        * platform/chromium/TestExpectations:
+
 2012-10-24  Simon Fraser  <simon.fraser@apple.com>
 
         Fix CALayer hiearchy when combining tiling with preserve-3d
diff --git a/LayoutTests/css3/flexbox/repaint-during-resize-no-flex-expected.txt b/LayoutTests/css3/flexbox/repaint-during-resize-no-flex-expected.txt
new file mode 100644 (file)
index 0000000..18213fa
--- /dev/null
@@ -0,0 +1,9 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x90
+  RenderBlock {HTML} at (0,0) size 800x90
+    RenderBody {BODY} at (0,0) size 800x90
+      RenderFlexibleBox {DIV} at (0,0) size 800x90 [bgcolor=#333333]
+        RenderBlock {DIV} at (20,20) size 50x50 [bgcolor=#EEEEEE]
+        RenderBlock {DIV} at (110,20) size 10x50 [bgcolor=#EEEEEE]
+        RenderBlock {DIV} at (160,20) size 10x50 [bgcolor=#EEEEEE]
diff --git a/LayoutTests/css3/flexbox/repaint-during-resize-no-flex.html b/LayoutTests/css3/flexbox/repaint-during-resize-no-flex.html
new file mode 100644 (file)
index 0000000..6b1cbd9
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href="resources/flexbox.css">
+<style>
+body {
+    margin: 0;
+}
+.flexbox {
+    background: #333;
+}
+.flex-item {
+    height: 50px;
+    margin: 20px;
+    background: #eee;
+    width: 10px;
+}
+
+.width {
+    width:50px;
+}
+</style>
+</head>
+<body>
+<div class="flexbox">
+    <div id="flex-item-1" class="flex-item"></div>
+    <div class="flex-item"></div>
+    <div class="flex-item"></div>
+</div>
+<script>
+function resizeFlexItem() {
+    var div = document.getElementById("flex-item-1");
+    div.classList.add("width");
+    if (window.testRunner)
+        testRunner.notifyDone();
+};
+window.onload = function() {
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.display();
+    } else {
+        document.body.appendChild(document.createTextNode(
+            "Tests to make sure that when changing the size of one flex item changes the "
+            + "location of another flex item, we properly repaint. The repaint rect should "
+            + "include the three flex items."));
+    }
+    setTimeout(resizeFlexItem, 0);
+};
+</script>
+</body></html>
diff --git a/LayoutTests/platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png b/LayoutTests/platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png
new file mode 100644 (file)
index 0000000..ffc97eb
Binary files /dev/null and b/LayoutTests/platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png differ
index a8498f7..9054009 100644 (file)
@@ -4040,6 +4040,8 @@ webkit.org/b/99802 [ MountainLion ] fast/forms/text-control-intrinsic-widths.htm
 
 webkit.org/b/99873 platform/chromium/virtual/gpu/fast/canvas/webgl/array-bounds-clamping.html [ Text ]
 
+webkit.org/b/99925 [ Win Mac ] css3/flexbox/repaint-during-resize-no-flex.html [ Missing ImageOnlyFailure ]
+
 # These tests are failing already and because of virtual test suite for deferred
 # image decoding they need to be suppressed again.
 webkit.org/b/94240 platform/chromium/virtual/deferred/fast/images/animated-gif-restored-from-bfcache.html [ Failure ]
index 3011dd4..d345725 100644 (file)
@@ -1,3 +1,20 @@
+2012-10-24  Tony Chang  <tony@chromium.org>
+
+        Setting width of a flexitem causes the adjacent flex item to be displayed poorly.
+        https://bugs.webkit.org/show_bug.cgi?id=99925
+
+        Reviewed by Ojan Vafai.
+
+        Make sure that we always repaint when moving a child. This is similar to what RenderDeprecatedFlexibleBox does.
+
+        Test: css3/flexbox/repaint-during-resize-no-flex.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::setFlowAwareLocationForChild): Move logic for repaining into the helper method
+        for setting the location of a child.
+        (WebCore::RenderFlexibleBox::layoutColumnReverse): Remove code for repaint since it's now in setFlowAwareLocationForChild.
+        (WebCore::RenderFlexibleBox::adjustAlignmentForChild): Remove code for repaint since it's now in setFlowAwareLocationForChild.
+
 2012-10-24  Simon Fraser  <simon.fraser@apple.com>
 
         Fix CALayer hiearchy when combining tiling with preserve-3d
index ee425b3..e16b321 100644 (file)
@@ -661,10 +661,21 @@ LayoutPoint RenderFlexibleBox::flowAwareLocationForChild(RenderBox* child) const
 
 void RenderFlexibleBox::setFlowAwareLocationForChild(RenderBox* child, const LayoutPoint& location)
 {
+    LayoutRect oldFrameRect = child->frameRect();
+
     if (isHorizontalFlow())
         child->setLocation(location);
     else
         child->setLocation(location.transposedPoint());
+
+    // If the child moved, we have to repaint it as well as any floating/positioned
+    // descendants. An exception is if we need a layout. In this case, we know we're going to
+    // repaint ourselves (and the child) anyway.
+    // FIXME: In some cases, we might overpaint as we move a child multiple times. We could reduce
+    // overpainting by keeping track of the original position of a child and running this check on
+    // the final position.
+    if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
+        child->repaintDuringLayoutIfMoved(oldFrameRect);
 }
 
 LayoutUnit RenderFlexibleBox::mainAxisBorderAndPaddingExtentForChild(RenderBox* child) const
@@ -1142,10 +1153,7 @@ void RenderFlexibleBox::layoutColumnReverse(const OrderedFlexItemList& children,
         }
         mainAxisOffset -= mainAxisExtentForChild(child) + flowAwareMarginEndForChild(child);
 
-        LayoutRect oldRect = child->frameRect();
         setFlowAwareLocationForChild(child, LayoutPoint(mainAxisOffset, crossAxisOffset + flowAwareMarginBeforeForChild(child)));
-        if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
-            child->repaintDuringLayoutIfMoved(oldRect);
 
         mainAxisOffset -= flowAwareMarginStartForChild(child);
 
@@ -1216,14 +1224,7 @@ void RenderFlexibleBox::adjustAlignmentForChild(RenderBox* child, LayoutUnit del
         return;
     }
 
-    LayoutRect oldRect = child->frameRect();
     setFlowAwareLocationForChild(child, flowAwareLocationForChild(child) + LayoutSize(0, delta));
-
-    // If the child moved, we have to repaint it as well as any floating/positioned
-    // descendants. An exception is if we need a layout. In this case, we know we're going to
-    // repaint ourselves (and the child) anyway.
-    if (!selfNeedsLayout() && child->checkForRepaintDuringLayout())
-        child->repaintDuringLayoutIfMoved(oldRect);
 }
 
 void RenderFlexibleBox::alignChildren(OrderIterator& iterator, const WTF::Vector<LineContext>& lineContexts)