For oval paths, GrPath ignores the point order and only uses the bounds
when building its key. This is problematic because
1) point order is important when dashing
2) GrStencilAndCoverPathRenderer asserts that the lookup SkPath is equal
to the cached SkPath - which is not the case for ovals with different
directions/different point order.
With this CL we no longer use the reduced oval key when dashing, and
instead fall through to the more general path cases. The assert is
adjusted to accommodate "equivalent" ovals (when not dashing).
Also re-enabled & updated the GpuDrawPath unit test (disabled in
https://codereview.chromium.org/1456463003/, presumably due to the use
of uninitialized SkRects).
R=bsalomon@google.com,robertphillips@google.com,cdalton@nvidia.com
Review URL: https://codereview.chromium.org/1457073002
Reason for revert:
Causes failures on Android and Win8:
...
( 137/1245MB 9) 73.9ms unit test GpuLayerCachec:\0\build\slave\workdir\build\skia\include\private\skuniqueptr.h:164: failed assertion "get() != pointer()"
Caught exception 2147483651 EXCEPTION_BREAKPOINT
...
Original issue's description:
> Fix NVPR assert for equivalent ovals
>
> For oval paths, GrPath ignores the point order and only uses the bounds
> when building its key. This is problematic because
>
> 1) point order is important when dashing
> 2) GrStencilAndCoverPathRenderer asserts that the lookup SkPath is equal
> to the cached SkPath - which is not the case for ovals with different
> directions/different point order.
>
> With this CL we no longer use the reduced oval key when dashing, and
> instead fall through to the more general path cases. The assert is
> adjusted to accommodate "equivalent" ovals (when not dashing).
>
> Also re-enabled & updated the GpuDrawPath unit test (disabled in
> https://codereview.chromium.org/1456463003/, presumably due to the use
> of uninitialized SkRects).
>
> R=bsalomon@google.com,robertphillips@google.com,cdalton@nvidia.com
>
> Committed: https://skia.googlesource.com/skia/+/f9b1577d763988ebc043ddabf80674f71571ecffTBR=bsalomon@google.com,cdalton@nvidia.com,robertphillips@google.com,fmalita@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1461913002
For oval paths, GrPath ignores the point order and only uses the bounds
when building its key. This is problematic because
1) point order is important when dashing
2) GrStencilAndCoverPathRenderer asserts that the lookup SkPath is equal
to the cached SkPath - which is not the case for ovals with different
directions/different point order.
With this CL we no longer use the reduced oval key when dashing, and
instead fall through to the more general path cases. The assert is
adjusted to accommodate "equivalent" ovals (when not dashing).
Also re-enabled & updated the GpuDrawPath unit test (disabled in
https://codereview.chromium.org/1456463003/, presumably due to the use
of uninitialized SkRects).
R=bsalomon@google.com,robertphillips@google.com,cdalton@nvidia.com
Review URL: https://codereview.chromium.org/1457073002
The general SkImage features seem to be tested in
ImageTest instead of SurfaceTest.
Helps in reviewing further reformatting of SurfaceTest.
BUG=skia:2992
Review URL: https://codereview.chromium.org/1452123002
- streamline the testing down to just byte multiplies
(that's always where the blend algorithms vary)
- add another approximate multiply (x*y+255)>>8
- add another variant of the perfect multiply, ((x*y+128)*257)>>16
I've realized ((x*y+128)*257)>>16 might be just as fast in SSE/NEON
as our current (x*y+x)>>8 approximation. Good to be testing it here.
BUG=skia:
Review URL: https://codereview.chromium.org/1453043005
This shouldn't really make any difference but allocating and holding on to a GrRenderTarget for each test target generates image differences for Mali GPUs. This CL allows an existing render target to be used for the test target.
TBR=bsalomon@google.com
Review URL: https://codereview.chromium.org/1447113002
This CL:
Fixes an ordering problem w.r.t. drawTarget clean up in GrContext::abandonContext (for text test that abandons context)
Fixes when the lastDrawTarget field is set on a RenderTarget (now in GrDrawTarget ctor) due to GrTestTarget use case
Updates the ProgramUnitTest to use multiple drawTargets
Adds renderTarget creation to GrTestTargets (in MDB drawTargets require them)
BUG=skia:4094
Committed: https://skia.googlesource.com/skia/+/9c8605144a0f15e3e69a4e1dcd5d3e63f339380e
Review URL: https://codereview.chromium.org/1441533003
Reason for revert:
Speculative revert for leaks based on ASAN bot (and some local valgrind testing)
Original issue's description:
> Update testing frameworks/tests for MDB
>
> This CL:
>
> Fixes an ordering problem w.r.t. drawTarget clean up in GrContext::abandonContext (for text test that abandons context)
> Fixes when the lastDrawTarget field is set on a RenderTarget (now in GrDrawTarget ctor) due to GrTestTarget use case
> Updates the ProgramUnitTest to use multiple drawTargets
> Adds renderTarget creation to GrTestTargets (in MDB drawTargets require them)
>
> BUG=skia:4094
>
> Committed: https://skia.googlesource.com/skia/+/9c8605144a0f15e3e69a4e1dcd5d3e63f339380eTBR=joshualitt@chromium.org,joshualitt@google.com,robertphillips@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:4094
Review URL: https://codereview.chromium.org/1450513002
This CL:
Fixes an ordering problem w.r.t. drawTarget clean up in GrContext::abandonContext (for text test that abandons context)
Fixes when the lastDrawTarget field is set on a RenderTarget (now in GrDrawTarget ctor) due to GrTestTarget use case
Updates the ProgramUnitTest to use multiple drawTargets
Adds renderTarget creation to GrTestTargets (in MDB drawTargets require them)
BUG=skia:4094
Review URL: https://codereview.chromium.org/1441533003
SkChecksum::Compute is a very, very poorly distributed hash function.
This replaces all remaining uses with Murmur3.
The only interesting stuff is in src/gpu.
BUG=skia:
Review URL: https://codereview.chromium.org/1436973003
Unit tests added to check all color types in SkOneShotDiscardablePixelref
SkResourceCacheTest seems to have been allways using default Heap Allocator.
Fixed so that it uses private (not global that is) SkDiscardableMemory.
BUG=skia:4355
Review URL: https://codereview.chromium.org/1426753006
- remove float -> int conversion, keeping float -> byte
- remove support for doubles
I was thinking of specializing Sk8f for AVX. This will help keep the complexity down.
This may cause minor diffs in radial gradients: toBytes() rounds where castTrunc() truncated. But I don't see any diffs in Gold.
https://gold.skia.org/search2?issue=1411563008&unt=true&query=source_type%3Dgm&master=false
BUG=skia:4117
CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
Review URL: https://codereview.chromium.org/1411563008
Avoid hang in OncePtr test when using "dm --threads 1".
The test will hang the threads until sk_num_cores() threads have run
the code. This requires that sk_num_cores() threads to be run in
parallel, which the global thread pool will not do if the thread count
is smaller than sk_num_cores().
BUG=skia:
Review URL: https://codereview.chromium.org/1419593004
Reason for revert:
failing on msaa
Original issue's description:
> Revert[2] of "stop using drawSprite (at least w/ no filters) as it is going away"
>
> Fixed assert in test that was calling GrRecordReplaceDraw(). That function now uses
> drawBitmap, so updated the test to check for that (instead of drawSprite).
>
> This reverts commit 21b7663470.
>
> BUG=skia:
> TBR=
>
> Committed: https://skia.googlesource.com/skia/+/7b81994d95f10cda80ddb85af68a2651ff31782f
TBR=
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review URL: https://codereview.chromium.org/1420053009
In SkSampledCodec, allow the native codec to do its scaling first, then
sample on top of that. Since the only codec which can do native scaling
is JPEG, and we know what it can do, hard-code for JPEG. Check to see
if the sampleSize is something JPEG supports, or a multiple of
something it supports. If so, use JPEG directly or combine them.
BUG=skia:4320
Review URL: https://codereview.chromium.org/1417583009
Fixed assert in test that was calling GrRecordReplaceDraw(). That function now uses
drawBitmap, so updated the test to check for that (instead of drawSprite).
This reverts commit 21b7663470.
BUG=skia:
TBR=
Review URL: https://codereview.chromium.org/1410343011
An additional positive of this CL is that GrGLShaderBuilder is now GL independent besides GrGLProgramBuilder
BUG=skia:
Review URL: https://codereview.chromium.org/1431433003
The utility SubsetPath removes parts of a potentially very large path to isolate a minimal test case. SubsetPath is very useful for debugging path ops, but is not path ops specific.
PathOpsBuilderConicTest compares the output of the Path Ops Builder, sequential calls to Simplify, and SkRegions for some number of rotated ovals.
Some tests caused path ops to hang. It was caught adding a loop of curves because the head was not found by the tail. Even though the root cause has been fixed, SkSegment::addCurveTo callers now abort the path op if the same curve was added twice.
The subdivided conic weight was been computed anew. Fortunately, it's a simpler computation that the one it replaces.
Some Simplify() subroutines returned false to signal that the results needed assembling. Change these to abort the current operation instead.
Coincident curve intersection triggered two small bugs; one where no perpendicular could be found for coincident curves, and one where no coincident curves remain after looping.
The SixtyOvals test can be run through multiple processes instead of multiple threads. This strategy allows a 48 core machine to saturate all cores at 100%.
The DEBUG_VISUALIZE_CONICS code in PathOpsConicIntersectionTest acknowleges that it is easier to visualize conics with Skia than with script and html canvas. This test also verifies that path ops subdivision matches geometry chopping.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/1405383004
Motivation: This will be easier than adding a friend every time I want
to create a one-off SkCanvas subclass or SkRemote::Encoder subclass.
See also: SkPath::Iter.
Review URL: https://codereview.chromium.org/1411723005
This should be a strict refactor, just pulling out the bounds array.
(It's the rescued nice parts of a dead-end CL targeting skia:4492.)
BUG=skia:
Review URL: https://codereview.chromium.org/1424553002
Move convert_CMYK_to_RGBA into two functions in SkSwizzler: one for 565
and one for 8888.
For simplicity, when converting to 8888, we no longer convert in place.
BUG=skia:4476
Review URL: https://codereview.chromium.org/1411083009
Reason for revert:
suppressions have landed in chrome
Original issue's description:
> Revert of path ops: fix conic weight and partial coincidence (patchset #5 id:80001 of https://codereview.chromium.org/1413763002/ )
>
> Reason for revert:
> path ops change breaks svg clipping layout tests -- conic is now more accurate, changing edge of circle in clip
>
> These need to be rebaselined
>
> svg/clip-path/clip-path-child-clipped.svg
> svg/clip-path/clip-path-nonzero.svg
> svg/clip-path/clip-path-evenodd-nonzero.svg
> svg/clip-path/clip-path-nonzero-evenodd.svg
>
> Original issue's description:
> > The remaining 1m skp bugs are asserts that can be harmlessly
> > suppressed and bugs around conics.
> >
> > The conic calculation for a subdivided w was just wrong.
> >
> > Also added debugging to template intersection to initialize
> > reused structures and dump additional data.
> >
> > TBR=reed@google.com
> >
> > Committed: https://skia.googlesource.com/skia/+/ef33b1e739b23a1201100ff17a572da85b03d9af
>
> TBR=
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
>
> Committed: https://skia.googlesource.com/skia/+/f428df1be3e96d3f8970d0f7f415b862f7da5404
TBR=
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1407003016
We will implement this API using SkCodecs.
SkAndroidCodecs will be used to implement the
BitmapRegionDecoder Java API (and possibly
BitmapFactory).
BUG=skia:
Review URL: https://codereview.chromium.org/1406223002
Reason for revert:
path ops change breaks svg clipping layout tests -- conic is now more accurate, changing edge of circle in clip
These need to be rebaselined
svg/clip-path/clip-path-child-clipped.svg
svg/clip-path/clip-path-nonzero.svg
svg/clip-path/clip-path-evenodd-nonzero.svg
svg/clip-path/clip-path-nonzero-evenodd.svg
Original issue's description:
> The remaining 1m skp bugs are asserts that can be harmlessly
> suppressed and bugs around conics.
>
> The conic calculation for a subdivided w was just wrong.
>
> Also added debugging to template intersection to initialize
> reused structures and dump additional data.
>
> TBR=reed@google.com
>
> Committed: https://skia.googlesource.com/skia/+/ef33b1e739b23a1201100ff17a572da85b03d9af
TBR=
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1408923003
suppressed and bugs around conics.
The conic calculation for a subdivided w was just wrong.
Also added debugging to template intersection to initialize
reused structures and dump additional data.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/1413763002
It's only used by a couple unit tests. We have other ways of getting
the same quality testing of our ref-count code now (e.g. TSAN).
BUG=skia:
CQ_EXTRA_TRYBOTS=client.skia.compile:Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot,Build-Mac10.9-Clang-x86_64-Release-CMake-Trybot
Review URL: https://codereview.chromium.org/1408213005
Now, that we are subsetting, fX0 is not necessarily less than
fSrcWidth (since fSrcWidth is really the subset width).
Ex: We may want a 10 pixel subset starting twenty pixels from the
left edge. In that case, fX0=20 and fSrcWidth=10.
Let's rename the width as fSubsetWidth to avoid confusion
and remove the check.
BUG=skia:
Review URL: https://codereview.chromium.org/1407603003
Iterating through the 903K skps that represent the
imagable 1M top web pages triggers a number of
bugs, some of which are addressed here.
Some web pages trigger intersecting cubic
representations of arc with their conic
counterparts. This exposed a flaw in coincident
detection that caused an infinite loop. The loop
alternatively extended the coincident section and,
determining the that the bounds of the curve pairs
did not overlap, deleted the extension.
Track the number of times the coincident detection
is called, and if it exceeds an empirically found
limit, assume that the curves are coincident and
force it to be so.
The loop count limit can be determined by enabling
DEBUG_T_SECT_LOOP_COUNT and running all tests. The
largest count is reported on completion.
Another class of bugs was caused by concident
detection duplicating nearly identical points that
had been merged earlier. To track these bugs, the
'handle coincidence' code was duplicated as a
const debug variety that reported if one of a
dozen or so irregularities are present; then it is
easier to see when a block of code that fixes one
irregularity regresses another.
Creating the debug const code version exposed some
non-debug code that could be const, and some that
was experimental and could be removed. Set
DEBUG_COINCIDENCE to track coincidence health and
handling.
For running on Chrome, DEBUG_VERIFY checks the
result of pathops against the same operation
using SkRegion to verify that the results are
nearly the same.
When visualizing the pathops work using
tools/pathops_visualizer.htm, set
DEBUG_DUMP_ALIGNMENT to see the curves after
they've been aligned for coincidence.
Other bugs fixed include detecting when a
section of a pair of curves have devolved into
lines and are coincident.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/1394503003
Passing &SkGoodHash to SkTHashMap and SkTHashSet doesn't guarantee that it's actually instantiated. Using a functor does.
BUG=skia:
Review URL: https://codereview.chromium.org/1405053002
At the end of TightRunBounds, glyphPosX cannot exceed the start of the
next run. But glyphPosY is running ahead of glyphPosX (for
kFull_Positioning) => the glyphPosY assert is incorrect.
Since the two pointers advance in lock-step, there isn't much value in
the glyphPosY assert anyway - we might as well remove it.
BUG=chromium:542643
R=reed@google.com,bungeman@google.com
Review URL: https://codereview.chromium.org/1405463004
Result:
- clients can get a filtered version of an image without having to setup a temp drawing environment
- for some cases, the process is more efficient even than (deprecated) drawSprite, since there is no need to draw/copy the result
Impl:
- made Proxy virtual so we don't need to have an existing device to use it
This, in conjunction with LocalMatrixImageFilter, should allow us to simplify and optimize ApplyImageFilter() in cc/output/gl_renderer.cc
BUG=skia:
Review URL: https://codereview.chromium.org/1390913005
Rather than implementing some sort of "fill" in every
SkCodec subclass for incomplete images, let's make the
parent class handle this situation.
This includes an API change to SkCodec.h
SkCodec::getScanlines() now returns the number of lines it
read successfully, rather than an SkCodec::Result enum.
getScanlines() most often fails on an incomplete input, in
which case it is useful to know how many lines were
successfully decoded - this provides more information than
kIncomplete vs kSuccess. We do lose information when the
API is used improperly, as we are no longer able to return
kInvalidParameter or kScanlineNotStarted.
Known Issues:
Does not work for incomplete fFrameIsSubset gifs.
Does not work for incomplete icos.
BUG=skia:
Review URL: https://codereview.chromium.org/1332053002
The primary goal of SkScaledCodec is to replace the current
implementation of BitmapRegionDecoder, which depends on modified
versions of libjpeg and libpng, with an implementation that uses
standard versions of the libaries. Since BitmapRegionDecoder only
supports PNG, WEBP and JPEG, limit SkScaledCodec to those classes.
We will focus on those three until we complete this primary goal.
Then we can continue to make SkScaledCodec work for other formats.
Fix some bugs in SkScaledCodec::NewFromStream:
- Handle a NULL input stream properly
- Ensure that the input stream is deleted as expected on bad data
Add tests for these error cases.
BUG=skia:4428
Review URL: https://codereview.chromium.org/1389053002
Reason for revert:
Breaks Chrome with this link error: ../../third_party/skia/include/effects/SkMorphologyImageFilter.h:75: error: undefined reference to 'SkMorphologyImageFilter::SkMorphologyImageFilter(int, int, SkImageFilter*, SkImageFilter::CropRect const*)'
../../third_party/skia/include/effects/SkMorphologyImageFilter.h:104: error: undefined reference to 'SkMorphologyImageFilter::SkMorphologyImageFilter(int, int, SkImageFilter*, SkImageFilter::CropRect const*)'
Presumably due to code in third_party/WebKit/Source/platform/graphics/filters/FEMorphology.cpp that contains:
#include "SkMorphologyImageFilter.h"
...
if (m_type == FEMORPHOLOGY_OPERATOR_DILATE)
return adoptRef(SkDilateImageFilter::Create(radiusX, radiusY, input.get(), &rect));
return adoptRef(SkErodeImageFilter::Create(radiusX, radiusY, input.get(), &rect));
Original issue's description:
> factories should return baseclass, allowing the impl to specialize
>
> waiting on https://codereview.chromium.org/1386163002/# to land
>
> BUG=skia:4424
>
> Committed: https://skia.googlesource.com/skia/+/80a6dcaa1b757826ed7414f64b035d512d9ccbf8TBR=senorblanco@google.com,robertphillips@google.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:4424
Review URL: https://codereview.chromium.org/1389063002
To further consolidate the various unique owning classes, this bases
SkAutoTUnref on skstd::unique_ptr. Users are updated because of two
breaking changes, swap now takes a reference and reset no longer
returns its argument.
Review URL: https://codereview.chromium.org/1370803002
drawImage calls now properly embeds the original jpeg.
NOTE: drawBitmap*() calls no longer embed JPEG files when
possible (this is in advance of eliminating bitmaps backed
by encoded data). Chromium has already moved from
drawBitmap to drawImage.
Comparisons:
control:
total PDF drawImage/drawBitmap calls: 8010
total PDF jpeg images: 0
total PDF regular images: 3581
experiament:
total PDF drawImage/drawBitmap calls: 8014
total PDF jpeg images: 271
total PDF regular images: 3311
total PDF regular images: 3582 (271 + 3311)
When comparing rendered output there were perceptual
differences in the following four GMs: colorcube, emboss,
colormatrix, and tablecolorfilter. All of these differences
were improvements (that is, closer to the 8888 rendering)
due fixing a bug with colorfilters and forgetting to call
notifyPixelsChanged.
No SKPs had perceptual differences.
Total PDF size dropped from 133964 kB to 126276 kB, a 5.7%
improvement (mostly due to restoring use of JPG images in
SKPs).
BUG=skia:4370
Review URL: https://codereview.chromium.org/1372783003
Benefits:
- This mimics other decoding APIs (including the ones SkCodec relies
on, e.g. a png_struct, which can be used to decode an entire image or
one line at a time).
- It allows a client to ask us to do what we can do efficiently - i.e.
start from encoded data and either decode the whole thing or scanlines.
- It removes the duplicate methods which appeared in both SkCodec and
SkScanlineDecoder (some of which, e.g. in SkJpegScanlineDecoder, just
call fCodec->sameMethod()).
- It simplifies moving more checks into the base class (e.g. the
examples in skbug.com/4284).
BUG=skia:4175
BUG=skia:4284
=====================================================================
SkScanlineDecoder.h/.cpp:
Removed.
SkCodec.h/.cpp:
Add methods, enums, and variables which were previously in
SkScanlineDecoder.
Default fCurrScanline to -1, as a sentinel that start has not been
called.
General changes:
Convert SkScanlineDecoders to SkCodecs.
General changes in SkCodec subclasses:
Merge SkScanlineDecoder implementation into SkCodec. Most (all?) owned
an SkCodec, so they now call this-> instead of fCodec->.
SkBmpCodec.h/.cpp:
Replace the unused rowOrder method with an override for
onGetScanlineOrder.
Make getDstRow const, since it is called by onGetY, which is const.
SkCodec_libpng.h/.cpp:
Make SkPngCodec an abstract class, with two subclasses which handle
scanline decoding separately (they share code for decoding the entire
image). Reimplement onReallyHasAlpha so that it can return the most
recent result (e.g. after a scanline decode which only decoded part
of the image) or a better answer (e.g. if the whole image is known to
be opaque).
Compute fNumberPasses early, so we know which subclass to instantiate.
Make SkPngInterlaceScanlineDecoder use the base class' fCurrScanline
rather than a separate variable.
CodexTest.cpp:
Add tests for the state changes in SkCodec (need to call start before
decoding scanlines; calling getPixels means that start will need to
be called again before decoding more scanlines).
Add a test which decodes in stripes, currently only used for an
interlaced PNG.
TODO: Add tests for onReallyHasAlpha.
Review URL: https://codereview.chromium.org/1365313002
SkGlyphCache dumps too many glyphs, and to reduce the trace size,
RequestDetails enum is added to SkTraceMemoryDump interface. This
would tell caches to dump only requested details or not a detailed
dump.
BUG=499731
Review URL: https://codereview.chromium.org/1310123007
An alternative way of addressing this is to alter SkCanvas::drawRoundRect to just reject isEmpty (i.e., un-sorted or truly empty) input rects.
BUG=skia:3786
Review URL: https://codereview.chromium.org/1373293002
- use C++11 features ({} init, move constructors) to eliminate the need
for explicit constructors
- collapse RECORD0...RECORD8 into just one RECORD macro
- explicitly tag record types instead of using member detectors.
Removing member detectors makes this code significantly less fragile.
This exposes a few places where we didn't really think through what to do
with SkDrawable. I've marked them TODO for now.
BUG=skia:
Review URL: https://codereview.chromium.org/1360943003
Reason for revert:
Need to somehow get access to encoders in chrome -- link error on the roll since SkImageEncoder is not built as part of chrome.
Original issue's description:
> change pixel-serializer to support reencoding existing data
>
> Trying to evolve this interface so it can
> - support rich set of backend-encoders (including ones like ETC1 that can cheaply convert to KXT
> - allow for encoding images as well as bitmaps (e.g. for picture serialization)
> - perhaps replace SkImageEncoder as an API (assuming we create a factory that returns a serializer given a format)
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/13f48dc85aa68a60da66aaf39c93d527d11d1278TBR=scroggo@google.com,msarett@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review URL: https://codereview.chromium.org/1371983003
Trying to evolve this interface so it can
- support rich set of backend-encoders (including ones like ETC1 that can cheaply convert to KXT
- allow for encoding images as well as bitmaps (e.g. for picture serialization)
- perhaps replace SkImageEncoder as an API (assuming we create a factory that returns a serializer given a format)
BUG=skia:
Review URL: https://codereview.chromium.org/1373683003
The test "nullptr == fBlockMemory->fHead->fNext" tests that
all of the data is in one block, but it is also necessary to
test "fBlockMemory->fHead" to ensure that there is a first
block with data in it.
Review URL: https://codereview.chromium.org/1374493002
One side effect is that the SkShader's (or implicit shader's) fragment processor is responsible for the transition from an unpremul paint color to a premul color.
Review URL: https://codereview.chromium.org/1348583002
These tests were isolated from their respective minimized test cases.
The tests work fine and pass path ops internal validation;
hopefully some more intensive x-san or valgrind test will help
isolate the bug.
Sheriff, please revert if it fails and I don't get to it first.
TBR=reed@google.com,halcanary@google.com
BUG=535127,535151
Review URL: https://codereview.chromium.org/1359263003
Motivation: I want too finalize this API before working on the more
complex problem of adding XMP metadata for PDF/A.
BUG=skia:3110
Review URL: https://codereview.chromium.org/1359943003
libjpeg-turbo turns on dithering by default. When we
decode to RGBA, it uses Floyd-Steinberg dithering -
consistent with the libjpeg6b gold standard,
When we decode to 565, it uses ordered dithering. Ordered
dithering for 565 is not part of the 6b standard (libjpeg6b
doesn't even support 565) and is causing us a number of
issues:
(1) Ordered dithering + nearest neighbor sampling is
causing checkerboard visual artifacts in some outputs.
(2) The ordered dither function in libjpeg-turbo actually
behaves differently depending on the alignment of the
memory that it decodes into. This means that two image
decodes that should be identical may look different just
because they decode into different memory blocks. This
was causing some diffs on Gold with the scanline_subset
test that were causing me some confusion.
(3) Maybe not the best evidence, but visually I can't tell
a difference with and without dithering (except when
nearest neighbor scaling causes the checkerboard artifact).
(4) Turning off dithering should be a more significant
performance improvement than you might expect.
libjpeg-turbo has SIMD color conversions to 565, but when
dithering is on, it defaults to scalar code.
This CL should make every jpeg decode to 565 on Gold look
slightly different. Yay!
BUG=skia:
Review URL: https://codereview.chromium.org/1349563007
The new FP is used to implement SkXM::Mode color filters and SkXM::Mode image filters. Also, these now support all advanced SkXM::Mode xfermodes.
Review URL: https://codereview.chromium.org/1334293003
Blink is migrating away from SkBitmaps, so we need an SkImage-based
SkImageFilter source. This is pretty much a 1-1 equivalent of
SkBitmapSource.
To avoid duplication, relocate the SkImage deserialization logic
from SkPictureData to SkReadBuffer.
R=reed@google.com,robertphillips@google.com,senorblanco@chromium.org
Review URL: https://codereview.chromium.org/1343703005
The new flow of calls for attaching a Stencil looks like:
Client
rt->attachStencilAttachment()
gpu->getStencilAttachment()
glgpu->createStencilAttachment()
glrt->completeStencilAttachment() //actually attaches
BUG=skia:
Review URL: https://codereview.chromium.org/1333383002
Had to move GrComposeEffect class definition from SkComposeShader.cpp to SkComposeShader.h so that GLProgramsTest can call GrComposeEffect::Create()
BUG=skia:4182
Review URL: https://codereview.chromium.org/1314923002
Reason for revert:
DEPS should be fixed now
Original issue's description:
> Revert of more zero-length changes for svg compatibility (patchset #6 id:100001 of https://codereview.chromium.org/1330623003/ )
>
> Reason for revert:
> breaks DEPS
>
> Original issue's description:
> > more zero-length changes for svg compatibility
> >
> > If a path contains a moveTo followed by a line or curve,
> > even if the line or curve has no length, SVG expects
> > the end caps to draw if the cap style is round or square.
> >
> > Fredrik Söderquist attached a patch to the chrome bug
> > (slightly modified here) that fixes layout test failures
> > resulting from deleting special-case code in SVG
> > dealing with zero-length path segments.
> >
> > R=reed@google.com,fs@opera.com
> > BUG=22974
> >
> > Committed: https://skia.googlesource.com/skia/+/62fb1ba1786863e545c89839b5706ad5151cec15
>
> TBR=fs@opera.com,reed@google.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=22974
>
> Committed: https://skia.googlesource.com/skia/+/5ca4fa3846067a47e88d35ace895df3ebe3ec2a5TBR=fs@opera.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=22974
Review URL: https://codereview.chromium.org/1314833004
Reason for revert:
breaks DEPS
Original issue's description:
> more zero-length changes for svg compatibility
>
> If a path contains a moveTo followed by a line or curve,
> even if the line or curve has no length, SVG expects
> the end caps to draw if the cap style is round or square.
>
> Fredrik Söderquist attached a patch to the chrome bug
> (slightly modified here) that fixes layout test failures
> resulting from deleting special-case code in SVG
> dealing with zero-length path segments.
>
> R=reed@google.com,fs@opera.com
> BUG=22974
>
> Committed: https://skia.googlesource.com/skia/+/62fb1ba1786863e545c89839b5706ad5151cec15TBR=fs@opera.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=22974
Review URL: https://codereview.chromium.org/1334543002
Reason for revert:
Breaks Chrome roll.
obj/skia/ext/skia_chrome.skia_memory_dump_provider.o
does not have -I include/private on its include path, but transitively includes SkMessageBus.h.
Original issue's description:
> Port uses of SkLazyPtr to SkOncePtr.
>
> This gives SkOncePtr a non-trivial destructor that uses std::default_delete
> by default. This is overrideable, as seen in SkColorTable.
>
> SK_DECLARE_STATIC_ONCE_PTR still just leaves its pointers hanging at EOP.
>
> BUG=skia:
>
> No public API changes.
> TBR=reed@google.com
>
> Committed: https://skia.googlesource.com/skia/+/a1254acdb344174e761f5061c820559dab64a74cTBR=herb@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Review URL: https://codereview.chromium.org/1334523002
If a path contains a moveTo followed by a line or curve,
even if the line or curve has no length, SVG expects
the end caps to draw if the cap style is round or square.
Fredrik Söderquist attached a patch to the chrome bug
(slightly modified here) that fixes layout test failures
resulting from deleting special-case code in SVG
dealing with zero-length path segments.
R=reed@google.com,fs@opera.com
BUG=22974
Review URL: https://codereview.chromium.org/1330623003
This gives SkOncePtr a non-trivial destructor that uses std::default_delete
by default. This is overrideable, as seen in SkColorTable.
SK_DECLARE_STATIC_ONCE_PTR still just leaves its pointers hanging at EOP.
BUG=skia:
No public API changes.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/1322933005
Image encoding may fail during serialization, resulting in zero-length
encoded data in the SKP.
Instead of invalidating the stream (and preventing deserialization of
the whole picture) we can instantiate placeholder images.
BUG=skia:4285
R=reed@google.com,robertphillips@google.com
Review URL: https://codereview.chromium.org/1308273011
crbug.com/526025 includes a minimized SVG test case.
Translating that test case into native code (fuzzTNG)
did not reproduce the bug. That test case should
have not been included with skia issue 1323813003,
and is deleted here.
Running the minimal test case in a modified version
of chrome isolated the bug. The modified version
generated the test fuzz763_3 with the edit
#define DEBUGGING_PATHOPS_FROM_HOST 1
in src/pathops/SkPathopsOp.cpp line 188.
Rename fuzz763_3 to issue_526025 to associate the test
with the bug. Note that the bug contains the body of the
CL in comment $5.
R=reed@google.com
Review URL: https://codereview.chromium.org/1315503005
This switches over SkXfermodes_opts.h and SkColorMatrixFilter to use Sk4f,
and converts the SkPMFloat benches to Sk4f benches.
No pixels should change here, and no code beyond the Sk4f_ benches should change speed.
The benches are faster than the old versions.
BUG=skia:4117
Review URL: https://codereview.chromium.org/1324743002
The list of intersection points on a curve segment may have
entries that can be safely removed when nearby points have
nearly the same t value and point value. When a path includes
very large curves as well as small ones, as is the case with
this fuzzer, additional points may lie between the similar
points that do not meet the nearby criteria.
After merging the nearby point with its doppelganger,
SkOpSegment::moveNearby() unnecessarily set the doppelganger's
next pointer to the one following the nearby point. While
this usually has no effect, since the merge already updated
the linked list, the explicit call removes the additional
outlier points from the segment.
TBR=reed@google.com
BUG=526025
Review URL: https://codereview.chromium.org/1323813003
The million SKPs generated require >5T of storage. A good deal
of that are copies of system fonts.
Chrome built with
#DEFINE SK_WHITELIST_SERIALIZED_TYPEFACES
will omit the font data if the font matches a precomputed
checksum.
The captured SKP prepends sk_ to the names of fonts that
have their data omitted. The SKP consumer can either add
renamed fonts from the recording machine, or add
gDeserializeTypefaceDelegate = WhitelistDeserializeTypeface;
which strips the sk_ prefix when deserializing typefaces.
whitelist_typefaces --check
Computes the checksums of fallback
fonts and returns 0 if the checksums match the checked-in
file SkWhitelistChecksum.cpp.
whitelist_typefaces --generate
Writes an updated version of SkWhitelistChecksum.cpp.
(Added Mike since this modifies a public header)
R=bungeman@google.com,rmistry@google.com,reed@google.com
Review URL: https://codereview.chromium.org/1317913005
Redesigns SkScanlineDecoder.h to indicate the ordering
in which the scanlines are provided
Refactors SkSwizzler::Fill() to include the zeroInit check
and to actually be correct.
BUG=skia:3257
BUG=skia:4198
Review URL: https://codereview.chromium.org/1287423002
This macro was responsible for producing code like:
static SkAlignedStorage<sizeof(Foo)> g_gFoo_Storage;
static Foo* gFoo = new(g_gFoo_Storage.get()) Foo;
static SkAutoTDestroy<Foo> gFoo_ad(gFoo);
which would allocate static storage for an object of type Foo
(g_gFoo_Storage), lazily instantiate the object in that memory (via
gFoo's initializer), and then ensure that at global destruction time
the object is destroyed (via gFoo_Ad's destructor).
However, the exact same effect is achieved by just writing:
static Foo gFoo;
Review URL: https://codereview.chromium.org/1314763009
Unfortunately, immintrin.h (which is also included by SkTypes)
includes xmmintrin.h which includes mm_malloc.h which includes
stdlib.h for malloc even though, from the implementation, it is
difficult to see why.
Fortunately, arm_neon.h does not seem to be involved in such
shenanigans, so building for Android will keep things sane.
TBR=reed@google.com
Doesn't change Skia API, just moves an include.
Review URL: https://codereview.chromium.org/1313203003
Reason for revert:
This causes a syntax error.
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/87819/steps/compile%20%28with%20patch%29/logs/stdio
Original issue's description:
> Implement canComputeFastBounds() for image filters.
>
> Image filters have never implemented this check, which means that
> filters which affect transparent black falsely claim they can compute
> their bounds.
>
> Implemented an affectsTransparentBlack() virtual for image
> filters, and a similar helper function for color filters.
>
> This will affect the following GMs: imagefiltersscaled
> (lighting, perlin noise now filter to clip),
> colorfilterimagefilter (new test case), imagefiltersclipped
> (perlin noise now filters to clip).
>
> Note: I de-inlined SkPaint::canComputeFastBounds() to avoid adding
> a dependency from SkPaint.h to SkImageFilter.h.h. Skia benches show
> no impact from this change, but will watch the perf bots carefully.
>
> BUG=4212
>
> Committed: https://skia.googlesource.com/skia/+/915881fe743f9a789037695f543bc6ea189cd0cbTBR=reed@google.com,senorblanco@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=4212
Review URL: https://codereview.chromium.org/1300403003
Image filters have never implemented this check, which means that
filters which affect transparent black falsely claim they can compute
their bounds.
Implemented an affectsTransparentBlack() virtual for image
filters, and a similar helper function for color filters.
This will affect the following GMs: imagefiltersscaled
(lighting, perlin noise now filter to clip),
colorfilterimagefilter (new test case), imagefiltersclipped
(perlin noise now filters to clip).
Note: I de-inlined SkPaint::canComputeFastBounds() to avoid adding
a dependency from SkPaint.h to SkImageFilter.h.h. Skia benches show
no impact from this change, but will watch the perf bots carefully.
BUG=4212
Review URL: https://codereview.chromium.org/1296943002
We don't want to test small images on Gold because they are
not interested to look at. Instead, I wrote a unit test to
verify that scaling small images does not cause crashes.
BUG=skia:
Review URL: https://codereview.chromium.org/1287863004
If a curve has the identical start and control points, the
initial or final tangent can't be trivally determined. The
perpendicular to the tangent is used to measure coincidence.
Add logic for cubics, quadratics, and conics, to use the
secondary control points or the end points if the initial
control point alone can't determine the tangent.
Add debugging (currently untriggered by exhaustive testing)
to detect zero-length tangents which are not at the curve
endpoints.
Increase the number of temporary intersecions gathered from
10 to 12 but reduce the max passed in by cubic intersection from
27 to 12. Also, add checks if the max passed exceeds the
storage allocated.
When cleaning up parallel lines, choose the intersection which
is on the end of both segments over the intersection which
is on the end of a single segment.
TBR=reed@google.com
BUG=425140,516266
Review URL: https://codereview.chromium.org/1288863004
Currently, SkPaintFilterCanvas does not provide any help in cloning
target canvas state. While that could be handled in subclasses, it is
easy to miss (see linked bug).
This CL adds a new constructor variant which ensures that the initial
matrix and clip bounds are inherited from the target canvas.
BUG=516790
R=reed@google.com,robertphillips@google.com
Review URL: https://codereview.chromium.org/1294013002
Add more swizzling functions for swizzling to 565. Much of this
code was revived from crrev.com/1055743003 (for BMP). Also added
swizzling functions for WBMP.
Consolidate the static function conversion_possible.
In SkCodec::getPixels, check that the alphatype corresponds to the
colorType. This prevents requesting 565 + non-opaque.
In SkIcoCodec, report that the image is unpremul (instead of
whatever the largest embedded codec thinks), but modify the
requested info to have the alpha type expected/required by the
embedded codec.
Add tests for decoding to 565.
BUG=skia:3257
BUG=skia:3683
Review URL: https://codereview.chromium.org/1277213002
Prior to this patch clients who were solely uploading to textures (e.g., SW Mask Mgr) would cause extra flushes b.c., even though kPreferNoIO was being specified, resources with pending IO would still be returned even though there was plenty of space in the resource cache.
Review URL: https://codereview.chromium.org/1286203002
Rename to SkDeflate so Chromium builders continue to work
Next, we change remove SkFlate from Chromium build files, then we can delete SkFlate.
Review URL: https://codereview.chromium.org/1285913002
Add a static function to CodexTest, which consolidates decoding,
comparing to an expected SkCodec::Result, and optionally comparing to a
digest.
Test decoding non-opaque to opaque (fails) and premul to unpremul/vice
versa (succeeds).
BUG=skia:3475
Review URL: https://codereview.chromium.org/1277253003
Chromium's test suite contains an RLE image that reports a certain
file size in the header, but then contains additional encoded data.
Our bmp decoder would only decode half of the image, before stopping.
With this fix, we check for additional data before returning
kIncompleteInput.
If this lands, I will upload the test image to the bots.
Also adding an invalid image test to CodexTest.
BUG=skia:
Review URL: https://codereview.chromium.org/1273853004
When possible use run-time checks (via SkDocument::CreatePDF)
When PDF is disabled, do not compile tests/PDF*.cpp
Review URL: https://codereview.chromium.org/1278403006
Prepare_rt_for_external_access() grabs the render target and then fires
access notifications.
But the notification handlers may trigger copy-on-write, causing the
returned render target to be stale (pointing at the detached snapshot).
We should grab the render target after firing notifications.
R=reed@google.com,bsalomon@google.com
Review URL: https://codereview.chromium.org/1276713002
We are also changing the wbmp to use SkSwizzler. This
will allow us to take advantage of the sampling routines
that are being implemented in SkSwizzler.
The image in this upload came from:
https://commons.wikimedia.org/wiki/File:Android_robot.png
It is licensed under the Creative Commons Attribution-Share Alike 3.0 Unported license.
BUG=skia:
Review URL: https://codereview.chromium.org/1254483004
Motivation: maintaining this code doesn't seem worth the time,
since no one seems to be using it. If someone wants to use it
in the future, just revert this CL.
Review URL: https://codereview.chromium.org/1266093003
Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached.
Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold.
In order to invalidate the cache when an SkPath is changed or deleted,
this required implementing genID change notification in SkPath. This is
modelled almost exactly on SkPixelRef::GenIDChangeListener.
However, It does not currently implement the check for unique genIDs,
so notifiers will fire when the first instance of an SkPathRef
using a given genID is destroyed.
Another caveat is that you cannot successfully add a change notifier
to an empty path, since it uses the "canonical" empty path which is
never modified or destroyed. For this reason, we prevent adding
listeners to it.
BUG=skia:4121,skia:4122, 497403
DOCS_PREVIEW= https://skia.org/?cl=1114353004
Committed: https://skia.googlesource.com/skia/+/468dfa72eb6694145487be17876804dfca3b7adb
Review URL: https://codereview.chromium.org/1114353004
Prior to this CL, if a client wanted to decode scanlines, they had to
create an SkCodec in order to get an SkScanlineDecoder. This introduces
complications if input data is not easily shared between the two
objects.
Instead, add methods to SkScanlineDecoder for creating a new one from
input data, and remove the creation functions from SkCodec.
Update DM and tests.
Review URL: https://codereview.chromium.org/1267583002
Does not try to cache calls to readPixels at the moment:
- not triggered by drawing
- not clear if we want to perform any pixel transformations (that readPixels allows) on the GPU or CPU
Can consider that another time.
BUG=513695
Review URL: https://codereview.chromium.org/1262923003
Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached.
Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold.
In order to invalidate the cache when an SkPath is changed or deleted,
this required implementing genID change notification in SkPath. This is
modelled almost exactly on SkPixelRef::GenIDChangeListener.
However, It does not currently implement the check for unique genIDs,
so notifiers will fire when the first instance of an SkPathRef
using a given genID is destroyed.
Another caveat is that you cannot successfully add a change notifier
to an empty path, since it uses the "canonical" empty path which is
never modified or destroyed. For this reason, we prevent adding
listeners to it.
BUG=skia:4121,skia:4122, 497403
DOCS_PREVIEW= https://skia.org/?cl=1114353004
Review URL: https://codereview.chromium.org/1114353004
To make this work, we tag their pixelrefs as temporarily immutable, allowing
ourselves to restore the pixels to mutability only when the image drops away.
This should allow us to wobble back and forth between writing to the Surface
and reading from the Image without a COW, with the Surface seeing mutable
pixels and the Image seeing immutable pixels.
The big idea is, Image doesn't need forever-immutable pixels, it just needs
pixels that are immutable as long as it's alive.
BUG=skia:
patch from issue 804523002 at patchset 40001 (http://crrev.com/804523002#ps40001)
Review URL: https://codereview.chromium.org/1254383006
This is a contract change for SkPath::getBounds(), which formally was defined to return 0,0,0,0 for a 1-point path, regardless of the coordinates of that point. This seems wacky/inconsistent, and was causing other bugs (incorrect bounds) when this was unioned with other rects.
Does anyone remember why we defined it this way?
BUG=513799
Review URL: https://codereview.chromium.org/1261773002
