Changeset 142638 in webkit


Ignore:
Timestamp:
Feb 12, 2013 9:40:28 AM (11 years ago)
Author:
eae@chromium.org
Message:

TransformState::move should not round offset to int
​https://bugs.webkit.org/show_bug.cgi?id=108266

Source/WebCore:

Reviewed by Simon Fraser.

Currently TransformState::move rounds the offset to the nearest
integer values, this results in operations using TransformState
to compute a position to misreport the location, specifically
Element:getBoundingClientRect and repaint rects. Sizes are
handled correctly and do not have the same problem.

Tests: fast/sub-pixel/boundingclientrect-subpixel-margin.html

fast/sub-pixel/clip-rect-box-consistent-rounding.html

  • page/FrameView.cpp:

(WebCore::FrameView::convertFromRenderer):
Change to use pixel snapping instead of enclosing box. All other
code paths use pixelSnappedIntRect to align the rects to device
pixels however this used enclosingIntRect (indirectly through
the FloatQuad::enclosingBoundingBox call).
Without the rounding in TransformState this causes repaint rects
for elements on subpixel bounds to be too large by up to one
pixel on each axis. For normal repaints this isn't really a
problem but in scrollContentsSlowPath it can result in moving
too large a rect.

  • platform/graphics/transforms/TransformState.cpp:

(WebCore::TransformState::translateTransform):
(WebCore::TransformState::translateMappedCoordinates):
Change to take a LayoutSize instead of an IntSize.

(WebCore::TransformState::move):
(WebCore::TransformState::applyAccumulatedOffset):

  • platform/graphics/transforms/TransformState.h:

Remove rounding logic and use original, more precise, value.

  • rendering/RenderGeometryMap.cpp:

(WebCore::RenderGeometryMap::mapToContainer):
Remove rounding logic and use original, more precise, value.

LayoutTests:

Reviewed by Simon Fraser.

Add new tests for Element::boundingClientRect and clip rects for
elements on subpixel boundaries.

  • fast/dom/Window/webkitConvertPoint.html:
  • platform/chromium-linux/fast/dom/Window/webkitConvertPoint-expected.txt:

Update test and expectations to take new rounding into account.

  • fast/sub-pixel/boundingclientrect-subpixel-margin-expected.txt: Added.
  • fast/sub-pixel/boundingclientrect-subpixel-margin.html: Added.

Add test ensuring that boundingClientRect returns accurate and
precise (as opposed to rounded) metrics.

  • fast/sub-pixel/clip-rect-box-consistent-rounding-expected.html: Added.
  • fast/sub-pixel/clip-rect-box-consistent-rounding.html: Added.

Add test ensuring that clip rects and elements use consistent rounding.

