Changeset 101903 in webkit


Ignore:
Timestamp:
Dec 2, 2011 9:34:42 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

REGRESSION (r91125): Polyline tool in google docs is broken
​https://bugs.webkit.org/show_bug.cgi?id=65796

Source/WebCore:

Patch by Stephen Chenney <​schenney@chromium.org> on 2011-12-02
Reviewed by Darin Adler.

Work around a bug in CoreGraphics, that caused incorrect bounds for paths
consisting only of move-to elements. This causes problems in SVG, when the enormous
bounds prevented the drawing of things behind.

Tests: svg/custom/path-moveto-only-rendering.svg

svg/custom/subpaths-moveto-only-rendering.svg

  • platform/graphics/cg/PathCG.cpp:

(WebCore::PathIsEmptyOrSingleMoveTester::PathIsEmptyOrSingleMoveTester): Class to
test for isEmpty accoridng ot the same rules as other platforms.
(WebCore::PathIsEmptyOrSingleMoveTester::isEmpty): Query the result
(WebCore::PathIsEmptyOrSingleMoveTester::testPathElement): Path iterator method
(WebCore::PathHasOnlyMoveToTester::PathHasOnlyMoveToTester): Class to test whether a
path contains only move-to elements, and hence should have null bounds.
(WebCore::PathHasOnlyMoveToTester::hasOnlyMoveTo): Query the result
(WebCore::PathHasOnlyMoveToTester::testPathElement): Path iterator method.
(WebCore::Path::boundingRect): Modified to check for move-to only paths
(WebCore::Path::fastBoundingRect): Modified to check for move-to only paths
(WebCore::Path::isEmpty): Now uses the method that matches other platforms.
(WebCore::Path::hasCurrentPoint): Now uses CGPathIsEmpty directly
(WebCore::Path::transform) : Now uses CGPathIsEmpty directly

LayoutTests:

Work around a bug in CoreGraphics, that caused incorrect bounds for paths
consisting only of move-to elements. This causes problems in SVG, when the enormous
bounds prevented the drawing of things behind.

Will revert expectation file when expectations are stable.

Patch by Stephen Chenney <​schenney@chromium.org> on 2011-12-02
Reviewed by Darin Adler.

  • platform/chromium-mac/svg/custom/zero-path-square-cap-rendering2-expected.txt: Removed.
  • platform/chromium-win/svg/custom/zero-path-square-cap-rendering2-expected.txt: Removed.
  • platform/mac/svg/custom/path-moveto-only-rendering-expected.png: Added.
  • platform/mac/svg/custom/path-moveto-only-rendering-expected.txt: Added.
  • platform/mac/svg/custom/subpaths-moveto-only-rendering-expected.png: Added.
  • platform/mac/svg/custom/subpaths-moveto-only-rendering-expected.txt: Added.
  • svg/custom/path-moveto-only-rendering.svg: Added.
  • svg/custom/subpaths-moveto-only-rendering.svg: Added.
  • svg/custom/zero-path-square-cap-rendering2-expected.txt: Modified text output
  • platform/chromium/test_expectations.txt: Added flakiness for new tests from this patch
