Changeset 132395 in webkit

Timestamp:

Oct 24, 2012 1:46:45 PM (12 years ago)

Author:

tony@chromium.org

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/repaint-during-resize-no-flex-expected.txt (added)
• LayoutTests/css3/flexbox/repaint-during-resize-no-flex.html (added)
• LayoutTests/platform/chromium-linux/css3/flexbox/repaint-during-resize-no-flex-expected.png (added)
• LayoutTests/platform/chromium/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/chromium/TestExpectations

@@ -4041 +4041 @@
 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.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -662 +662 @@
 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);
 }
 
@@ -1143 +1154 @@
         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);
@@ -1217 +1225 @@
     }
 
-    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);
 }