It seems that we've already rebaselined the golden images
TBR: reed@google.com
Bug: skia:7271
Change-Id: I841dc4933f0cad015c3c6da7bd08ddfb2a35c5a2
Reviewed-on: https://skia-review.googlesource.com/92060
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Threaded backend will need this to always fill path in DAA.
Bug: skia:
Change-Id: Ifbfb0137b89eb71b7c2a2722aee1dad196a52553
Reviewed-on: https://skia-review.googlesource.com/76120
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Eventually, we'll remove containedInClip from the function parameters
and just compute it using ir and clipBounds. The current approach is
for preparing rebaseline.
TBR: reed@google.com
Bug: skia:7271
Change-Id: Ife60e173d9f4f1c9ec80d13a2f7a88a0433b61e9
Reviewed-on: https://skia-review.googlesource.com/69220
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
preserving the containedInClip boolean. We will eventually remove it
and rebaseline the layout tests.
Bug: skia:7271
Change-Id: I20e7220340d561ea2c50d30cd5d6ac6d2b4b3743
Reviewed-on: https://skia-review.googlesource.com/68100
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This reverts commit 12f322b9d4.
Reason for revert: Unexpectedly break Chrome layout tests. Will check why.
Original change's description:
> Simplify fill path call by removing do_fill_path
>
> The git diff is not very informative for this CL.
> Here's a better diff:
>
> 1. do_fill_path is removed and its content is copied to AntiFillPath
>
> 2. Any call to do_fill_path is removed.
>
> 3. std::function FillPathFunc is removed (and replaced by direct
> AAAFillPath, DAAFillPath, and SAAFillPath call).
>
> 4. The old call chain is:
> AntiFillPath -> (AAAFillPath/DAAFillPath/...)
> -> do_fill_path
> -> specific FillPathFunc
> The new call chain is:
> AntiFillPath -> AAAFillPath/DAAFillPath/SAAFillPath
>
> This is made possible by the removal of SK_SUPPORT_LEGACY_AA_CHOICE
> which makes sure that AntiFillPath is the only function that makes
> the choice of AAA/DAA/SAA.
>
> In the next CL, I'll improve the structure of SkScan::AntiFillPath
> to prepare for Threaded Backend's init-once change.
>
> Bug: skia:
> Change-Id: If6ebbdab207cadb7bfe2cb3fcf33ea3d180c3896
> Reviewed-on: https://skia-review.googlesource.com/67340
> Reviewed-by: Mike Reed <reed@google.com>
> Reviewed-by: Cary Clark <caryclark@google.com>
> Commit-Queue: Yuqian Li <liyuqian@google.com>
TBR=caryclark@google.com,liyuqian@google.com,reed@google.com,caryclark@skia.org
Change-Id: I7d9517574265db5bc372a5749e6480df8e938f2e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/67855
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
The git diff is not very informative for this CL.
Here's a better diff:
1. do_fill_path is removed and its content is copied to AntiFillPath
2. Any call to do_fill_path is removed.
3. std::function FillPathFunc is removed (and replaced by direct
AAAFillPath, DAAFillPath, and SAAFillPath call).
4. The old call chain is:
AntiFillPath -> (AAAFillPath/DAAFillPath/...)
-> do_fill_path
-> specific FillPathFunc
The new call chain is:
AntiFillPath -> AAAFillPath/DAAFillPath/SAAFillPath
This is made possible by the removal of SK_SUPPORT_LEGACY_AA_CHOICE
which makes sure that AntiFillPath is the only function that makes
the choice of AAA/DAA/SAA.
In the next CL, I'll improve the structure of SkScan::AntiFillPath
to prepare for Threaded Backend's init-once change.
Bug: skia:
Change-Id: If6ebbdab207cadb7bfe2cb3fcf33ea3d180c3896
Reviewed-on: https://skia-review.googlesource.com/67340
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
So I can rebaseline the golden images and unify AA choices without
fully turning on DAA.
Bug: skia:
Change-Id: I33447f60c8eac6318dda7730abd6caefdb14c5dc
Reviewed-on: https://skia-review.googlesource.com/62840
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Yuqian Li <liyuqian@google.com>
All scan converters need an SkIRect clipBounds and a bool
containedInClip. However, we previously sent in an SkRegion
and a SkIRect*, and convert them into clipBounds and
containedInClip all over the places. This CL converts them
only once inside do_fill_path and change all args to SkIRect
and bool.
Bug: skia:
Change-Id: I05f1d76322942d8817860fd33991f7f7ce918e7c
Reviewed-on: https://skia-review.googlesource.com/52741
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
DAA is:
1. Much simpler than AAA.
SkScan_AAAPath.cpp is about 1700 lines.
SkScan_DAAPath.cpp is about 300 lines.
The whole DAA CL is only about 800 lines.
2. Much faster than AAA for complicated paths.
The speedup applies to GL backend (including ccpr)!
Here's the frame time of 'SampleApp --slide Chart' on macbook pro:
AAA-raster: 33ms
DAA-raster: 21ms
AAA-gl: 30ms
DAA-gl: 20ms
AAA-ccpr: 18ms
DAA-ccpr: 12ms
My linux desktop doesn't have SSE3 so the speedup is smaller
(~25% for Chart). I believe that DAA is so fast that I can enable
it for any paths (AAA is not enabled by default for complicated
paths because it is slow; hence our older supersampling scan
converter is used for stroking on Chart for AAA-xxx config.)
3. The SkCoverageDelta is suitable for threaded backend with
out-of-order concurrent scan conversion as commented in the source
code. Maybe we can also just send deltas to GPU.
4. Similar to most analytic path renderers, the quality is on the best
ground-truth level, unless there are intersections within a pixel.
The intersections look good to my eyes although theoretically that
could be arbitrary far from the ground truth (see my AAA slides).
5. For simple paths, such as circle, triangle, rrect, etc., DAA is
slower than AAA. But DAA is faster than our older supersampling
scan converter in most cases. As those simple paths usually don't
constitute the bottleneck of a picture (skp or svg), I strongly
recommend use DAA.
6. DAA also heavily favors blitMask so it may work quite well with
SkRasterPipeline and SkRasterPipelineBlitter.
Finally, please check https://skia-review.googlesource.com/c/22420/
which accelerate DAA by specializing blitCoverageDeltas for
SkARGB32_Blitter and SkARGB32_Black_Blitter. It brings a little(<5%)
speedup. But I couldn't figure out how to reduce the duplicate code
so I don't intend to land it.
Bug: skia:
Change-Id: I3b7ed6a727447922e645b1acb737a506e7c09a4c
Reviewed-on: https://skia-review.googlesource.com/19666
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
The rebaseline has finished in Chromium.
Bug: skia:
Change-Id: Ibafb32b3683f1bfc3e451911e6fb8382bfc9b857
Reviewed-on: https://skia-review.googlesource.com/23481
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Previously, suitableForAAA disabled Analytic AA for small rects.
But that's a mistake. Re-enabling AAA will bring 10-20% speedup for small rects
(~800ns -> ~700ns measured by path_fill_small_rect nanobench).
Bug: skia:
Change-Id: I943f1c754669391f55e46471781fa65840629377
Reviewed-on: https://skia-review.googlesource.com/22205
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This reverts commit 6d70274ccf.
Reason for revert: need rebaseline
Original change's description:
> Always enable Analytic AA for rects
>
> Previously, suitableForAAA disabled Analytic AA for small rects.
> But that's a mistake. Re-enabling AAA will bring 10-20% speedup for small rects
> (~800ns -> ~700ns measured by path_fill_small_rect nanobench).
>
> Bug: skia:
> Change-Id: I1d8bf4c105d8d116ea441a46e6c955c546bcbb22
> Reviewed-on: https://skia-review.googlesource.com/22074
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Yuqian Li <liyuqian@google.com>
TBR=mtklein@chromium.org,herb@google.com,caryclark@google.com,liyuqian@google.com,reed@google.com
Change-Id: I87011bf7b8f30fa3be5d46287afbaf38c0be7da9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/22140
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Previously, suitableForAAA disabled Analytic AA for small rects.
But that's a mistake. Re-enabling AAA will bring 10-20% speedup for small rects
(~800ns -> ~700ns measured by path_fill_small_rect nanobench).
Bug: skia:
Change-Id: I1d8bf4c105d8d116ea441a46e6c955c546bcbb22
Reviewed-on: https://skia-review.googlesource.com/22074
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This silences a new warning in clang 5.0
Change-Id: Ieb5b75a6ffed60107c3fd16075d2ecfd515b55e8
Reviewed-on: https://skia-review.googlesource.com/10006
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
The only difference is that we now also put the guard flag
SK_SUPPORT_LEGACY_AAA in SkUserConfig.h. Previously, SkAnalyticEdge.cpp doesn't
get that flag from SkScan.h and that caused many problems.
BUG=skia:
TBR=reed@google.com,caryclark@google.com
Change-Id: I134bb76cebd6fffa712f438076668765321bba3b
Reviewed-on: https://skia-review.googlesource.com/6992
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This reverts commit b46fff60bc.
Reason for revert: possible chromium cc unit tests failure
Change-Id: Ie174c55e4d0fc3ae45854b5897ba26b7ad5a9c13
Reviewed-on: https://skia-review.googlesource.com/6981
Commit-Queue: Yuqian Li <liyuqian@google.com>
Reviewed-by: Yuqian Li <liyuqian@google.com>
The only difference is that we now put the guard flag SK_SUPPORT_LEGACY_AAA in
SkUserConfig.h instead of SkScan.h. Previously, SkAnalyticEdge.cpp doesn't get
that flag from SkScan.h and that caused many problems.
BUG=skia:
TBR=reed@google.com,caryclark@google.com
Change-Id: I7b89d3cb64ad71715101d2a5e8e77be3a8a6fa16
Reviewed-on: https://skia-review.googlesource.com/6972
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
This reverts commit 89a0e72287.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Implement Analytic AA for General Paths (with Guard against Chrome)
>
> I've set up a SK_SUPPORT_LEGACY_AAA flag to guard against Chromium layout tests. I also set that flag in this CL so theoretically this CL won't trigger any GM changes. I'll use this to verify my guard, and remove that flag and actually enables concave AAA in a future CL.
>
> When enabled, for most simple concave paths (e.g., rectangle stroke, rrect stroke, sawtooth, stars...), the Analytic AA achieves 1.3x-2x speedup, and they look much prettier. And they probably are the majority in our use cases by number. But they probably are not the majority by time cost; a single complicated path may cost 10x-100x more time to render than a rectangle stroke... For those complicated paths, we fall back to supersampling by default as we're likely to be 1.1-1.2x slower and the quality improvement is not visually significant. However, one can use gSkForceAnalyticAA to disable that fallback.
>
> BUG=skia:
>
> Change-Id: If9549a3acc4a187cfaf7eb51890c148da3083d31
> Reviewed-on: https://skia-review.googlesource.com/6091
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Yuqian Li <liyuqian@google.com>
>
TBR=caryclark@google.com,liyuqian@google.com,reed@google.com
BUG=skia:
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I13c05aaa1bcb14956bd0fe01bb404e41be75af22
Reviewed-on: https://skia-review.googlesource.com/6961
Commit-Queue: Yuqian Li <liyuqian@google.com>
Reviewed-by: Yuqian Li <liyuqian@google.com>
I've set up a SK_SUPPORT_LEGACY_AAA flag to guard against Chromium layout tests. I also set that flag in this CL so theoretically this CL won't trigger any GM changes. I'll use this to verify my guard, and remove that flag and actually enables concave AAA in a future CL.
When enabled, for most simple concave paths (e.g., rectangle stroke, rrect stroke, sawtooth, stars...), the Analytic AA achieves 1.3x-2x speedup, and they look much prettier. And they probably are the majority in our use cases by number. But they probably are not the majority by time cost; a single complicated path may cost 10x-100x more time to render than a rectangle stroke... For those complicated paths, we fall back to supersampling by default as we're likely to be 1.1-1.2x slower and the quality improvement is not visually significant. However, one can use gSkForceAnalyticAA to disable that fallback.
BUG=skia:
Change-Id: If9549a3acc4a187cfaf7eb51890c148da3083d31
Reviewed-on: https://skia-review.googlesource.com/6091
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Previously, the clipRect is either equal to nullptr or clipRgn's bound
(after necessary supersampling shift). Hence we drop one of them to make
the signature simpler.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4874
Change-Id: Ied8d5313809d6cf90374365b01f2b8d52f2236e2
Reviewed-on: https://skia-review.googlesource.com/4874
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
I also had to cut it down to just a global atomic bool... as a field in a global singleton accessed through instance(), it's very hard to make threadsafe.
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Release-TSAN-Trybot
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2937
Change-Id: If80be987906dd521fbe644d1d0d577009f06d0e3
Reviewed-on: https://skia-review.googlesource.com/2937
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Yuqian Li <liyuqian@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Use an inline function that does a normal shift. When built for the sanitizer, add casts so that the shift is unsigned.
Also make a few fixes to do unsigned shifts or avoid the shift altogether; and add an argument spec to some macros.
R=reed@google.com,mtklein@google.com
BUG=skia:4633
Review URL: https://codereview.chromium.org/1503423003
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
