Commit Graph

171 Commits

Author SHA1 Message Date
reed
b7ed856fad Revert of Sketch splitting SkPicture into an interface and SkBigPicture. (patchset #22 id:420001 of https://codereview.chromium.org/1112523006/)
Reason for revert:
speculative revert to fix failures in DEPS roll

Original issue's description:
> Sketch splitting SkPicture into an interface and SkBigPicture.
>
> Adds small pictures for drawRect(), drawTextBlob(), and drawPath().
> These cover about 89% of draw calls from Blink SKPs,
> and about 25% of draw calls from our GMs.
>
> SkPicture handles:
>   - serialization and deserialization
>   - unique IDs
>
> Everything else is left to the subclasses:
>   - playback(), cullRect()
>   - hasBitmap(), hasText(), suitableForGPU(), etc.
>   - LayerInfo / AccelData if applicable.
>
> The time to record a 1-op picture improves a good chunk
> (2 mallocs to 1), and the time to record a 0-op picture
> greatly improves (2 mallocs to none):
>
>     picture_overhead_draw:   450ns -> 350ns
>     picture_overhead_nodraw: 300ns -> 90ns
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/c92c129ff85b05a714bd1bf921c02d5e14651f8b

TBR=reed@google.com,robertphillips@google.com,mtklein@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1130333002
2015-05-07 17:30:13 -07:00
mtklein
c92c129ff8 Sketch splitting SkPicture into an interface and SkBigPicture.
Adds small pictures for drawRect(), drawTextBlob(), and drawPath().
These cover about 89% of draw calls from Blink SKPs,
and about 25% of draw calls from our GMs.

SkPicture handles:
  - serialization and deserialization
  - unique IDs

Everything else is left to the subclasses:
  - playback(), cullRect()
  - hasBitmap(), hasText(), suitableForGPU(), etc.
  - LayerInfo / AccelData if applicable.

The time to record a 1-op picture improves a good chunk
(2 mallocs to 1), and the time to record a 0-op picture
greatly improves (2 mallocs to none):

    picture_overhead_draw:   450ns -> 350ns
    picture_overhead_nodraw: 300ns -> 90ns

BUG=skia:

Review URL: https://codereview.chromium.org/1112523006
2015-05-07 13:41:07 -07:00
reed
1c2c441fed add heuristic to pour small pictures into recordings, rather than ref'ing
BUG=skia:

Review URL: https://codereview.chromium.org/1118693003
2015-04-30 13:09:25 -07:00
mtklein
98b8485a4c O(1) SkPictureUtils::ApproxBytesUsed()
Chrome wants to call this more often, and it's quite slow today.

Seems like this could be clearer if SkPictureUtils::ApproxBytesUsed() were SkPicture::approxBytesUsed().

BUG=chromium:471873

Review URL: https://codereview.chromium.org/1090943004
2015-04-21 15:24:00 -07:00
mtklein
29b1afc169 Rearrange SkRecord with small N in mind
This rearranges the record pointers and types so they can go in a single array, then preallocates some space for them and for the SkVarAlloc.

picture_overhead_draw bench drops from ~1000ns to 500-600ns, with no effect on picture_overhead_nodraw.

I don't see any significant effect on large picture recording times from our .skps.

BUG=chromium:470553

Committed: https://skia.googlesource.com/skia/+/e2dd9408cd711777afaa9410427fb0d761ab004a

Review URL: https://codereview.chromium.org/1061783002
2015-04-09 07:46:41 -07:00
mtklein
35f55764b8 Revert of Rearrange SkRecord with small N in mind (patchset #8 id:120001 of https://codereview.chromium.org/1061783002/)
Reason for revert:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86-Debug/builds/149/steps/dm/logs/stdio

Original issue's description:
> Rearrange SkRecord with small N in mind
>
> This rearranges the record pointers and types so they can go in a single array, then preallocates some space for them and for the SkVarAlloc.
>
> picture_overhead_draw bench drops from ~1000ns to 500-600ns, with no effect on picture_overhead_nodraw.
>
> I don't see any significant effect on large picture recording times from our .skps.
>
> BUG=chromium:470553
>
> Committed: https://skia.googlesource.com/skia/+/e2dd9408cd711777afaa9410427fb0d761ab004a

TBR=reed@google.com,mtklein@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:470553

Review URL: https://codereview.chromium.org/1068383003
2015-04-08 14:09:41 -07:00
mtklein
e2dd9408cd Rearrange SkRecord with small N in mind
This rearranges the record pointers and types so they can go in a single array, then preallocates some space for them and for the SkVarAlloc.

picture_overhead_draw bench drops from ~1000ns to 500-600ns, with no effect on picture_overhead_nodraw.

I don't see any significant effect on large picture recording times from our .skps.

BUG=chromium:470553

Review URL: https://codereview.chromium.org/1061783002
2015-04-08 14:02:32 -07:00
mtklein
feaadee1c3 SkCanvas::resetForNextPicture()
No diffs against head for DM --config 8888 gpu 2ndpic-8888 2ndpic-gpu.

  picture_overhead_draw	1.62us ->  1.6us	0.99x
picture_overhead_nodraw	 792ns ->  342ns	0.43x

tiles and serialization modes will also test this a bit.

BUG=chromium:470553

Committed: https://skia.googlesource.com/skia/+/f920e468ac66a36c9653d1b11181480295044c7d

Review URL: https://codereview.chromium.org/1067893002
2015-04-08 11:25:48 -07:00
mtklein
36352bf5e3 C++11 override should now be supported by all of {bots,Chrome,Android,Mozilla}
NOPRESUBMIT=true

BUG=skia:
DOCS_PREVIEW= https://skia.org/?cl=1037793002

Review URL: https://codereview.chromium.org/1037793002
2015-03-25 18:17:32 -07:00
robertphillips
3e5c2b1041 Preserve texture compression when saving a revised skp from debugger
Without this CL, saving a revised skp out the debugger can greatly expand the memory it requires.

Review URL: https://codereview.chromium.org/1020103005
2015-03-23 05:46:51 -07:00
schenney
23d8593f81 Update SkPicture cull rects with RTree information
When computed, the RTree for an SkPicture will have a root
bounds that reflects the best bounding information available,
rather than the best estimate at the time the picture recorder
is created. Given that creators frequently don't know ahead of
time what will be drawn, the RTree bound is often tighter.

Perf testing on Chrome indicates a small raster performance
advantage. For upcoming painting changes in Chrome the
performance advantage is much larger.

BUG=

Committed: https://skia.googlesource.com/skia/+/2dd3b6647dc726f36fd8774b3d0d2e83b493aeac

Review URL: https://codereview.chromium.org/971803002
2015-03-06 16:20:28 -08:00
joshualitt
a0865b4620 Revert of Update SkPicture cull rects with RTree information (patchset #6 id:140001 of https://codereview.chromium.org/971803002/)
Reason for revert:
Might be breaking deps roll

Original issue's description:
> Update SkPicture cull rects with RTree information
>
> When computed, the RTree for an SkPicture will have a root
> bounds that reflects the best bounding information available,
> rather than the best estimate at the time the picture recorder
> is created. Given that creators frequently don't know ahead of
> time what will be drawn, the RTree bound is often tighter.
>
> Perf testing on Chrome indicates a small raster performance
> advantage. For upcoming painting changes in Chrome the
> performance advantage is much larger.
>
> BUG=
>
> Committed: https://skia.googlesource.com/skia/+/2dd3b6647dc726f36fd8774b3d0d2e83b493aeac

TBR=mtklein@google.com,schenney@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/977413003
2015-03-05 12:18:38 -08:00
schenney
2dd3b6647d Update SkPicture cull rects with RTree information
When computed, the RTree for an SkPicture will have a root
bounds that reflects the best bounding information available,
rather than the best estimate at the time the picture recorder
is created. Given that creators frequently don't know ahead of
time what will be drawn, the RTree bound is often tighter.

Perf testing on Chrome indicates a small raster performance
advantage. For upcoming painting changes in Chrome the
performance advantage is much larger.

BUG=

Review URL: https://codereview.chromium.org/971803002
2015-03-05 07:43:10 -08:00
mtklein
bfd5bff75c Simplify SkBBH::insert API
No one's exploiting the ability to take ownership of the array anymore.

BUG=skia:

Review URL: https://codereview.chromium.org/913833002
2015-02-10 13:44:27 -08:00
rmistry
465206af18 Add missing SK_OVERRIDE
Tested by running on clang head + ubuntu 14.04:
GYP_DEFINES=”skia_gpu=0 skia_warnings_as_errors=1" tools/xsan_build thread dm BUILDTYPE=Release
out/Release/dm -v

BUG=skia:3386

Review URL: https://codereview.chromium.org/894833002
2015-02-02 12:08:18 -08:00
robertphillips
98b0315ad6 Alter gpu veto
This CL unifies the treatment of the dashed and concave paths.

Before:
TP 28 FP 15 TN 8 FN 3 IND 3

After:
TP 28 FP 18 TN 7 FN 2 IND 2

One of the TrueNegatives that became a FalsePositive was the motivation use case (the Chromium busy spinner).

Committed: https://skia.googlesource.com/skia/+/87a6a8e18c7d5bbc94f478b44c53dc0e0549f927

Review URL: https://codereview.chromium.org/875913002
2015-01-26 11:29:36 -08:00
reed
e44acc9617 remove more dead code
BUG=skia:

Review URL: https://codereview.chromium.org/865873003
2015-01-22 09:37:46 -08:00
reed
b0dfb546f5 remove (unused) GatherPixelRefs
BUG=skia:

Review URL: https://codereview.chromium.org/869463002
2015-01-22 09:03:25 -08:00
mtklein
72c9faab45 Fix up all the easy virtual ... SK_OVERRIDE cases.
This fixes every case where virtual and SK_OVERRIDE were on the same line,
which should be the bulk of cases.  We'll have to manually clean up the rest
over time unless I level up in regexes.

for f in (find . -type f); perl -p -i -e 's/virtual (.*)SK_OVERRIDE/\1SK_OVERRIDE/g' $f; end

BUG=skia:

Review URL: https://codereview.chromium.org/806653007
2015-01-09 10:06:40 -08:00
reed
5965c8ae4e add ImageGenerator::NewFromData to porting layer
BUG=skia:3275

Review URL: https://codereview.chromium.org/834633006
2015-01-07 18:04:45 -08:00
reed
d990e2f14f add testing flag to ignore saveLayer bounds
BUG=skia:

Review URL: https://codereview.chromium.org/819123003
2014-12-22 11:58:30 -08:00
reed
53b9e2e0f2 add const to encodePixels pixel parameter
BUG=skia:

Review URL: https://codereview.chromium.org/788143007
2014-12-19 12:26:07 -08:00
scroggo
895c43b28b Replace EncodeBitmap with an interface.
Gives more flexibility to the caller to decide whether to use the
encoded data returned by refEncodedData().

Provides an implementation that supports the old version of
SkPicture::serialize().

TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely

BUG=skia:3190

Review URL: https://codereview.chromium.org/784643002
2014-12-11 10:53:58 -08:00
Florin Malita
c54d8db4d1 Remove SkCanvas::drawBitmapMatrix()
R=mtklein@google.com, reed@google.com, robertphillips@google.com

Review URL: https://codereview.chromium.org/789033002
2014-12-10 12:02:16 -05:00
scroggo
3030445799 Revert of Replace EncodeBitmap with an interface. (patchset #12 id:210001 of https://codereview.chromium.org/784643002/)
Reason for revert:
Failing serialization tasks in DM:

http://build.chromium.org/p/client.skia/builders/Test-Win8-ShuttleA-GTX660-x86-Debug/builds/352/steps/dm/logs/stdio

Original issue's description:
> Replace EncodeBitmap with an interface.
>
> Gives more flexibility to the caller to decide whether to use the
> encoded data returned by refEncodedData().
>
> Provides an implementation that supports the old version of
> SkPicture::serialize().
>
> TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely
>
> BUG=skia:3190
>
> Committed: https://skia.googlesource.com/skia/+/0c4aba6edb9900c597359dfa49d3ce4a41bc5dd1
>
> Committed: https://skia.googlesource.com/skia/+/02b217f80b01a7dda8493422e5257c36a9ce8464

TBR=reed@google.com,rmistry@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:3190

Review URL: https://codereview.chromium.org/783393004
2014-12-09 08:27:37 -08:00
scroggo
02b217f80b Replace EncodeBitmap with an interface.
Gives more flexibility to the caller to decide whether to use the
encoded data returned by refEncodedData().

Provides an implementation that supports the old version of
SkPicture::serialize().

TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely

BUG=skia:3190

Committed: https://skia.googlesource.com/skia/+/0c4aba6edb9900c597359dfa49d3ce4a41bc5dd1

Review URL: https://codereview.chromium.org/784643002
2014-12-09 07:44:36 -08:00
robertphillips
c0708cb7bc Revert of Replace EncodeBitmap with an interface. (patchset #11 id:190001 of https://codereview.chromium.org/784643002/)
Reason for revert:
Compilation is failing on some bots

Original issue's description:
> Replace EncodeBitmap with an interface.
>
> Gives more flexibility to the caller to decide whether to use the
> encoded data returned by refEncodedData().
>
> Provides an implementation that supports the old version of
> SkPicture::serialize().
>
> TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely
>
> BUG=skia:3190
>
> Committed: https://skia.googlesource.com/skia/+/0c4aba6edb9900c597359dfa49d3ce4a41bc5dd1

TBR=reed@google.com,scroggo@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:3190

Review URL: https://codereview.chromium.org/787833002
2014-12-09 05:35:07 -08:00
scroggo
0c4aba6edb Replace EncodeBitmap with an interface.
Gives more flexibility to the caller to decide whether to use the
encoded data returned by refEncodedData().

Provides an implementation that supports the old version of
SkPicture::serialize().

TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely

BUG=skia:3190

Review URL: https://codereview.chromium.org/784643002
2014-12-09 05:23:12 -08:00
robertphillips
01d6e5f462 Use variable length key (rather than accumulated matrix) as save layer hoisting key
Adding the rendering canvas' CTM to the layer hoisting key (i.e., Add support for hoisting layers in pictures drawn with a matrix - https://codereview.chromium.org/748853002/) has increased the cache miss rate due to accumulated floating point error. This CL fixes part of the issue by using the chain of operation indices leading to each saveLayer as the key. The canvas' CTM must still form part of the key but should be less subject to accumulated error.

BUG=skia:2315

Review URL: https://codereview.chromium.org/753253002
2014-12-01 09:09:27 -08:00
reed
1bdfd3f4f0 option to return drawable from recording
patch from issue 747033005 at patchset 80001 (http://crrev.com/747033005#ps80001)

BUG=skia:

Review URL: https://codereview.chromium.org/732653004
2014-11-24 14:41:51 -08:00
reed
90d0ff013b add some debugging to SkNVRefCnt
BUG=skia:

Review URL: https://codereview.chromium.org/745383003
2014-11-24 12:02:31 -08:00
mtklein
04c9695055 Remove Picture deletion listeners.
Looks like we can just have ~SkPicture put the message on the bus directly.

BUG=skia:3144

Review URL: https://codereview.chromium.org/751663002
2014-11-24 08:20:58 -08:00
mtklein
08d1fccf6e Add SkNVRefCnt, prune down SkPicture's size
SkNVRefCnt is a variant of SkRefCnt that's Not Virtual, so weighs 4 bytes
instead of 8 or 16.  There's only benefit to doing this if the deriving class
does not otherwise need a vtable, e.g. SkPicture.

I've stripped out some cruft from SkPicture, rearranged fields to pack tightly,
and added compile asserts for the sizes of SkPicture, SkRecord, and
SkVarAlloc.

BUG=skia:3144

Review URL: https://codereview.chromium.org/741793002
2014-11-20 09:18:32 -08:00
tomhudson
158fcaa031 Implement SkPicture::bytesUsed() for SkRecord backend
BUG=chromium:230419
R=mtklein@google.com,reed@google.com

Review URL: https://codereview.chromium.org/490253003
2014-11-19 10:41:14 -08:00
reed
78e2768897 Allow pictures to have a full bounds
This reverts commit 7c4cdd2c5b.

BUG=skia:

Review URL: https://codereview.chromium.org/738083002
2014-11-19 08:04:34 -08:00
reed
7c4cdd2c5b Revert of allow pictures to have a full bounds (patchset #3 id:40001 of https://codereview.chromium.org/736583004/)
Reason for revert:
needed to update legacy width() helpers

Original issue's description:
> allow pictures to have a full bounds
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/7e76bff26e7c74902841ca4f607eb0b24a833a4a

TBR=fmalita@google.com,mtklein@google.com,robertphillips@google.com,fmalita@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/722043005
2014-11-19 07:17:17 -08:00
reed
7e76bff26e allow pictures to have a full bounds
BUG=skia:

Review URL: https://codereview.chromium.org/736583004
2014-11-19 06:59:41 -08:00
robertphillips
8236591547 Rename GrAccelData to SkLayerInfo and move it to src/core
Review URL: https://codereview.chromium.org/719133002
2014-11-12 09:32:34 -08:00
robertphillips
4e8e3421aa Move SkRecordComputeLayers and CollectLayers into SkRecordDraw.cpp
Rather then exposing parts of FillBounds (as in Expose FillBounds to allow GrPictureUtils::CollectLayers to be layered on top of it - https://codereview.chromium.org/698643002/), this CL moves CollectLayers into SkRecordDraw.cpp to accomplish the layering.

Review URL: https://codereview.chromium.org/716913003
2014-11-12 06:46:08 -08:00
robertphillips
81f71b6630 Change where layer hoisting data is gathered
This CL:
1) removes the EXPERIMENTAL_optimize on SkCanvas & SkDevice
2) moves the saveLayer gathering step to endRecording
3) Replaces GPUOptimize with SkRecordComputeLayers
4) Update bench_pictures & render_pictures to provide the new flag

#2 also necessitated moving the BBH computation (and record optimization) out of SkPicture's ctor (and into endRecording)

Review URL: https://codereview.chromium.org/718443002
2014-11-11 04:54:49 -08:00
jvanverth
d86b07a8ff Fix PictureTest for new GPU veto
NOTREECHECKS=true

Review URL: https://codereview.chromium.org/702733002
2014-11-04 08:50:16 -08:00
robertphillips
d8aa7b74c8 Upgrade GPUOptimize
This will be a bit hairy to review.

The FillBounds and CollectLayers code has diverged significantly resulting in the rendering path seeing different bounds than the hoisting path. This CL merges the FillBounds changes into CollectLayers. A follow on CL will, hopefully, find a way to layer CollectLayers on top of FillBounds.

The only code in CollectLayers that is different from FillBounds is bracketed by "LAYER HOISTING" comments.

NOTREECHECKS=true

Review URL: https://codereview.chromium.org/685263004
2014-10-30 16:45:02 -07:00
junov
a41d3c33bc Make RTree handle the case where the playback canvas has empty clip bounds
BUG=skia:3074

Review URL: https://codereview.chromium.org/678303004
2014-10-30 11:44:20 -07:00
mtklein
4477c3c0e6 Cut down SkBBH API more.
- The expected case is now a single bulk-load insert() call instead of N;
  - reserve() and flushDeferredInserts() can fold into insert() now;
  - SkBBH subclasses may take ownership of the bounds

This appears to be a performance no-op on both my Mac and N5.  I guess
even the simplest indirect branch predictor ("same as last time") can predict
the repeated virtual calls to SkBBH::insert() perfectly.

BUG=skia:

Review URL: https://codereview.chromium.org/670213002
2014-10-27 10:27:10 -07:00
robertphillips
9e6835da41 Track nested picture xform state for layer hoisting
The accumulated matrix state of any enclosing SkPictures must be stored separate from the picture-local CTM. Any setMatrix calls inside a layer need to replace the picture-local CTM but concatenate with the enclosing SkPicture transform state (and the transform state needed to translate the layer to the correct location in the cached GrTexture).

Review URL: https://codereview.chromium.org/639863005
2014-10-22 05:33:52 -07:00
robertphillips
3aac6e0848 Add clip to layer cache
This CL adds the clip region to the GPU layer hoisting image cache. It also switches back to the old caching behavior of using the entire CTM in the cache key rather then just the upper 2x2. This latter change is to focus more on hoisting rather then caching.

It also includes 2 smaller fixes:
a) layer's that have an image filter are no longer atlased (b.c. doing so complicates applying the image filter)

b) the result of clipping the layer's bounds to the current clip is used as the hoisted layer's size. This reduces the amount of pixels drawn to match a normal (non-hoisted) draw pass.

Review URL: https://codereview.chromium.org/640773004
2014-10-20 08:52:40 -07:00
robertphillips
232f6b0f90 Remove unused SaveLayerInfo::fValid field
Minor code cleanup

Review URL: https://codereview.chromium.org/643713002
2014-10-09 16:43:42 -07:00
mtklein
208d1704c2 Add SkBBoxHierarchy::reserve() as an optional size hint.
I want to play around with how SkTileGrid stores its tiles.  Having a
cap on the number of insert() calls can be pretty handy.

While I'm at it, I gave flush() a default empty impl.  Like reserve(),
it's really an optional hook for subclasses.

BUG=skia:

Review URL: https://codereview.chromium.org/639933003
2014-10-09 06:49:47 -07:00
mtklein
8f8c25eabb Demote getCount, getDepth, and clear to RTree-only methods.
We use them only to test RTree.

BUG=skia:

Review URL: https://codereview.chromium.org/622773003
2014-10-02 09:53:04 -07:00
mtklein
6bd41969a0 BBHs: void* data -> unsigned data
Now that the old backend's not using BBHs, we can specialize them for
SkRecord's needs.  The only thing we really want to store is op index, which
should always be small enough to fit into an unsigned (unsigned also helps keep
it straight from other ints floating around).

This means we'll need half (32-bit) or a quarter (64-bit) the bytes in SkTileGrid,
because we don't have to store an extra int for ordering.

BUG=skia:2834

Review URL: https://codereview.chromium.org/617393004
2014-10-02 07:41:56 -07:00