Location:
trunk
Files:
4 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r142635 r142638  
     12013-02-12  Emil A Eklund  <eae@chromium.org>
     2
     3        TransformState::move should not round offset to int
     4        https://bugs.webkit.org/show_bug.cgi?id=108266
     5
     6        Reviewed by Simon Fraser.
     7       
     8        Add new tests for Element::boundingClientRect and clip rects for
     9        elements on subpixel boundaries.
     10
     11        * fast/dom/Window/webkitConvertPoint.html:
     12        * platform/chromium-linux/fast/dom/Window/webkitConvertPoint-expected.txt:
     13        Update test and expectations to take new rounding into account.
     14       
     15        * fast/sub-pixel/boundingclientrect-subpixel-margin-expected.txt: Added.
     16        * fast/sub-pixel/boundingclientrect-subpixel-margin.html: Added.
     17        Add test ensuring that boundingClientRect returns accurate and
     18        precise (as opposed to rounded) metrics.
     19       
     20        * fast/sub-pixel/clip-rect-box-consistent-rounding-expected.html: Added.
     21        * fast/sub-pixel/clip-rect-box-consistent-rounding.html: Added.
     22        Add test ensuring that clip rects and elements use consistent rounding.
     23
     24
    1252013-02-12  Rafael Weinstein  <rafaelw@chromium.org>
    226

  • trunk/LayoutTests/fast/dom/Window/webkitConvertPoint.html

    r137847 r142638  
    146146                runTest("Test 9",  "test9",  28, 291, 33, 331);
    147147                runTest("Test 10", "test10", 28, 309, 33, 349);
    148                 runTest("Test 11", "test11", 158, 356, 174, 374);
    149                 runTest("Test 12", "test12", 168, 429, 184, 447);
     148                runTest("Test 11", "test11", 158, 376, 174, 394);
     149                runTest("Test 12", "test12", 168, 451, 184, 469);
    150150                runTest("Test 13", "test13", 28, 487, 33, 527);
    151151            } else {

  • trunk/LayoutTests/platform/chromium-linux/fast/dom/Window/webkitConvertPoint-expected.txt

    r138179 r142638  
    159159Test 11
    160160PASS x is 158
    161 FAIL y should be 356. Was 377.
     161PASS y is 376
    162162Round Trip of (0,0)
    163163PASS x is 0
    164164PASS y is 0
    165165PASS x is 174
    166 FAIL y should be 374. Was 395.
     166PASS y is 394
    167167Round Trip of (5,40)
    168168PASS x is 5
    … …  
    171171Test 12
    172172PASS x is 168
    173 FAIL y should be 429. Was 452.
     173PASS y is 451
    174174Round Trip of (0,0)
    175175PASS x is 0
    176176PASS y is 0
    177177PASS x is 184
    178 FAIL y should be 447. Was 470.
     178PASS y is 469
    179179Round Trip of (5,40)
    180180PASS x is 5

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r142619 r142638  
    955955webkit.org/b/109272 platform/chromium/fast/forms/suggestion-picker/datetime-suggestion-picker-mouse-operations.html [ Skip ]
    956956
     957# Skip until 109528 is fixed.
     958webkit.org/b/109528 platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar.html [ Skip ]
     959webkit.org/b/109528 platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar.html [ Skip ]
     960webkit.org/b/109528 platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar.html [ Skip ]
     961
    957962# Skipping rules for ANDROID are in platform/chromium-android/TestExpectations.
    958963

  • trunk/Source/WebCore/ChangeLog

    r142637 r142638  
     12013-02-12  Emil A Eklund  <eae@chromium.org>
     2
     3        TransformState::move should not round offset to int
     4        https://bugs.webkit.org/show_bug.cgi?id=108266
     5
     6        Reviewed by Simon Fraser.
     7       
     8        Currently TransformState::move rounds the offset to the nearest
     9        integer values, this results in operations using TransformState
     10        to compute a position to misreport the location, specifically
     11        Element:getBoundingClientRect and repaint rects. Sizes are
     12        handled correctly and do not have the same problem.
     13
     14        Tests: fast/sub-pixel/boundingclientrect-subpixel-margin.html
     15               fast/sub-pixel/clip-rect-box-consistent-rounding.html
     16
     17        * page/FrameView.cpp:
     18        (WebCore::FrameView::convertFromRenderer):
     19        Change to use pixel snapping instead of enclosing box. All other
     20        code paths use pixelSnappedIntRect to align the rects to device
     21        pixels however this used enclosingIntRect (indirectly through
     22        the FloatQuad::enclosingBoundingBox call).
     23        Without the rounding in TransformState this causes repaint rects
     24        for elements on subpixel bounds to be too large by up to one
     25        pixel on each axis. For normal repaints this isn't really a
     26        problem but in scrollContentsSlowPath it can result in moving
     27        too large a rect.
     28
     29        * platform/graphics/transforms/TransformState.cpp:
     30        (WebCore::TransformState::translateTransform):
     31        (WebCore::TransformState::translateMappedCoordinates):
     32        Change to take a LayoutSize instead of an IntSize.
     33
     34        (WebCore::TransformState::move):
     35        (WebCore::TransformState::applyAccumulatedOffset):
     36        * platform/graphics/transforms/TransformState.h:
     37        Remove rounding logic and use original, more precise, value.
     38
     39        * rendering/RenderGeometryMap.cpp:
     40        (WebCore::RenderGeometryMap::mapToContainer):
     41        Remove rounding logic and use original, more precise, value.
     42
    1432013-02-12  Jessie Berlin  <jberlin@apple.com>
    244

  • trunk/Source/WebCore/page/FrameView.cpp

    r142560 r142638  
    35883588IntRect FrameView::convertFromRenderer(const RenderObject* renderer, const IntRect& rendererRect) const
    35893589{
    3590     IntRect rect = renderer->localToAbsoluteQuad(FloatRect(rendererRect)).enclosingBoundingBox();
     3590    IntRect rect = pixelSnappedIntRect(enclosingLayoutRect(renderer->localToAbsoluteQuad(FloatRect(rendererRect)).boundingBox()));
    35913591
    35923592    // Convert from page ("absolute") to FrameView coordinates.

  • trunk/Source/WebCore/platform/graphics/transforms/TransformState.cpp

    r139479 r142638  
    5151}
    5252
    53 void TransformState::translateTransform(const IntSize& offset)
     53void TransformState::translateTransform(const LayoutSize& offset)
    5454{
    5555    if (m_direction == ApplyTransformDirection)
    … …  
    5959}
    6060
    61 void TransformState::translateMappedCoordinates(const IntSize& offset)
    62 {
    63     IntSize adjustedOffset = (m_direction == ApplyTransformDirection) ? offset : -offset;
     61void TransformState::translateMappedCoordinates(const LayoutSize& offset)
     62{
     63    LayoutSize adjustedOffset = (m_direction == ApplyTransformDirection) ? offset : -offset;
    6464    if (m_mapPoint)
    6565        m_lastPlanarPoint.move(adjustedOffset);
    … …  
    7676        if (m_accumulatingTransform && m_accumulatedTransform) {
    7777            // If we're accumulating into an existing transform, apply the translation.
    78             translateTransform(roundedIntSize(offset));
     78            translateTransform(offset);
    7979
    8080            // Then flatten if necessary.
    … …  
    8383        } else
    8484            // Just move the point and/or quad.
    85             translateMappedCoordinates(roundedIntSize(offset));
     85            translateMappedCoordinates(offset);
    8686    }
    8787    m_accumulatingTransform = accumulate == AccumulateTransform;
    … …  
    9090void TransformState::applyAccumulatedOffset()
    9191{
    92     IntSize offset = roundedIntSize(m_accumulatedOffset);
     92    LayoutSize offset = m_accumulatedOffset;
    9393    m_accumulatedOffset = LayoutSize();
    9494    if (!offset.isZero()) {

  • trunk/Source/WebCore/platform/graphics/transforms/TransformState.h

    r139479 r142638  
    102102
    103103private:
    104     void translateTransform(const IntSize&);
    105     void translateMappedCoordinates(const IntSize&);
     104    void translateTransform(const LayoutSize&);
     105    void translateMappedCoordinates(const LayoutSize&);
    106106    void flattenWithTransform(const TransformationMatrix&, bool* wasClamped);
    107107    void applyAccumulatedOffset();

  • trunk/Source/WebCore/rendering/RenderGeometryMap.cpp

    r137847 r142638  
    129129    if (!hasFixedPositionStep() && !hasTransformStep() && !hasNonUniformStep() && (!container || (m_mapping.size() && container == m_mapping[0].m_renderer))) {
    130130        result = rect;
    131         result.move(roundedIntSize(m_accumulatedOffset));
     131        result.move(m_accumulatedOffset);
    132132    } else {
    133133        TransformState transformState(TransformState::ApplyTransformDirection, rect.center(), rect);
Note: See TracChangeset for help on using the changeset viewer.