- most of the small diffs are because I moved GrWangsFormula.h out
of the tessellate/ directory and into the geometry/ directory since
it's more general than HW tessellation.
The previous implementation was based on the heuristic that the distance
from the true curve to the line segment would be divided by 4 every time
the curve was recursively subdivided. This was a reasonable
approximation if the curve had balanced curvature on both sides of the
split. However, in the case of the new GM's curve, the left half was
already very linear and the right half had much higher curves.
This lead to the approximation reporting fewer points than required.
Theoretically, those few points that weren't utilized by the left half
of the curve could have been made available to the right half, but the
implementation of that would be tricky.
Instead, it now uses Wang's formula to compute the number of points.
Since recursive subdivision leads to linearly spaced samples assuming it
can't stop early, this point count represents a valid upper bound on
what's needed. It also then ensures both left and right halves of a
curve have the point counts they might need w/o updating the
generation implementations. However, since the recursive point
generation exits once each section has reached the error tolerance, in
scenarios where the prior approximation was reasonable, we'll end up
using fewer points than reported by Wang's. Hopefully that means there
is negligible performance regression since we won't be increasing
vertex counts by that much (except where needed for correctness).
Bug: skia:11886
Change-Id: Iba39dbe4de82011775524583efd461b10c9259fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405197
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
In the new GM, the left and right portions of the path have a shared
vertex. The two edges leaving that vertex vertically towards the bottom
of the image are nearly parallel but the right edge has a steeper slope.
This leads to two sources of self intersections. The outset vertices
for the left and right sides intersect, which converts some of those
edges into "connecting" edges and removes others (preventing double
hitting of pixels). However, these outset but now "connecting" edges
from the right side also overlap with the inset vertices of the left
shape, requiring additional vertex splitting and connecting edge
creation.
The old alpha logic when one of the intersecting edges was a connecting
edge was to use its interpolated alpha value. In this case, since the
connecting edge originated from two outset vertices, its end point
alphas were 0 and the split alpha became 0, even though the other
intersecting edge was an interior edge.
This CL flips the logic around and ensures that any split vertex that
is on the interior edges remains fully opaque, any vertex that is
fully on the exterior remains fully transparent, and anything else
uses the max of the interpolated alphas (equivalent to the old
logic when one edge was connecting and one was exterior, but is more
accurate if somehow we get two connecting edges intersecting).
Bug: skia:11859
Change-Id: I85d2d54a8833e3c9da2fdd1a4f3a0513119730b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/400596
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Mechanically updated via Xcode "Replace Regular Expression":
typedef (.*) INHERITED;
-->
using INHERITED = $1;
The ClangTidy approach generated an even larger CL which would have
required a significant amount of hand-tweaking to be usable.
Change-Id: I671dc9d9efdf6d60151325c8d4d13fad7e10a15b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Namespace comments don't assist readability for blocks that can easily
fit on a single screen.
Change-Id: I93cbebe8e51400dead794c9eb41cb1eaa86bf756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312639
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
When trying to chop a quadratic along the right edge of a clip,
the computed t value was so close to 1.0 that our chopper method
returns false (we would have needed doubles to detect this).
To handle this, pin the X values to the right edge, so that we at least
maintain the contract that we are clipped.
Bug: 1070835
Change-Id: Ifdc59f97c7f5c32b321647f6739b37b33ce801c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285576
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This almost gets gms to be iwyu clean. The last bit is around gm.cpp
and the tracing framework and its use of atomic. Will also need a way
of keeping things from regressing, which is difficult due to needing to
do this outside-in.
Change-Id: I1393531e99da8b0f1a29f55c53c86d53f459af7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/211593
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Current strategy: everything from the top
Things to look at first are the manual changes:
- added tools/rewrite_includes.py
- removed -Idirectives from BUILD.gn
- various compile.sh simplifications
- tweak tools/embed_resources.py
- update gn/find_headers.py to write paths from the top
- update gn/gn_to_bp.py SkUserConfig.h layout
so that #include "include/config/SkUserConfig.h" always
gets the header we want.
No-Presubmit: true
Change-Id: I73a4b181654e0e38d229bc456c0d0854bae3363e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209706
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
This differed from the separate versions in that it snapped to zero.
It was also strictly worse than calling the two separate versions.
Most clients don't need the snapping, so just call the two existing
functions. For clients that need the snapping, call new variants of
each that do snap.
Change-Id: Ia4e09fd9651932fe15caeab1399df7f6281bdc17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/205303
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: 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>
gm bug7792 had wrong dimensions and clipped out half the draws
bm SkRect_Reference.bmh referred to SkIPoint::center.. which
has been removed.
Docs-Preview: https://skia.org/?cl=120640
Bug: skia:6898
Change-Id: Id4c59c82005e1060d0ca933d9a3650fe4f121264
Reviewed-on: https://skia-review.googlesource.com/120640
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Cary Clark <caryclark@skia.org>
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>
BUG=chromium:682918
Change-Id: Ieadb41229227a20d41b8e932ba0770fe72479898
Reviewed-on: https://skia-review.googlesource.com/9068
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
BUG=chromium:682918
Change-Id: I1a0608f7e6394ab05eebc4b78fb7087ca718f617
Reviewed-on: https://skia-review.googlesource.com/8971
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Should produce sharper results than arbitrary fixed sizes.
Adds a new test to pathfill GM.
Was: https://skia-review.googlesource.com/c/8328/
BUG=chromium:682918, skia:6238
Change-Id: Ia62ea5ce6b4a5ac2b8b51d06d57dc951d6c340b8
Reviewed-on: https://skia-review.googlesource.com/8384
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This reverts commit 6e83b13c22.
Reason for revert: Fractional path sizes are causing asserts on the bots.
Original change's description:
> Use SDF path miplevels based on the original path's size.
>
> Should produce sharper results than arbitrary fixed sizes.
> Adds a new test to pathfill GM.
>
> BUG=chromium:682918
>
> Change-Id: I5a394098665d01e995a244fde278236f1471e6c9
> Reviewed-on: https://skia-review.googlesource.com/8328
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
>
TBR=jvanverth@google.com,bsalomon@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:682918
Change-Id: I4a52df808ef3f769d0e6f75785148d46936a6747
Reviewed-on: https://skia-review.googlesource.com/8342
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Should produce sharper results than arbitrary fixed sizes.
Adds a new test to pathfill GM.
BUG=chromium:682918
Change-Id: I5a394098665d01e995a244fde278236f1471e6c9
Reviewed-on: https://skia-review.googlesource.com/8328
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Disables use of SDFs for very small paths (because of blurring) and
adds a border of 1 pixel in device space to handle antialiasing.
BUG=chromium:677889
Change-Id: Icd2f7e80323b1255f8de52b97360e9a2d995c765
Reviewed-on: https://skia-review.googlesource.com/6895
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This reverts commit d081ff314f.
Reason for revert: Failing unit tests.
Original change's description:
> More fixes for distance field paths
>
> Disables use of SDFs for very small paths (because of blurring) and
> adds a border of 1 pixel in device space to handle antialiasing.
>
> BUG=chromium:677889
>
> Change-Id: I81e49477c943d41523fd836e55abd696a985491f
> Reviewed-on: https://skia-review.googlesource.com/6832
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
>
TBR=jvanverth@google.com,bsalomon@google.com,robertphillips@google.com,reviews@skia.org
BUG=chromium:677889
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I4a6a698fa2e9e58c1c98a5a89f54bed724527951
Reviewed-on: https://skia-review.googlesource.com/6890
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Disables use of SDFs for very small paths (because of blurring) and
adds a border of 1 pixel in device space to handle antialiasing.
BUG=chromium:677889
Change-Id: I81e49477c943d41523fd836e55abd696a985491f
Reviewed-on: https://skia-review.googlesource.com/6832
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
BUG=668550
Change-Id: Ic771818bd5a4a46b83fdb82b69b98cb6b93a23a2
Reviewed-on: https://skia-review.googlesource.com/5697
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit 92964124c5.
Reason for revert: Causing roll failure. Need to find images to rebaseline
Change-Id: I09cad4c3a48fefcfc669fb1045613336c88cb33a
Reviewed-on: https://skia-review.googlesource.com/5686
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
BUG=668550
Change-Id: Ib496db82c7391aca61b31afaeb5445260170cc49
Reviewed-on: https://skia-review.googlesource.com/5549
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
'static const' means, there must be at most one of these, and initialize it at
compile time if possible or runtime if necessary. This leads to unexpected
code execution, and TSAN* will complain about races on the guard variables.
Generally 'constexpr' or 'const' are better choices. Neither can cause races:
they're either intialized at compile time (constexpr) or intialized each time
independently (const).
This CL prefers constexpr where possible, and uses const where not. It even
prefers constexpr over const where they don't make a difference... I want to have
lots of examples of constexpr for people to see and mimic.
The scoped-to-class static has nothing to do with any of this, and is not changed.
* Not yet on the bots, which use an older TSAN.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2300623005
Review-Url: https://codereview.chromium.org/2300623005
Add RotateCircles3 back as better-named QuadStroker.
Switch pathfill test to call skia before draw instead of in
initializer to avoid triggering debugging breakpoints.
Review URL: https://codereview.chromium.org/912273003
This fixes every case where virtual and SK_OVERRIDE were on the same line,
which should be the bulk of cases. We'll have to manually clean up the rest
over time unless I level up in regexes.
for f in (find . -type f); perl -p -i -e 's/virtual (.*)SK_OVERRIDE/\1SK_OVERRIDE/g' $f; end
BUG=skia:
Review URL: https://codereview.chromium.org/806653007
