This reverts commit 26d8d77aae.
Reason for revert: speculative, in case this is blocking the chrome roll
Original change's description:
> don't trust convexity with affine transforms
>
> In theory, a convex shape transformed by an affine matrix should still
> be convex. However, due to numerical nastiness of floats, when we try
> to determine if something is convex, we can get different answers pre
> and post a transformation (think of two line segments nearly colinear).
>
> Convex paths take a faster scan converter, but it is only well behaved
> if the path is, in fact, convex. Thus we have to be conservative about
> which paths we mark as convex.
>
> This bug found a case where a "convex" path, after going through a transform,
> became (according to our measure) non-convex. The bug was that we *thought*
> that once convex always convex, but in reality it was not. The fix (hack) is
> to notice when we transform by an affine matrix (we're still assuming/hoping
> that scaling and translate keep things convex (1)...) and mark the convexity
> as "unknown", forcing us to re-compute it.
>
> This will slow down these paths, since it costs something to compute convexity.
> Hopefully non-scale-translate transforms are rare, so we won't notice the
> speed loss too much.
>
> (1) This is not proven. If we find scaling/translation to break our notion of
> convexity, we'll need to get more aggressive/clever to find a fix.
>
>
> Bug: 899689
> Change-Id: I5921eca247428bf89380bc2395fe373fa70deb1d
> Reviewed-on: https://skia-review.googlesource.com/c/173080
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Cary Clark <caryclark@google.com>
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
TBR=mtklein@google.com,jvanverth@google.com,caryclark@google.com,reed@google.com,caryclark@skia.org
Change-Id: I5d846798f2c34c6576591a3c3125cfdc3c72dbdc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 899689
Reviewed-on: https://skia-review.googlesource.com/c/173162
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
In theory, a convex shape transformed by an affine matrix should still
be convex. However, due to numerical nastiness of floats, when we try
to determine if something is convex, we can get different answers pre
and post a transformation (think of two line segments nearly colinear).
Convex paths take a faster scan converter, but it is only well behaved
if the path is, in fact, convex. Thus we have to be conservative about
which paths we mark as convex.
This bug found a case where a "convex" path, after going through a transform,
became (according to our measure) non-convex. The bug was that we *thought*
that once convex always convex, but in reality it was not. The fix (hack) is
to notice when we transform by an affine matrix (we're still assuming/hoping
that scaling and translate keep things convex (1)...) and mark the convexity
as "unknown", forcing us to re-compute it.
This will slow down these paths, since it costs something to compute convexity.
Hopefully non-scale-translate transforms are rare, so we won't notice the
speed loss too much.
(1) This is not proven. If we find scaling/translation to break our notion of
convexity, we'll need to get more aggressive/clever to find a fix.
Bug: 899689
Change-Id: I5921eca247428bf89380bc2395fe373fa70deb1d
Reviewed-on: https://skia-review.googlesource.com/c/173080
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Spoiler alert... it doesn't.
Bug: oss-fuzz:10488
Change-Id: Ifafd92f40aed55ff14a5198ea7d79a20751e40aa
Reviewed-on: https://skia-review.googlesource.com/156661
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Add header and .bmh docs for shrinkToFit().
Also remove SkPath::debugging_private_getFreeSpace() and refactor tests
to use (friended) helper classes.
Docs-Preview: https://skia.org/?cl=153668
Change-Id: I3f93f9561b25025ce04a81d5e2f0271295b3daad
Reviewed-on: https://skia-review.googlesource.com/153668
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
UBSAN points out that -1 isn't required to be representable
in a FirstDirection, but I think the union of all possible
fields is.
In this case, the union is 3, oring together CW (0), CCW (1)
and Unknown (2). Since it's not one of the meaningful values
of the enum, it works as a nice sentinel.
Change-Id: Ib428a8f0d7f5edbf492501306604ba57e9d19e38
Reviewed-on: https://skia-review.googlesource.com/147002
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Yuqian Li <liyuqian@google.com>
We're testing here that when isRect() returns false,
the isClosed and direction fields are unchanged.
Instead of using illegal values (which trip up UBSAN)
test all combinations of legal values.
It looks like we were trying to use an illegal -1 bool
to do the same trick as the enum, but I think it was
legal and just always true.
Change-Id: Ia4ab4c3d52f61bf67e888dea67549ab09750902a
Reviewed-on: https://skia-review.googlesource.com/147001
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Use std::swap instead. It does not appear that any external user
specializes SkTSwap, but some may still use it. This removes all use in
Skia so that SkTSwap can later be removed in a smaller CL. After that
the <utility> include can be removed from SkTypes.h.
Change-Id: If03d4ee07dbecda961aa9f0dc34d171ef5168753
Reviewed-on: https://skia-review.googlesource.com/135578
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This reverts commit 2a2f675926.
Reason for revert: this appears to be what is holding up the Chrome roll.
Original change's description:
> SkTypes: extract SkTo
>
> Change-Id: I8de790d5013db2105ad885fa2683303d7c250b09
> Reviewed-on: https://skia-review.googlesource.com/133620
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,halcanary@google.com
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Iafd738aedfb679a23c061a51afe4b98a8d4cdfae
Reviewed-on: https://skia-review.googlesource.com/134504
Reviewed-by: Hal Canary <halcanary@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
Bug: skia:7994
Change-Id: I83bb309a2c8fb0bddaf78ba32c0a07537e483900
Reviewed-on: https://skia-review.googlesource.com/129648
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:
Change-Id: Ide917f54633370f1fce46a115fa923794b981e2e
Reviewed-on: https://skia-review.googlesource.com/129461
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This was triggered by an exploit that started the first
edge well outside the final rectangle, causing the captured
to exceed the correct result.
Ivan observes that we really only want the first and third
corners to compute the bounds, so remove the tracking code
that looks for a valid range of points, and record the
corners instead.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: If228573d0f05c7158dba8142c144d13834e691ec
Reviewed-on: https://skia-review.googlesource.com/122081
Commit-Queue: Cary Clark <caryclark@skia.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: Cary Clark <caryclark@skia.org>
One of the path is rect bug fixes changed
the behavior of zero-length strokes which
showed up as a change in Gold.
The bug is if a rect is defined by a
series of colinear movetos, the bounds
did not work out if the rect started
and stopped in the middle of a side.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: I226545efeda03dedd928eebc120d2508b428fef0
Reviewed-on: https://skia-review.googlesource.com/122002
Auto-Submit: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@skia.org>
Removed a test that appeared to go uncalled;
Ivan to the rescue, with a test case
proving that it is required.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: I7df9688072bd36b7597673148e3fe5dbbf82f5a7
Reviewed-on: https://skia-review.googlesource.com/121883
Commit-Queue: Cary Clark <caryclark@skia.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Exposes that final close along a diagonal need not
include a close verb if the subsequent verb is move;
so we have to check for a diagonal then.
The later check for diagonal included a comment that
it may not be needed which does appear to be the case.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: I17a9414e8b3e69b82c2eda28195696eae4e3d513
Reviewed-on: https://skia-review.googlesource.com/121801
Commit-Queue: Cary Clark <caryclark@skia.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This one accumulates the othershoot when all four sides
have the same direction, and the final side when closed
should cause the overshoot to be ignored.
Docs-Preview: https://skia.org/?cl=121787
Bug: 824145,skia:7792
Change-Id: I71ea0fcdd0f03a4fcac224b57220c65c321112f6
Reviewed-on: https://skia-review.googlesource.com/121787
Commit-Queue: Cary Clark <caryclark@skia.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is bug number ten in the series, and is the
most interesting. It exploits that the code tracks
corners 0, 2, and 3 but not corner 1.
Changing the code to track all corners is the biggest
so far, and while it (hopefully) simplifies things,
the presence of new code may signify more bugs to come.
R=robertphillips@google.com
Bug: 824145,skia::7792
Change-Id: Ia18e4d80fbed06ae6d9c89dcb4c462c5610213cc
Reviewed-on: https://skia-review.googlesource.com/121487
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
This variation exploits a sequence which uses a zero
length line to note that lines have been recorded, but
no rectangle edge has been encountered.
R=robertphillips@google.com
Docs-Preview: https://skia.org/?cl=121282
Bug: 824145,skia:7792
Change-Id: I652e9482b2867c3d7da30d5f5df2aecbfd0d716d
Reviewed-on: https://skia-review.googlesource.com/121282
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@skia.org>
This variation tricks SkPath::isRect by exploiting
that the implementation resets the point pointer to
process the close verb, and using the reset pointer
to walk over a series of points that don't move.
In addition to fixing this, rename variables to
make the line creation more obvious, since left,
right, and friends, are not the left and right.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: If8ebbc3eedd270652670d6e111a5bc02e61f0eec
Reviewed-on: https://skia-review.googlesource.com/121122
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@skia.org>
This addresses comment #17 of skbug.com/7792.
The bug overshoots the end and exploits that the
first point tracked by close isn't the first
point in the rectangle.
Fixing this slightly regresses the example
in comment #14; before it was treated as a filled
rect but now it is not; this conservative approach
doesn't cause any other regressions.
bug7792 in pathfill.cpp verifies that all paths
in the bug draw correctly by comparing CPU and GPU.
R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: I55bea023d2ad7456c8c3ebd9d1df95fe34e0a0d4
Reviewed-on: https://skia-review.googlesource.com/120996
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Robert Phillips <robertphillips@google.com>
More edge cases found; clean up the logic a bit
to make more clear where the rectangle points
start and stop.
R=robertphillips@google.com,brianosman@google.com
Bug: 824145,skia:7792
Change-Id: Ie24dfd1519f30875f44ffac68e20d777490b00b9
Reviewed-on: https://skia-review.googlesource.com/120422
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Add a check to see that the close path generated line
is horizontal or vertical when determining that path
is a rect.
Also change several tests to defer their initialization
to reduce debugging interference.
R=brianosman@google.com,robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: I4a081ee4ffd3558b499a7a1aede2d6232059715e
Reviewed-on: https://skia-review.googlesource.com/120081
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
R=brianosman@google.com,robertphillips@google.com
TBR=reed@google.com
Bug: 824145,skia:7792
Change-Id: I24f121cfa7d437c95b94bd917d3c4888a10c519e
Reviewed-on: https://skia-review.googlesource.com/119569
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
New format should be much simpler:
- only store public data (e.g. points, verbs, filltype)
- deserialize just uses public APIs
Refactor reading code to manage different (older) versions, to make
it clear (hopefully) what we can delete when we can abandon version
3 support.
Bug: skia:
Change-Id: I30465f891cba3f044ae1cb2c13c04f04fdc9da78
Reviewed-on: https://skia-review.googlesource.com/109160
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Remove REPORTER_ASSERT_MESSAGE.
Change-Id: I6d00715901159c93e22d182fe24aac92b5fdbcf4
Reviewed-on: https://skia-review.googlesource.com/100361
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Make insetting greater than width or height collapse to a point/line.
SkPath::addRRect() doesn't ignore an empty SkRRect.
Change-Id: I933a3419a6d75be534f1d8328faa715772045f67
Reviewed-on: https://skia-review.googlesource.com/85680
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit c382384848.
Reason for revert: need to add legacy flag to chrome first
Original change's description:
> keep SVG arcs axis aligned
>
> Computing the arc width introduces rounding errors that cause the
> arc to exceed 1/4 circle and cause integer anchored arcs to start
> outside their marks. A round rect may lose convexity as a result.
>
> Check if arcTo() inputs are integers and arc is 90 degrees;
> if so, output conics which are axis-aligned on integers as well.
>
> This is triggered when using SVG to represent a round rect.
>
> Possible future enhancements are recorded in bug.skia.org/7383
>
> R=reed@google.com,djsollen@google.com
> Change-Id: I6609456fcefabcda6c9560a044533ecb5cda2d31
> Reviewed-on: https://skia-review.googlesource.com/79423
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
TBR=djsollen@google.com,caryclark@google.com,reed@google.com,caryclark@skia.org
Change-Id: Ia503ea62def15322df2b11da30d377a9543d6999
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/81281
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
Computing the arc width introduces rounding errors that cause the
arc to exceed 1/4 circle and cause integer anchored arcs to start
outside their marks. A round rect may lose convexity as a result.
Check if arcTo() inputs are integers and arc is 90 degrees;
if so, output conics which are axis-aligned on integers as well.
This is triggered when using SVG to represent a round rect.
Possible future enhancements are recorded in bug.skia.org/7383
R=reed@google.com,djsollen@google.com
Change-Id: I6609456fcefabcda6c9560a044533ecb5cda2d31
Reviewed-on: https://skia-review.googlesource.com/79423
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
The added unit test shows that we'll trigger SkASSERT failure if we
forgot to check that the edges are lines (instead of quads or cubics).
Similar to skia:7015, this bug is not triggered in our production
because we don't use DAA for convex paths by default. The skipRect is
an optimization just for convex paths.
Bug: skia:7051
Change-Id: Id87ce2d452ede0db9a48425541f473af19ee0572
Reviewed-on: https://skia-review.googlesource.com/48045
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This reverts commit f953523224.
Reason for revert: Re-landing now that gold has settled.
Original change's description:
> Revert "Don't ignore degenerates when deciding if a path is convex"
>
> This reverts commit 53cd6c4331.
>
> Reason for revert: Temporary to limit gold cross-talk with other CLs.
>
> Original change's description:
> > Don't ignore degenerates when deciding if a path is convex
> >
> > This ensures that a path with a second contour will always be marked
> > concave. GrDefaultPathRenderer was incorrectly drawing paths of this type
> > (thinking that it could fill them with simple triangulation).
> >
> > Bug: skia:7020 skia:1460
> > Change-Id: I62bfd72e4c61da427687acf53c552357b57707aa
> > Reviewed-on: https://skia-review.googlesource.com/47082
> > Reviewed-by: Cary Clark <caryclark@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
>
> TBR=bsalomon@google.com,brianosman@google.com,caryclark@google.com,reed@google.com
>
> Change-Id: Id7d121633faeb8a43dbd334409408ba51db43d68
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:7020 skia:1460
> Reviewed-on: https://skia-review.googlesource.com/47343
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=bsalomon@google.com,brianosman@google.com,caryclark@google.com,reed@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:7020 skia:1460
Change-Id: I45ff90a54b66ce9ea068f246d066cc24b310c966
Reviewed-on: https://skia-review.googlesource.com/47820
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 53cd6c4331.
Reason for revert: Temporary to limit gold cross-talk with other CLs.
Original change's description:
> Don't ignore degenerates when deciding if a path is convex
>
> This ensures that a path with a second contour will always be marked
> concave. GrDefaultPathRenderer was incorrectly drawing paths of this type
> (thinking that it could fill them with simple triangulation).
>
> Bug: skia:7020 skia:1460
> Change-Id: I62bfd72e4c61da427687acf53c552357b57707aa
> Reviewed-on: https://skia-review.googlesource.com/47082
> Reviewed-by: Cary Clark <caryclark@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=bsalomon@google.com,brianosman@google.com,caryclark@google.com,reed@google.com
Change-Id: Id7d121633faeb8a43dbd334409408ba51db43d68
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:7020 skia:1460
Reviewed-on: https://skia-review.googlesource.com/47343
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This ensures that a path with a second contour will always be marked
concave. GrDefaultPathRenderer was incorrectly drawing paths of this type
(thinking that it could fill them with simple triangulation).
Bug: skia:7020 skia:1460
Change-Id: I62bfd72e4c61da427687acf53c552357b57707aa
Reviewed-on: https://skia-review.googlesource.com/47082
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