Location:
trunk
Files:
6 added
2 deleted
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r101900 r101903  
     12011-12-02  Stephen Chenney  <schenney@chromium.org>
     2
     3        REGRESSION (r91125): Polyline tool in google docs is broken
     4        https://bugs.webkit.org/show_bug.cgi?id=65796
     5
     6        Work around a bug in CoreGraphics, that caused incorrect bounds for paths
     7        consisting only of move-to elements. This causes problems in SVG, when the enormous
     8        bounds prevented the drawing of things behind.
     9
     10        Will revert expectation file when expectations are stable.
     11
     12        Reviewed by Darin Adler.
     13
     14        * platform/chromium-mac/svg/custom/zero-path-square-cap-rendering2-expected.txt: Removed.
     15        * platform/chromium-win/svg/custom/zero-path-square-cap-rendering2-expected.txt: Removed.
     16        * platform/mac/svg/custom/path-moveto-only-rendering-expected.png: Added.
     17        * platform/mac/svg/custom/path-moveto-only-rendering-expected.txt: Added.
     18        * platform/mac/svg/custom/subpaths-moveto-only-rendering-expected.png: Added.
     19        * platform/mac/svg/custom/subpaths-moveto-only-rendering-expected.txt: Added.
     20        * svg/custom/path-moveto-only-rendering.svg: Added.
     21        * svg/custom/subpaths-moveto-only-rendering.svg: Added.
     22        * svg/custom/zero-path-square-cap-rendering2-expected.txt: Modified text output
     23        * platform/chromium/test_expectations.txt: Added flakiness for new tests from this patch
     24
    1252011-12-02  Vincent Scheib  <scheib@chromium.org>
    226

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r101900 r101903  
    38193819BUGWK73494 DEBUG : svg/hixie/perf/007.xml = PASS TIMEOUT
    38203820
     3821BUGWK73587 : svg/custom/path-moveto-only-rendering.svg = PASS IMAGE
     3822BUGWK73587 : svg/custom/subpaths-moveto-only-rendering.svg = PASS IMAGE
     3823
    38213824BUGWK73514 WIN DEBUG : platform/chromium/compositing/lost-compositor-context-permanently.html = TIMEOUT
    38223825BUGWK73517 WIN DEBUG : inspector/console/console-command-clear.html = CRASH

  • trunk/LayoutTests/svg/custom/zero-path-square-cap-rendering2-expected.txt

    r101816 r101903  
    55    RenderSVGContainer {g} at (0,165) size 134x122 [transform={m=((1.09,0.00)(0.00,1.09)) t=(0.00,0.00)}]
    66      RenderSVGPath {path} at (0,165) size 134x122 [fill={[type=SOLID] [color=#008000]}] [data="M 0 39403 L 103176 39403 L 103176 132091 L 0 132091 Z"]
    7       RenderSVGPath {path} at (-2147483648,-2147483648) size -2147483648x-2147483648 [fill={[type=SOLID] [color=#000000]}] [data="M 0 0"]
     7      RenderSVGPath {path} at (0,0) size 0x0 [fill={[type=SOLID] [color=#000000]}] [data="M 0 0"]

  • trunk/Source/WebCore/ChangeLog

    r101899 r101903  
     12011-12-02  Stephen Chenney  <schenney@chromium.org>
     2
     3        REGRESSION (r91125): Polyline tool in google docs is broken
     4        https://bugs.webkit.org/show_bug.cgi?id=65796
     5
     6        Reviewed by Darin Adler.
     7
     8        Work around a bug in CoreGraphics, that caused incorrect bounds for paths
     9        consisting only of move-to elements. This causes problems in SVG, when the enormous
     10        bounds prevented the drawing of things behind.
     11
     12        Tests: svg/custom/path-moveto-only-rendering.svg
     13               svg/custom/subpaths-moveto-only-rendering.svg
     14
     15        * platform/graphics/cg/PathCG.cpp:
     16        (WebCore::PathIsEmptyOrSingleMoveTester::PathIsEmptyOrSingleMoveTester): Class to
     17        test for isEmpty accoridng ot the same rules as other platforms.
     18        (WebCore::PathIsEmptyOrSingleMoveTester::isEmpty): Query the result
     19        (WebCore::PathIsEmptyOrSingleMoveTester::testPathElement): Path iterator method
     20        (WebCore::PathHasOnlyMoveToTester::PathHasOnlyMoveToTester): Class to test whether a
     21        path contains only move-to elements, and hence should have null bounds.
     22        (WebCore::PathHasOnlyMoveToTester::hasOnlyMoveTo): Query the result
     23        (WebCore::PathHasOnlyMoveToTester::testPathElement): Path iterator method.
     24        (WebCore::Path::boundingRect): Modified to check for move-to only paths
     25        (WebCore::Path::fastBoundingRect): Modified to check for move-to only paths
     26        (WebCore::Path::isEmpty): Now uses the method that matches other platforms.
     27        (WebCore::Path::hasCurrentPoint): Now uses CGPathIsEmpty directly
     28        (WebCore::Path::transform) : Now uses CGPathIsEmpty directly
     29
    1302011-12-02  Mihnea Ovidenie  <mihnea@adobe.com>
    231

  • trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp

    r101816 r101903  
    4242namespace WebCore {
    4343
     44// A class to provide an isEmpty test that considers a one-element path with only a MoveTo element
     45// to be empty. This behavior is consistent with other platforms in WebKit, and is needed to prevent
     46// incorrect (according to the spec) linecap stroking for zero length paths in SVG.
     47class PathIsEmptyOrSingleMoveTester {
     48public:
     49    PathIsEmptyOrSingleMoveTester() : m_moveCount(0) { }
     50
     51    bool isEmpty() const
     52    {
     53        return m_moveCount <= 1;
     54    }
     55
     56    static void testPathElement(void* info, const CGPathElement* element)
     57    {
     58        PathIsEmptyOrSingleMoveTester* tester = static_cast<PathIsEmptyOrSingleMoveTester*>(info);
     59        if (element->type == kCGPathElementMoveToPoint)
     60            ++tester->m_moveCount;
     61        else {
     62            // Any non move element implies a non-empty path; set the count to 2 to force
     63            // isEmpty to return false.
     64            tester->m_moveCount = 2;
     65        }
     66    }
     67
     68private:
     69    // Any non-move-to element, or more than one move-to element, will make the count >= 2.
     70    unsigned m_moveCount;
     71};
     72
     73// Paths with only move-to elements do not draw under any circumstances, so their bound should
     74// be empty. Currently, CoreGraphics returns non-empty bounds for such paths. Radar 10450621
     75// tracks this. This class reports paths that have only move-to elements, allowing the
     76// bounding box code to work around the CoreGraphics problem.
     77class PathHasOnlyMoveToTester {
     78public:
     79    PathHasOnlyMoveToTester() : m_hasSeenOnlyMoveTo(true) { }
     80
     81    bool hasOnlyMoveTo() const
     82    {
     83        return m_hasSeenOnlyMoveTo;
     84    }
     85
     86    static void testPathElement(void* info, const CGPathElement* element)
     87    {
     88        PathHasOnlyMoveToTester* tester = static_cast<PathHasOnlyMoveToTester*>(info);
     89        if (tester->m_hasSeenOnlyMoveTo && element->type != kCGPathElementMoveToPoint)
     90            tester->m_hasSeenOnlyMoveTo = false;
     91    }
     92
     93private:
     94    bool m_hasSeenOnlyMoveTo;
     95};
     96
    4497static size_t putBytesNowhere(void*, const void*, size_t count)
    4598{
    … …  
    166219    // CGPathGetBoundingBox includes the path's control points, CGPathGetPathBoundingBox
    167220    // does not, but only exists on 10.6 and above.
     221    // A bug in CoreGraphics leads to an incorrect bound on paths containing only move-to elements
     222    // with a linecap style that is non-butt. All paths with only move-to elements (regardless of
     223    // linecap) are effectively empty for bounding purposes and here we make it so.
     224    PathHasOnlyMoveToTester tester;
     225    CGPathApply(m_path, &tester, PathHasOnlyMoveToTester::testPathElement);
     226    if (tester.hasOnlyMoveTo())
     227        return FloatRect(0, 0, 0, 0);
     228
    168229#if !defined(BUILDING_ON_LEOPARD)
    169230    return CGPathGetPathBoundingBox(m_path);
    … …  
    175236FloatRect Path::fastBoundingRect() const
    176237{
     238    // A bug in CoreGraphics leads to an incorrect bound on paths containing only move-to elements
     239    // with a linecap style that is non-butt. All paths with only move-to elements (regardless of
     240    // linecap) are effectively empty for bounding purposes and here we make it so.
     241    PathHasOnlyMoveToTester tester;
     242    CGPathApply(m_path, &tester, PathHasOnlyMoveToTester::testPathElement);
     243    if (tester.hasOnlyMoveTo())
     244        return FloatRect(0, 0, 0, 0);
     245
    177246    return CGPathGetBoundingBox(m_path);
    178247}
    … …  
    253322bool Path::isEmpty() const
    254323{
    255     return CGPathIsEmpty(m_path);
     324    // The SVG rendering code that uses this method relies on paths with a single move-to
     325    // element, and nothing else, as being empty. Until that code is refactored to avoid
     326    // the dependence on isEmpty, we match the behavior of other platforms.
     327    // When the SVG code is refactored, we could use CGPathIsEmpty(m_path);
     328    PathIsEmptyOrSingleMoveTester tester;
     329    CGPathApply(m_path, &tester, PathIsEmptyOrSingleMoveTester::testPathElement);
     330    return tester.isEmpty();
    256331}
    257332
    258333bool Path::hasCurrentPoint() const
    259334{
    260     return !isEmpty();
     335    return !CGPathIsEmpty(m_path);
    261336}
    262337   
    … …  
    312387void Path::transform(const AffineTransform& transform)
    313388{
    314     if (transform.isIdentity() || isEmpty())
     389    if (transform.isIdentity() || CGPathIsEmpty(m_path))
    315390        return;
    316391
Note: See TracChangeset for help on using the changeset viewer.