If a quad a conic intersect only where the end of one
is contained by the convex hull of the other, and the
curve contained by the hull is nearly a straight line,
treating it as a line may move the end point to the
other side of the curve.
Detect this by checking to see if the end point is in
the hull, and if so, continue to subdivide the curve
rather than treating it as a line.
This fixes several existing tests that were disabled
earlier this year.
A typo in SkDCurve::nearPoint() prevented detecting when
the end of a line was nearly touching a curve.
Also fixed concidence a bit to get the second half of
tiger further along.
All existing tests, including extended testing in
Release and the first half of tiger, work.
TBR=reed@google.com
BUG=skia:5131
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2338323002
Review-Url: https://codereview.chromium.org/2338323002
The path writer takes constructs the output path out of
curves that satisfy the pathop operation.
Curves contain lists of t/point pairs that may not be
comparable to each other. To match up curve ends in the
output path, look for adjacent curves to have a shared
membership rather than comparing point values.
Use path utilities to connect partial curve lists into
closed contours.
Share the angle code that determines if a curve has become
a degenerate line with the path writer.
Clean up some code on the way, and delete some unused
functions.
TBR=reed@google.com
BUG=5188
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321973005
Review-Url: https://codereview.chromium.org/2321973005
Also: make sure that all SkPDF unit tests are named SkPDF_* to
make testing changes to SkPDF easier. Other cleanup.
Add test: SkPDF_pdfa_document to verify that flag in public API
works.
SkPDF_JpegIdentification test: test slightly malformed JPEGs to
verify that all code paths work.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2322133003
Review-Url: https://codereview.chromium.org/2322133003
We were effectively storing the transpose, which made all of our
operations on individual colors, and our concatenation of matrices
awkward and backwards.
I'm planning to push this further into Ganesh, where I had incorrectly
adjusted to the previous layout, treating colors as row vectors in the
shaders.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2324843003
Review-Url: https://codereview.chromium.org/2324843003
during curve intersection if their
ends are nearly the same.
Loosen conic/line intersection point
check.
Detect when coincident points are
unordered. This means that points
a/b/c on one curve may appear in
b/c/a order on the opposite curve.
Restructure addMissing to return
success and return if a coincidence
was added as a parameter.
With this, tiger part a works.
Tiger part b exposes bugs around
tight quads that are nearly coincident
with themselves, and are coincident
with something else.
The greedy coicident matcher
may cause the point order to be
out of sync.
Still working out what to do in
this case.
TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2321773002
Review-Url: https://codereview.chromium.org/2321773002
Reason for revert:
Broken perf bots
Original issue's description:
> Checking for valid colorType, alphaType, colorSpace in SkCodec
>
> * Refactor to share code between SkPngCodec and SkWebpCodec
> * Didn't end up sharing with SkJpegCodec but did refactor
> that code a bit
> * Disallow conversions to F16 with non-linear color spaces
> * Fail to decode if we fail to create a SkColorSpaceXform
> (should be an assert soon). We used to fallback on a
> legacy decode if we failed to create the transform.
> * A bunch of name changes
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2319293003
>
> Committed: https://skia.googlesource.com/skia/+/7a9900d6d34e437bb24beb5524a1f6488ae138c9TBR=scroggo@google.com,brianosman@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review-Url: https://codereview.chromium.org/2328663002
* Refactor to share code between SkPngCodec and SkWebpCodec
* Didn't end up sharing with SkJpegCodec but did refactor
that code a bit
* Disallow conversions to F16 with non-linear color spaces
* Fail to decode if we fail to create a SkColorSpaceXform
(should be an assert soon). We used to fallback on a
legacy decode if we failed to create the transform.
* A bunch of name changes
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2319293003
Review-Url: https://codereview.chromium.org/2319293003
* Necessary to read ICC profiles
* Will be necessary to implement animation
* Requires consolidated data with length
Doesn't affect decode performance (thought I believe
our performance tests only cover SkStreams with memory
bases).
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2311793004
Review-Url: https://codereview.chromium.org/2311793004
Consolidates all flush actions into GrDrawingManager and makes GrContext::flush a passthrough.
Removes the unused and untested discard flush variation.
Replaces the indirect overbudget callback mechanism of GrResourceCache with a flag set by resource cache when it wants to flush that is checked after each draw by GrDrawContext.
Modifies GrResourceCache::notifyFlushOccurred() to take a param indicating whether it triggered the
flush that just occurred.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2307053002
Committed: https://skia.googlesource.com/skia/+/1dbb207babecdae8f1f74ed9d9900c73064df744
Review-Url: https://codereview.chromium.org/2307053002
Reason for revert:
Causing assertions on bots
Original issue's description:
> Restructure flushing relationship between GrContext, GrDrawingManager, and GrResourceCache.
>
> Consolidates all flush actions into GrDrawingManager and makes GrContext::flush a passthrough.
>
> Removes the unused and untested discard flush variation.
>
> Replaces the indirect overbudget callback mechanism of GrResourceCache with a flag set by resource cache when it wants to flush that is checked after each draw by GrDrawContext.
>
> Modifies GrResourceCache::notifyFlushOccurred() to take a param indicating whether it triggered the
> flush that just occurred.
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2307053002
>
> Committed: https://skia.googlesource.com/skia/+/1dbb207babecdae8f1f74ed9d9900c73064df744TBR=robertphillips@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.chromium.org/2312123003
Consolidates all flush actions into GrDrawingManager and makes GrContext::flush a passthrough.
Removes the unused and untested discard flush variation.
Replaces the indirect overbudget callback mechanism of GrResourceCache with a flag set by resource cache when it wants to flush that is checked after each draw by GrDrawContext.
Modifies GrResourceCache::notifyFlushOccurred() to take a param indicating whether it triggered the
flush that just occurred.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2307053002
Review-Url: https://codereview.chromium.org/2307053002
Conics with very large w values can
be approximated with two straight lines.
This avoids iterating endlessly in an
attempt to create quadratics with unstable
numerics.
Check to see if the first chop generated
a pair of lines within the default
point comparison tolerance.
R=reed@google.com
BUG=643933, 643665
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2312923002
Review-Url: https://codereview.chromium.org/2312923002
Pathops makes up intersections that it doesn't detect directly,
but do exist. For instance, if a is coincident with b, and
b is coincident with c, then for where they overlap
a is coincident with c.
The intersections are made up in different ways. In a few
places, the t values that are detected are interpolated to
guess the t values that represent invented intersections.
The interpolated t is not necessarily linear, but a linear
guess is good enough if the invented t lies between known
t values.
Additionally, improve debugging.
This passes the extended release test suite and additionally
passes the first 17 levels in the tiger test suite;
previously, path ops passed 7 levels.
The tiger suite is composed of 37 levels in increasing
complexity, described by about 300K tests.
TBR=reed@google.com
BUG=skia:5131
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2300203002
Review-Url: https://codereview.chromium.org/2300203002
This is working towards fixing all bugs around simplifying the tiger.
This installment simplifies the point-t intersection list as it is built rather than doing the analysis once the intersections are complete. This avoids getting the list in an inconsistent state and makes coincident checks faster and more stable.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2237223002TBR=reed@google.com
BUG=skia:5131
Review-Url: https://codereview.chromium.org/2237223002
This seems to fix text extraction on Adobe Reader
- Registry/Ordering is now set to Skia/SkiaOrdering.
- Type3 fonts now get a FontDescriptor (force symbolic font).
- CMapName is now Skia-Identity-SkiaOrdering
- CMap behaves correctly for single-byte fonts.
Also:
- SkTestTypeface returns tounicode map for testing.
- Unit test updated
All PDFs render the same
BUG=skia:5606
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2292303004
Review-Url: https://codereview.chromium.org/2292303004
Verify the rules that we're converging on for surfaces:
- For 8888, we only support sRGB-like gamma, or no color space at all.
- For F16, we require a color space, with linear gamma.
- For all other formats, we do not support color spaces.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2270823002
Review-Url: https://codereview.chromium.org/2270823002
Instead of mapping invaid glyphIDs to zero or maxGlyphID,
don't draw them at all.
Validate glyphs when glyph is written, not ahead of time.
Don't allocate array to copy user-provided glyphs.
Easy early exit from SkPDFDevice::internalDrawText()
GlyphPositioner::flush() called ~GlyphPositioner()
SkScopeExit class now exists.
Assume SkTypeface* pointers are now never null in more
places.
precalculate alignmentFactor to clean up code.
SkPDFDevice::updateFont must be called with validated
glyphID. Skip bad glyphs to make this true.
SkPDFDevice::updateFont always succeeds.
SkPDFFont::GetFontResource always succeeds (preconditions are
asserted). If GetMetrics fails, don't call GetFontResource.
SkPDFFont::glyphsToPDFFontEncodingCount() becomes
SkPDFFont::countStretch() and is inlined.
SkPDFFont::glyphsToPDFFontEncoding now works one Glyph at a
time and is inlined.
SkPDFFont::noteGlyphUsage() operates one glyph at a time.
Add SkScopeExit.h; also a unit test for it.
SkPostConfig: Fix SK_UNUSED for Win32.
No public API changes.
TBR=reed@google.com
BUG=625995
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2278703002
Review-Url: https://codereview.chromium.org/2278703002
No more:
#include SK_SFNTLY_SUBSETTER
#include ZLIB_INCLUDE
Also, rename SK_SFNTLY_SUBSETTER to SK_PDF_USE_SFNTLY
to follow my pattern of prefixing SkPDF-specific defines
with 'SK_PDF_'.
The ZLIB_INCLUDE define is no longer is used by anyone.
TODO: rename Sfntly to something pronounceable.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2273343002
Review-Url: https://codereview.chromium.org/2273343002
This removes the notion of keeping track of every different t value
that resolves to the same or a similar point. Other fixes make
this concept unnecessary, and removing it simplifies the code.
This removes an allocation, and speeds up paths with many
overlapping curves.
As a bonus, four fuzzer tests that failed before now succeed.
TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2275703003
Review-Url: https://codereview.chromium.org/2275703003
SkColorSpaces may be once-ptrs, so we can reuse common color spaces.
This means that they must be thread safe. SkMatrix44 is not
thread safe because it maintains a mutable type mask.
This CL ensures that we precompute the type mask so we
can use const SkMatrix44's safely.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2277463002
Review-Url: https://codereview.chromium.org/2277463002
I think we convinced ourselves that denorms, while a good chunk of half floats,
cover a rather small fraction of the representable range, which is always
close enough to zero to flush.
This makes both paths of the conversion to or from float considerably simpler.
These functions now work for zero-or-normal half floats (excluding infinite, NaN).
I'm not aware of a term for this class so I've called them "ordinary".
A handful of GMs and SKPs draw differently in --config f16, but all imperceptibly.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2256023002
Review-Url: https://codereview.chromium.org/2256023002
PNGs may contain a gAMA chunk that specifies gamma
values as floats. If so, we will use these floats
to create an SkColorSpace.
This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221983002
Review-Url: https://codereview.chromium.org/2221983002
Motivation: gross code simplification, also no bitset lookups at draw time.
SkPDFFont owns its glyph useage bitset.
SkPDFSubstituteMap goes away.
SkPDFObject interface is simplified.
SkPDFDocument tracks font usage (as hash set), not glyph usage.
SkPDFFont gets a simpler constructor.
SkPDFFont has first and last glyph set in constructor, not adjusted later.
SkPDFFont implementations are simplified.
SkPDFGlyphSet is replaced with simple SkBitSet.
SkPDFFont sizes its SkBitSets based on glyph count.
SkPDFGlyphSetMap goes away.
SkBitSet is now non-copyable.
SkBitSet now how utility methods to match old SkPDFGlyphSet.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2253283004
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Win-MSVC-GCE-CPU-AVX2-x86_64-Release-GDI-Trybot,Test-Win-MSVC-GCE-CPU-AVX2-x86_64-Debug-GDI-Trybot
Review-Url: https://codereview.chromium.org/2253283004
In rare cases, floating point error causes the tesselator to add the
same Vertex to more than one Poly on the same side. This was not a big
problem when we were allocating new vertices when constructing Polys,
but after https://codereview.chromium.org/2029243002 it causes more serious
issues, since each Edge can only belong to two Polys, and violating this
condition messes up the linked list of Edges used for left & right Polys
and the associated estimated vertex count.
The fix is to simply let the first Poly win, and skip that vertex for
subsequent Polys. Since this only occurs in cases where vertices are very
close to each other, it should have little visual effect.
This is also exercised by Nebraska-StateSeal.svg.
BUG=skia:5636
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2259493002
Review-Url: https://codereview.chromium.org/2259493002
Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).
In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250663002
Review-Url: https://codereview.chromium.org/2250663002
GrAppliedClip was about at its limit for how many "make" functions it
could have. Window rectangles would push it over the edge. This change
makes it so GrDrawTarget supplies the original draw bounds to the
constructor, and then GrClip adds the various required clipping
techniques.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246113002
Review-Url: https://codereview.chromium.org/2246113002
Impl Overview
(1) Keep the device clip bounds up to date. This
requires minimal additional work in a few places
throughout canvas.
(2) Keep track of if the ctm isScaleTranslate. Yes,
there's a function that does this, but it's slow
to call.
(3) Perform the src->device transform in quick reject,
then check intersection/nan.
Other Notes:
(1) NaN and intersection checks are performed
simultaneously.
(2) We no longer quick reject infinity.
(3) Affine and perspective are both handled in the slow
case.
(4) SkRasterClip::isEmpty() is handled by the intersection
check.
Performance on Nexus 6P:
93.2ms -> 59.8ms
Overall Android Jank Tests Performance Impact:
Should gain us a ms or two on some tests.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2225393002
Committed: https://skia.googlesource.com/skia/+/d22a817ff57986407facd16af36320fc86ce02da
Review-Url: https://codereview.chromium.org/2225393002
Impl Overview
(1) Keep the device clip bounds up to date. This
requires minimal additional work in a few places
throughout canvas.
(2) Keep track of if the ctm isScaleTranslate. Yes,
there's a function that does this, but it's slow
to call.
(3) Perform the src->device transform in quick reject,
then check intersection/nan.
Other Notes:
(1) NaN and intersection checks are performed
simultaneously.
(2) We no longer quick reject infinity.
(3) Affine and perspective are both handled in the slow
case.
(4) SkRasterClip::isEmpty() is handled by the intersection
check.
Performance on Nexus 6P:
93.2ms -> 59.8ms
Overall Android Jank Tests Performance Impact:
Should gain us a ms or two on some tests.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2225393002
Review-Url: https://codereview.chromium.org/2225393002
Reason for revert:
Erg - dumb bug
Original issue's description:
> Create blurred RRect mask on GPU (rather than uploading it)
>
> This CL doesn't try to resolve any of the larger issues. It just moves the computation of the blurred RRect to the gpu and sets up to start using vertex attributes for a nine patch draw (i.e., returning the texture coordinates)
>
> All blurred rrects using the "analytic" path will change slightly with this CL.
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2222083004
>
> Committed: https://skia.googlesource.com/skia/+/75ccdc77a70ec2083141bf9ba98eb2f01ece2479TBR=bsalomon@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.chromium.org/2236493002
This CL doesn't try to resolve any of the larger issues. It just moves the computation of the blurred RRect to the gpu and sets up to start using vertex attributes for a nine patch draw (i.e., returning the texture coordinates)
All blurred rrects using the "analytic" path will change slightly with this CL.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2222083004
Review-Url: https://codereview.chromium.org/2222083004
SkPDFFont:
- SkPDFType1Font::populate() encode advances correctly.
- break out logically independent code into new files:
* SkPDFConvertType1FontStream
* SkPDFMakeToUnicodeCmap
SkPDFFont.cpp is now 380 lines smaller.
Expose `SkPDFAppendCmapSections()` for testing.
SkPDFFontImpl.h
- Fold into SkPDFFont.
SkPDFConvertType1FontStream:
- Now assume given a SkStreamAsset
SkPDFFont:
- AdvanceMetric now hidden in a anonymous namespace.
No public API changes.
TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221163002
Review-Url: https://codereview.chromium.org/2221163002
About 9x faster than Murmur3 for long inputs.
Most of this is a mechanical change from SkChecksum::Murmur3(...) to SkOpts::hash(...).
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2208903002
CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot;master.client.skia.compile:Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot,Build-Mac-Clang-x86_64-Release-CMake-Trybot
Review-Url: https://codereview.chromium.org/2208903002
SkLiteRecorder, a new SkCanvas, fills out SkLiteDL, a new SkDrawable.
This SkDrawable is a display list similar to SkRecord and SkBigPicture / SkRecordedDrawable, but with a few new design points inspired by Android and slimming paint:
1) SkLiteDL is structured as one big contiguous array rather than the two layer structure of SkRecord. This trades away flexibility and large-op-count performance for better data locality for small to medium size pictures.
2) We keep a global freelist of SkLiteDLs, both reusing the SkLiteDL struct itself and its contiguous byte array. This keeps the expected number of mallocs per display list allocation <1 (really, ~0) for cyclical use cases.
These two together mean recording is faster. Measuring against the code we use at head, SkLiteRecorder trends about ~3x faster across various size pictures, matching speed at 0 draws and beating the special-case 1-draw pictures we have today. (I.e. we won't need those special case implementations anymore, because they're slower than this new generic code.) This new strategy records 10 drawRects() in about the same time the old strategy took for 2.
This strategy stays the winner until at least 500 drawRect()s on my laptop, where I stopped checking.
A simpler alternative to freelisting is also possible (but not implemented here), where we allow the client to manually reset() an SkLiteDL for reuse when its refcnt is 1. That's essentially what we're doing with the freelist, except tracking what's available for reuse globally instead of making the client do it.
This code is not fully capable yet, but most of the key design points are there. The internal structure of SkLiteDL is the area I expect to be most volatile (anything involving Op), but its interface and the whole of SkLiteRecorder ought to be just about done.
You can run nanobench --match picture_overhead as a demo. Everything it exercises is fully fleshed out, so what it tests is an apples-to-apples comparison as far as recording costs go. I have not yet compared playback performance.
It should be simple to wrap this into an SkPicture subclass if we want.
I won't start proposing we replace anything old with anything new quite yet until I have more ducks in a row, but this does look pretty promising (similar to the SkRecord over old SkPicture change a couple years ago) and I'd like to land, experiment, iterate, especially with an eye toward Android.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2213333002
Review-Url: https://codereview.chromium.org/2213333002
Change SkDataTable::NewXXX to SkDataTable::MakeXXX and return sk_sp.
This updates users of SkDataTable to sk_sp as well.
There do not appear to be any external users of these methods.
Review-Url: https://codereview.chromium.org/2211143002
With the move from SkData::NewXXX to SkData::MakeXXX most
SkAutoTUnref<SkData> were changed to sk_sp<SkData>. However,
there are still a few SkAutoTUnref<SkData> around, so clean
them up.
Review-Url: https://codereview.chromium.org/2212493002
Motivation: reduce code complexity.
SkCanon stores SkPDFShader::State next to SkDFObject, not inside.
many places use sk_sp<T> rather than T* to represent ownership.
SkPDFShader::State no longer holds bitmap.
SkPDFShader::State gets move constructor, no longer heap-allocated.
Classes removed:
SkPDFFunctionShader
SkPDFAlphaFunctionShader
SkPDFImageShader
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2193973002
Review-Url: https://codereview.chromium.org/2193973002
Lessons learned
1. ImageShader (correctly) always compresses (typically via PNG) during serialization. This has the surprise results of
- if the image was marked opaque, but has some non-opaque pixels (i.e. bug in blitter or caller), then compressing may "fix" those pixels, making the deserialized version draw differently. bug filed.
- 565 compressess/decompresses to 8888 (at least on Mac), which draws differently (esp. under some filters). bug filed.
2. BitmapShader did not enforce a copy for mutable bitmaps, but ImageShader does (since it creates an Image). Thus the former would see subsequent changes to the pixels after shader creation, while the latter does not, hence the change to the BlitRow test to avoid this modify-after-create pattern. I sure hope this prev. behavior was a bug/undefined-behavior, since this CL changes that.
BUG=skia:5595
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2195893002
Review-Url: https://codereview.chromium.org/2195893002
