This CL sets the stage for retracting the SkPicture::kOptimizeForClippedPlayback_RecordingFlag flag
from the public API (more work needs to be done in Blink & Chrome). In the new world the only way
to set this flag (and thus instantiate an SkPicture-derived
class) is by passing a factory to the SkPictureRecorder class. This is to get all clients always using
factories so that we can then change the factory call used (i.e., so the factory just creates a BBH) and
do away with the SkPicture-derived classes.
BUG=skia:2315
R=reed@google.com
Author: robertphillips@google.com
Review URL: https://codereview.chromium.org/239703006
git-svn-id: http://skia.googlecode.com/svn/trunk@14221 2bbb7eff-a529-9590-31e7-b0007b416f81
Mike K: please sanity check Test.cpp and skia_test.cpp
Feel free to look at the rest, but I don't expect any in depth review of path ops innards.
Path Ops first iteration used QuickSort to order segments radiating from an intersection to compute the winding rule.
This revision uses a circular sort instead. Breaking out the circular sort into its own long-lived structure (SkOpAngle) allows doing less work and provides a home for caching additional sorting data.
The circle sort is more stable than the former sort, has a robust ordering and fewer exceptions. It finds unsortable ordering less often. It is less reliant on the initial curve tangent, using convex hulls instead whenever it can.
Additional debug validation makes sure that the computed structures are self-consistent. A new visualization tool helps verify that the angle ordering is correct.
The 70+M tests pass with this change on Windows, Mac, Linux 32 and Linux 64 in debug and release.
R=mtklein@google.com, reed@google.com
Author: caryclark@google.com
Review URL: https://codereview.chromium.org/131103009
git-svn-id: http://skia.googlecode.com/svn/trunk@14183 2bbb7eff-a529-9590-31e7-b0007b416f81
It is not completely fair to compare --skr and --noskr numbers. SKR looks
unfairly good because some optimizations are baked into our SKPs at record
time. But, at least, by using a kWriteOnly_Mode SkRecorder, we prevent
SkPicturePlayback from compounding that unfairness. SkRecordDraw must handle
its own playback-time optimizations (quickrejects) on its own.
This code should look suspiciously similar to bench_record.
BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/233833002
git-svn-id: http://skia.googlecode.com/svn/trunk@14151 2bbb7eff-a529-9590-31e7-b0007b416f81
Record performance as measured by bench_record (out/Release/bench_record --skr) improves by at least 1.9x, at most 6.7x, arithmetic mean 2.6x, geometric mean 3.0x. So, good.
Correctness as measured by DM (out/Debug/dm --skr) is ~ok. One GM (shadertext2) fails because we're assuming all paint effects are immutable, but SkShaders are still mutable.
To do after this CL:
- measure playback speed
- catch up feature-wise to SkPicture
- match today's playback speed
BUG=skia:
R=robertphillips@google.com, bsalomon@google.com, reed@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/206313003
git-svn-id: http://skia.googlecode.com/svn/trunk@14010 2bbb7eff-a529-9590-31e7-b0007b416f81
Similar in spirit to gflags' undefok, I'd like to be able to ignore
specific unknown flags. This lets me run the same command line on, say,
a branch that's got a new flag and on a clean branch tracking
origin/master. This is handy for performance comparison, etc.
It's not essential, and if you hate this I can find another way.
As an example, I want to compare the runtime of SKP recording with my new code. I've added a flag --skr to bench_record to help this. So I want to compare
origin/master: out/Release/bench_record
my patch: out/Release/bench_record --skr
This lets me run both as out/Release/bench_record --undefok skr --skr, which is handy for scripts and things.
BUG=skia:
R=scroggo@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/209393015
git-svn-id: http://skia.googlecode.com/svn/trunk@13945 2bbb7eff-a529-9590-31e7-b0007b416f81
(clang 3.3):
../tools/PictureRenderer.cpp:350:13: error: variable 'hash' is used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
if (!generatedHash) {
^~~~~~~~~~~~~~
../tools/PictureRenderer.cpp:354:53: note: uninitialized use occurs here
jsonSummaryPtr->add(outputFilename.c_str(), hash);
^~~~
../tools/PictureRenderer.cpp:350:9: note: remove the 'if' if its condition is always true
if (!generatedHash) {
^~~~~~~~~~~~~~~~~~~~
../tools/PictureRenderer.cpp:334:18: note: initialize the variable 'hash' to silence this warning
uint64_t hash;
^
= 0
The warning is wrong, but the compiler does have a point: generatedHash is always false at that point.
R=epoger@google.com
Author: fmalita@chromium.org
Review URL: https://codereview.chromium.org/196823004
git-svn-id: http://skia.googlecode.com/svn/trunk@13869 2bbb7eff-a529-9590-31e7-b0007b416f81
There were a bunch of problems with the existing code, all of which I would of ignored, except it could only benchmark none and rtree.
The new version measures the same numbers, in almost exactly the same way, it's the structure over the top of the actual test runner that I changed.
Now it only loads the pictures once, it is configurable in which bbh's it runs and how may loops on them it does, it uses standard command line processing, and it does not do so much redundant work so it runs quicker. It is also much easier to understand and change IMHO.
Doing this so I can reasonably compare the new QuadTree implementation.
BUG=skia:2242
Committed: http://code.google.com/p/skia/source/detail?r=13736R=tomhudson@google.com, mtklein@google.com, reed@google.com
Author: iancottrell@google.com
Review URL: https://codereview.chromium.org/186973005
git-svn-id: http://skia.googlecode.com/svn/trunk@13743 2bbb7eff-a529-9590-31e7-b0007b416f81
There were a bunch of problems with the existing code, all of which I would of ignored, except it could only benchmark none and rtree.
The new version measures the same numbers, in almost exactly the same way, it's the structure over the top of the actual test runner that I changed.
Now it only loads the pictures once, it is configurable in which bbh's it runs and how may loops on them it does, it uses standard command line processing, and it does not do so much redundant work so it runs quicker. It is also much easier to understand and change IMHO.
Doing this so I can reasonably compare the new QuadTree implementation.
BUG=skia:2242
R=tomhudson@google.com, mtklein@google.com, reed@google.com
Author: iancottrell@google.com
Review URL: https://codereview.chromium.org/186973005
git-svn-id: http://skia.googlecode.com/svn/trunk@13736 2bbb7eff-a529-9590-31e7-b0007b416f81
Add add_codereview_message.py script. This script takes a message and
a codereview URL as arguments calls `git cl upload` to append the
message to the given codereview issue.
Motivation: We are automating a process of testing Chromium/Skia DEPS
rolls where roll_deps.py is automatically run every night, and then at
a later time, compare_codereview.py is run to generate a report of the
trybot results. This script can be used to append that report to the
DEPS roll Cl.
This CL also refactors functioanlity common to roll_deps and
add_codereview_message:
* Add tools/git_utils.py module.
- ChangeGitBranch class was factored out of
roll_deps.GitBranchCLUpload. The other half of that class
became roll_deps.git_cl_uploader function. I make use of
this new class in both roll_deps and upload_deps_roll.
- test_git_executable moved to this new module.
* Add tools/misc_utils.py - move VerboseSubprocess, ChangeDir,
and ReSearch classes from roll_deps module.
* Remove generic functions from roll_deps.
* Add git_cl_uploader function to roll_deps. Refactor
roll_deps.roll_deps function to make use of it
BUG=skia:
R=borenet@google.com, robertphillips@google.com
Author: halcanary@google.com
Review URL: https://codereview.chromium.org/141483011
git-svn-id: http://skia.googlecode.com/svn/trunk@13251 2bbb7eff-a529-9590-31e7-b0007b416f81
LeakSanitizer is built into AddressSanitizer, and runs with no extra speed
penalty. Most interestingly, it's got a lock-free allocator, so it doesn't
bottleneck threaded code.
This file is enough to supress all leak warnings except from the Nvidia driver
on my desktop for tests and dm. Looks like lsan doesn't or can't pay attention
to leaked globals (-fno-common doesn't affect this).
BUG=skia:
R=robertphillips@google.com
Author: mtklein@google.com
Review URL: https://codereview.chromium.org/145033010
git-svn-id: http://skia.googlecode.com/svn/trunk@13153 2bbb7eff-a529-9590-31e7-b0007b416f81
This script:
- searches through the last N commits to find out the hash that is
associated with the revision number.
- creates a new branch in the chromium tree, modifies the DEPS
file, commits, and uploads to Rietveld.
- create a whitespace-only commit and uploads that to to Rietveld.
BUG=
R=borenet@google.com
Review URL: https://codereview.chromium.org/123523003
git-svn-id: http://skia.googlecode.com/svn/trunk@12921 2bbb7eff-a529-9590-31e7-b0007b416f81
Motivation: We want to remove redundant classes from Skia. To
that end we want to remove SkImageRef and its subclasses and
replace their uses with SkDiscardablePixelRef +
SkDecodingImageGenerator. Since Android uses SkImageRef, we need
to make sure that SkDecodingImageGenerator allows all of the
settings that Android exposes in BitmapFactory.Options.
To that end, we have created an Options struct for the
SkDecodingImageGenerator which lets the client of the generator set
sample size, dithering, and bitmap config.
We have made the SkDecodingImageGenerator constructor private
and replaced the SkDecodingImageGenerator::Install functions
with a SkDecodingImageGenerator::Create functions (one for
SkData and one for SkStream) which now take a
SkDecodingImageGenerator::Options struct.
Also added a ImageDecoderOptions test which loops through a list
of sets of options and tries them on a set of 5 small encoded
images.
Also updated several users of SkDecodingImageGenerator::Install to
follow new call signature - gm/factory.cpp, LazyDecodeBitmap.cpp,
and PictureTest.cpp, CachedDecodingPixelRefTest.cpp.
We also added a new ImprovedBitmapFactory Test which simulates the
exact function that Android will need to modify to use this,
installPixelRef() in BitmapFactory.
R=reed@google.com, scroggo@google.com
Committed: https://code.google.com/p/skia/source/detail?r=12744
Review URL: https://codereview.chromium.org/93703004
git-svn-id: http://skia.googlecode.com/svn/trunk@12855 2bbb7eff-a529-9590-31e7-b0007b416f81
Motivation: We want to remove redundant classes from Skia. To
that end we want to remove SkImageRef and its subclasses and
replace their uses with SkDiscardablePixelRef +
SkDecodingImageGenerator. Since Android uses SkImageRef, we need
to make sure that SkDecodingImageGenerator allows all of the
settings that Android exposes in BitmapFactory.Options.
To that end, we have created an Options struct for the
SkDecodingImageGenerator which lets the client of the generator set
sample size, dithering, and bitmap config.
We have made the SkDecodingImageGenerator constructor private
and replaced the SkDecodingImageGenerator::Install functions
with a SkDecodingImageGenerator::Create functions (one for
SkData and one for SkStream) which now take a
SkDecodingImageGenerator::Options struct.
Also added a ImageDecoderOptions test which loops through a list
of sets of options and tries them on a set of 5 small encoded
images.
Also updated several users of SkDecodingImageGenerator::Install to
follow new call signature - gm/factory.cpp, LazyDecodeBitmap.cpp,
and PictureTest.cpp, CachedDecodingPixelRefTest.cpp.
We also added a new ImprovedBitmapFactory Test which simulates the
exact function that Android will need to modify to use this,
installPixelRef() in BitmapFactory.
R=reed@google.com, scroggo@google.com
Review URL: https://codereview.chromium.org/93703004
git-svn-id: http://skia.googlecode.com/svn/trunk@12744 2bbb7eff-a529-9590-31e7-b0007b416f81
Added SK_API to SkImageGenerator (already in include/).
Moved SkDiscardablePixelRef::Install to SkInstallDiscardablePixelRef,
added SK_API to that function, and moved declaration to
SkImageGenerator.h
This keeps the SkDiscardablePixelRef internal to Skia, but exposes a
method to install it into a bitmap.
Modifed tests that rely on this functio to use new version.
BUG=
R=mtklein@google.com, reed@google.com
Review URL: https://codereview.chromium.org/111713002
git-svn-id: http://skia.googlecode.com/svn/trunk@12612 2bbb7eff-a529-9590-31e7-b0007b416f81
- Implement ashmem-backed SkDiscardableMemory subclass:
This class in only accesible via the SkDiscardableMemory::Create()
function, which replaces the mock implementation in
SkDiscardableMemory_none.cpp
- Added SkDiscardableMemory_ashmem.cpp to the Android port of Skia
Removed SkDiscardableMemory_none.cpp from the Android port.
- Added DiscardableMemoryTest.
Still needs work.
- SkDiscardablePixelRef Bugfix:
onLockPixels() now calls SkDELETE on the SkDiscardableMemory pointer
when it fails to unlock.
- Improved documentation inside ashmem.h
BUG=
R=scroggo@google.com
Review URL: https://codereview.chromium.org/83563002
git-svn-id: http://skia.googlecode.com/svn/trunk@12608 2bbb7eff-a529-9590-31e7-b0007b416f81
Removed SkBitmapFactory since no clients were using it. New cache
selection mechanism can simply pass a SkDiscardableMemory::Factory
into the SkDiscardablePixelRef if non-default SkDiscardableMemory
should be used. Removed BitmapFactoryTest.
SkDiscardableMemory::Factory interface. Android will need this
functionality in the future inside their BitmapFactory.
Removed SkLazyPixelRef, since it's functionality is now subsumed into
SkDiscardablePixelRef. Removed LazyPixelRef test.
Modified SkDiscardablePixelRef to optionally allow it to use a
SkDiscardableMemory::Factory. This tiny change makes it a replacement
for SkLazyPixelRef. This functioanlity is also necessary for moving
Android over to SkDiscardablePixelRef from SkImageRef in a later CL.
Added a test for this.
SkDecodingImageGenerator::Install can optionally pass a factory in to
SkDiscardablePixelRef.
Removed SkImageCache, SkLruImageCache, and SkPurgeableImageCache.
This functionality can be handled much more cleanly by
SkDiscardableMemory.
New SkDiscardableMemoryPool class to replace SkLruImageCache. In a
later CL, we will replace SkImageRef_GlobalPool (used by android) as
well. This is a concrete implementation of
SkDiscardableMemory::Factory. Added a test for this.
modified gm/factory.cpp to remove dependnce on SkBitmapFactory +
SkLruImageCache. Now uses SkDecodingImageGenerator +
SkDiscardablePixelRef + SkDiscardableMemoryPool.
SkImageDecoder::Target replaces SkBitmapFactory::Target. The
DecodeMemoryToTarget function may disappear in the future.
Moved SkLazyCachingPixelRef::DecodeProc replaces
SkBitmapFactory::DecodeProc. This is a short term change, since
another CL changes SkLazyCachingPixelRef to use SkImageGenerator
instead of DecodeProc.
Modified DrawBitmapRectTest to use SkDiscardablePixelRef instead of
SkLazyPixelRef.
tools/LazyDecodeBitmap.cpp now uses SkDecodingImageGenerator +
SkDiscardablePixelRef instead of a SkBitmapFactory.
bench_pictures uses the Global SkDiscardableMemoryPool instead of a
global gLruImageCache.
R=reed@google.com, scroggo@google.com
Review URL: https://codereview.chromium.org/103033002
git-svn-id: http://skia.googlecode.com/svn/trunk@12515 2bbb7eff-a529-9590-31e7-b0007b416f81
Using Mike Klein's excellent coverage tool, increase the
unit testing of SkPath.cpp from 70% to 95%.
Along the way, determined that these functions were not
maintained or used:
SkPath::pathTo
SkPath::contains
as well as a large block of SkPath::cheapGetDirection().
Changed SkPath::validate() to permit infinities in
the path data points.
Fixed errors in preserving direction.
Fixed error setting direction when convexity is unknown.
Added missing conic to moveTo only detector.
BUG=
R=bsalomon@google.com, reed@google.com
Author: caryclark@google.com
Review URL: https://codereview.chromium.org/65493004
git-svn-id: http://skia.googlecode.com/svn/trunk@12291 2bbb7eff-a529-9590-31e7-b0007b416f81
bitmap_to_cielab now returns a boolean. Instead of asserting that the
config is 8888, copy to 8888 (and return false on failure). This allows
skpdiff to work on Index8 bitmaps (without changing the code that does
the real work). If it returns false, do not attempt to create a pmetric.
In pmetric, exit early if maxLevels is <= 2, since this would crash later
in the function (creation of an ImageL3D if maxLevels is 0, and then the
creation of an array of a negative number of entries). maxLevels is small
if the width or height of the image is small (i.e. a bitmap with width 1
has maxLevels of 0).
R=djsollen@google.com
Review URL: https://codereview.chromium.org/65253002
git-svn-id: http://skia.googlecode.com/svn/trunk@12239 2bbb7eff-a529-9590-31e7-b0007b416f81
tools/skimage_main.cpp:
Add the ability to write the results to checksum based filenames,
much like GM uses. This will allow using the skpdiff server to
rebaseline images.
Write the keys in the JSON file as <original image>_<pref config>.png,
so it matches gm_json.IMAGE_FILENAME_PATTERN. Also replace '_' with
'-' in the original file name, to avoid confusing the pattern matcher.
The '_' to '-' replacement also happens on the output filename.
Read the keys in a similar manner.
In make_outname, no longer remove a suffix. This fixes a bug where
subset decoding writes multiple subsets to the same file.
tools/rebaseline.py:
Since the filenames written to json files now match
gm_json.IMAGE_FILENAME_PATTERN, enable the option to match based
on configs/tests when rebaselining skimage.
test json files:
Update to match the new format of output.
gm/gm_expectations:
Add a constructor that takes a BitmapAndDigest as input.
tools/tests/skimage_self_test.py:
Test that reading the expectations file just created by skimage with
the same file actually compares to the original file (rather than just
succeeding because expectations were missing).
Change the expectations files to match the new format.
Will require a buildbot change to use the new flag: https://codereview.chromium.org/27389002/
BUG=1466
R=epoger@google.com
Review URL: https://codereview.chromium.org/26297004
git-svn-id: http://skia.googlecode.com/svn/trunk@11902 2bbb7eff-a529-9590-31e7-b0007b416f81
If the expectation is set to ignore failures, do not return a
negative 1 (indicating a failure) when a successful decode
does not match the expectation.
If the file could not be decoded at all, report this to the
user depending on the input expectations file:
No expectations:
Report failure. The user wanted to know if the file could be
decoded.
Expectations expected a valid result:
Report failure. The decode should have matched the result.
Empty expectations:
Print a message that the expectation was missing, but still
return success from skimage, since this is a newly added file
(i.e. it has no expectation yet).
Ignore failure:
Return success from skimage, since it is a known failure.
Update the self tests to ensure these behaviors.
R=epoger@google.com
Review URL: https://codereview.chromium.org/22293006
git-svn-id: http://skia.googlecode.com/svn/trunk@11790 2bbb7eff-a529-9590-31e7-b0007b416f81
Add new runtime config variable, images.jpeg.suppressDecoderErrors
which defaults to false in Debug and true otherwise. When Jpeg errors
are suppressed and an error happens, SkJPEGImageDecoder::onDecode()
will return silently false (Consequently, so will SkImageDecoder's
DecodeFile() and DecodeMemory() functions).
Also, the test_image_decoder program now respects runtime
configuration settings.
BUG=skia:1680
R=scroggo@google.com
Review URL: https://codereview.chromium.org/27230002
git-svn-id: http://skia.googlecode.com/svn/trunk@11763 2bbb7eff-a529-9590-31e7-b0007b416f81
This change address what happens when a jpeg is partially downloaded
before failing. Many browsers will render it anyway: we want Skia to
do the same. The JpegTest takes a perfectly cromulent jpeg file and
only passes into the ImageDecoder the first half of the image. We
then verify that the image decoder returns a valid bitmap of the
correct dimensions.
We also fixed some png library errors, including issue 1691.
Also, suppressed the majority of warnings from using libpng and
libjpeg. By default, most warnings are *not* suppressed in debug mode.
If you have a debug binary and wish to suppress warnings, set the
following environment variables to true
skia_images_png_suppressDecoderWarnings
skia_images_jpeg_suppressDecoderWarnings
or from within a program that links to Skia:
#if defined(SK_DEBUG)
#include "SkRTConf.h"
SK_CONF_SET("images.jpeg.suppressDecoderWarnings", true);
SK_CONF_SET("images.png.suppressDecoderWarnings", true);
#endif
I tested this, before (control) and after these changes (test), on
364,295 skps from the cluster telemetry.
- number of errors+warnings in control = 2804
- number of errors+warnings fixed = 2283
- number of PNG verbosity fixed = 2152
- number of PNG error fixed = 4
- number of PNG segfault fixed = 3
- number of PNG errors changed to warnings = 62
- number of JPG verbosity fixed = 26
- number of JPG error fixed = 91
Not all errors and warning have been fixed.
These numbers were generated using the find_bad_images_in_skps.py
program. This program may be useful going forward for testing
image-decoding libraries on skp files from the cluster telemetry.
find_bad_images_in_skps.py depends on the test_image_decoder program,
which simply executes the SkImageDecoder::DecodeFile function and uses
its exit status to report success or failure.
BUG=skia:1649
BUG=skia:1691
BUG=skia:1680
R=scroggo@google.com
Review URL: https://codereview.chromium.org/24449003
git-svn-id: http://skia.googlecode.com/svn/trunk@11597 2bbb7eff-a529-9590-31e7-b0007b416f81
src/images/SkImageDecoder_libpng.cpp:
A8 images are not opaque, so do not set the opaque flag.
This fixes a bug where copyTo does not work as expected (when
copying an A8 decoded image to ARGB_8888), leading to
a bitmap hash that does not represent the image correctly
(in skimage).
tools/skimage_main.cpp:
In write_bitmap, which is creating the image for visual comparison,
copy A8 to 8888, since A8 cannot be encoded.
In the section that tests reencoding, do not test reencoding A8,
which is known to not work.
R=mtklein@google.com, djsollen@google.com
Author: scroggo@google.com
Review URL: https://codereview.chromium.org/25726004
git-svn-id: http://skia.googlecode.com/svn/trunk@11589 2bbb7eff-a529-9590-31e7-b0007b416f81
Only implemented for PNG.
Add a getter and setter, and sets the default to false in the
constructor. Also copies the setting in copyFieldsToOther.
Fix an indpendent bug where fDitherImage was not being copied in
copyFieldsToOther.
In SkScaledBitmapSampler::begin, consolidate the settings passed in
by passing a const reference to the decoder. The decoder can be
referenced for its settings of dither, unpremultiplied, and now
skipping writing zeroes. Update callers to use the new API. In png
decoder, rather than passing around a pointer to an initial
read of getDitherImage, and potentially changing it, look at the
field on the decoder itself, and modify it directly. This is a
change in behavior - now if that same decoder is used to decode
a different image, the dither setting has changed. I think this is
okay because A) the typical use case is to use a new decoder for
each decode, B) we do not make any promises that a decode does not
change the decoder and C) it makes the code in SkScaledBitmapSampler
much cleaner.
In SkScaledBitmapScampler, add new row procs for skipping zeroes. Now
that choosing the row proc has five dimensions (src config, dst config,
dither, skip writing zeroes, unpremultiplied), use a new method: each
src/dst combination has a function for choosing the right proc depending
on the decoder.
SkScaledBitmapScampler::RowProc is now public for convenience.
Remove Sample_Gray_D8888_Unpremul, which is effectively no different
from Sample_Gray_D8888.
In cases where unpremultiplied was trivial, such as 565 and when
sampling from gray, decoding may now succeed.
Add a benchmark (currently disabled) for comparing the speed of skipping
writing zeroes versus not skipping. For this particular image, which is
mostly transparent pixels, normal decoding took about 3.6 milliseconds,
while skipping zeroes in the decode took only about 2.5 milliseconds
(this is on a Nexus 4). Presumably it would be slower on an image
with a small amount of transparency, but there will be no slowdown
for an image which reports that it has no transparency.
In SkImageRef_ashmem, always skip writing zeroes, since ashmem
memory is guaranteed to be initialized to 0.
Add a flag to skip writing zeroes in skimage.
Add a regression test for choosing the rowproc to ensure I did not
change any behavior accidentally.
BUG=skia:1661
R=reed@google.com
Review URL: https://codereview.chromium.org/24269006
git-svn-id: http://skia.googlecode.com/svn/trunk@11558 2bbb7eff-a529-9590-31e7-b0007b416f81
