Commit Graph

34 Commits

Author SHA1 Message Date
mtklein
ed167acb7b Revert of Use SkTypeface::getBounds() in bounding-box calculations. (patchset #5 id:80001 of https://codereview.chromium.org/680363003/)
Reason for revert:
Mac mini asserting

Original issue's description:
> Use SkTypeface::getBounds() in bounding-box calculations.
>
> This should produce tighter conservative bounding boxes for text than the
> approximation code it replaces.
>
> Recording performance is neutral on my desktop.  Playback performance
> improves by up to 15% on text heavy pages, e.g.
>
>   desk_pokemonwiki.skp_1 3.24ms -> 2.83ms  0.87x
>         desk_baidu.skp_1 1.91ms -> 1.58ms  0.83x
>
> Committed: https://skia.googlesource.com/skia/+/bf8dc343df4fbdcb8af546eb68b640e011a33489
>
> CQ_EXTRA_TRYBOTS=client.skia:Test-Win7-ShuttleA-HD2000-x86-Debug-Trybot
>
> Committed: https://skia.googlesource.com/skia/+/c51add674dfb89b988a7fbc05f41838c203f9dcd

TBR=reed@google.com,mtklein@chromium.org
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/690833002
2014-10-29 16:07:11 -07:00
mtklein
c51add674d Use SkTypeface::getBounds() in bounding-box calculations.
This should produce tighter conservative bounding boxes for text than the
approximation code it replaces.

Recording performance is neutral on my desktop.  Playback performance
improves by up to 15% on text heavy pages, e.g.

  desk_pokemonwiki.skp_1 3.24ms -> 2.83ms  0.87x
        desk_baidu.skp_1 1.91ms -> 1.58ms  0.83x

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

CQ_EXTRA_TRYBOTS=client.skia:Test-Win7-ShuttleA-HD2000-x86-Debug-Trybot

Review URL: https://codereview.chromium.org/680363003
2014-10-29 15:36:38 -07:00
mtklein
27f965a577 Revert of Use SkTypeface::getBounds() in bounding-box calculations. (patchset #3 id:40001 of https://codereview.chromium.org/680363003/)
Reason for revert:
http://build.chromium.org/p/client.skia/builders/Test-Win7-ShuttleA-HD2000-x86-Debug/builds/97/steps/nanobench/logs/stdio

Original issue's description:
> Use SkTypeface::getBounds() in bounding-box calculations.
>
> This should produce tighter conservative bounding boxes for text than the
> approximation code it replaces.
>
> Recording performance is neutral on my desktop.  Playback performance
> improves by up to 15% on text heavy pages, e.g.
>
>   desk_pokemonwiki.skp_1 3.24ms -> 2.83ms  0.87x
>         desk_baidu.skp_1 1.91ms -> 1.58ms  0.83x
>
> Committed: https://skia.googlesource.com/skia/+/bf8dc343df4fbdcb8af546eb68b640e011a33489

TBR=reed@google.com,mtklein@chromium.org
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/685173002
2014-10-29 08:33:38 -07:00
mtklein
bf8dc343df Use SkTypeface::getBounds() in bounding-box calculations.
This should produce tighter conservative bounding boxes for text than the
approximation code it replaces.

Recording performance is neutral on my desktop.  Playback performance
improves by up to 15% on text heavy pages, e.g.

  desk_pokemonwiki.skp_1 3.24ms -> 2.83ms  0.87x
        desk_baidu.skp_1 1.91ms -> 1.58ms  0.83x

Review URL: https://codereview.chromium.org/680363003
2014-10-29 08:12:08 -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
sugoi
234f036b3e Adding an option to render only the shadow in SkDropShadowImageFilter
This is basically how blink uses the filter. Currently, I can't use it for "ShadowOnly" mode with the filter at all, but instead of copying the code and risking to have the codepaths diverge, I'm simply going to add the option here.

BUG=skia:

Review URL: https://codereview.chromium.org/646213004
2014-10-23 13:59:52 -07:00
mtklein
41966d49b0 Dilate approximated text bounds to squelch recent assertion failure.
This is once again an issue related to logo fonts, so I don't
see any easy way to add a regression test for this.

BUG=424824

Review URL: https://codereview.chromium.org/665103002
2014-10-20 13:44:24 -07:00
piotaixr
65151754b9 Override SkCanvas::drawImage() in SkRecorder.
BUG=skia:2947

Review URL: https://codereview.chromium.org/610003002
2014-10-16 11:58:39 -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
mtklein
8e393bf70e Don't adjust the bounds after a restore with the restore's paired saveLayer's paint.
It makes no sense for the paint from a saveLayer to effect anything outside its saveLayer/restore block.  But as currently written, we'll adjust the clip bounds just after a restore by that paint.

Turns out the test I wrote for the last CL (which caused this bug) actually had the bug baked into its expectations.  I've updated them and added some notes to explain.

BUG=418417

Review URL: https://codereview.chromium.org/623563002
2014-10-01 12:48:58 -07:00
Mike Klein
271a030f5d We need to adjust the bounds of clip ops with SaveLayer paints too.
Before this CL, SkRecord only adjusted the bounds of draw ops for SaveLayers' paints.
That worked fine, but as a final step we intersect the bounds of draw ops with the
bounds of the current clip, essentially undoing all that work.

I think the right fix here is to also adjust the bounds of the clip ops.

BUG=skia:2957, 415468
R=robertphillips@google.com

Review URL: https://codereview.chromium.org/595953002
2014-09-23 15:28:38 -04:00
robertphillips
4815fe5a0a Fix bug in layer hoisting transition to SkRecord backend
Care must be taken when setting up the initial CTM matrix for partial SkRecord playbacks b.c. all the setMatrix calls will concatenate with the initial matrix (which may be different then the CTM that is required to draw correctly).

R=mtklein@google.com, bsalomon@google.com

Author: robertphillips@google.com

Review URL: https://codereview.chromium.org/549143003
2014-09-16 10:32:43 -07:00
mtklein
937c9c7eb4 Fix drawPosText() bounds bug.
We didn't catch this in our local tests because we tend to use default
kUTF8_TextEncoding with single-byte characters, which means N == byteLength.

BUG=409110
R=reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/531933002
2014-09-02 15:19:48 -07:00
mtklein
00f30bdc9e SkRecordPartialDraw with less code duplication
BUG=skia:
R=robertphillips@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/527423002
2014-09-02 12:03:31 -07:00
mtklein
90e8457ab3 Increase test tolerance.
NOTREECHECKS=true

CQ_EXTRA_TRYBOTS=tryserver.skia:Test-Mac10.8-MacMini4.1-GeForce320M-x86_64-Release-Trybot

BUG=skia:
R=mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/516503003
2014-08-28 05:14:31 -07:00
Mike Klein
5ff9132693 More test debugging. Too close for decimal. I'm switching to hex.
BUG=skia:

Review URL: https://codereview.chromium.org/515753005
2014-08-28 07:15:32 -04:00
Mike Klein
56fa442503 Add some debugging to figure out what's up with failing Mac 10.8 Release bot.
BUG=skia:

Review URL: https://codereview.chromium.org/511013002
2014-08-27 19:08:52 -04:00
mtklein
533eb782ed Convert BBH APIs to use SkRect.
Still TODO: convert internals of SkTileGrid.cpp and SkRTree.cpp to work in floats too.

NOTREECHECKS=true

BUG=skia:1021
R=robertphillips@google.com, reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/511613002
2014-08-27 10:39:42 -07:00
mtklein
62b67ae96e Start actually bounding some draw ops.
This covers most of the common draws.

BUG=skia:
R=robertphillips@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/469213007
2014-08-18 11:10:37 -07:00
mtklein
a723b576ae SkRecordDraw: incorporate clip into BBH
NOTREECHECKS=true

BUG=skia:
R=robertphillips@google.com, senorblanco@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/474983002
2014-08-15 11:49:49 -07:00
mtklein
5ad6ee1b2c Plumbing for using a BBH in SkRecordDraw.
For now this only creates a degenerate bounding box hierarchy where all ops
just have maximal bounds.  I will flesh out FillBounds in future CL(s).

Not quite sure why QuadTree and TileGrid aren't drawing right---haven't even
looked at the diffs yet---so I've disabled those test modes for now.  RTree
seems fine, so that'll at least get us coverage for all this new plumbing.

BUG=skia:
R=robertphillips@google.com, mtklein@google.com, reed@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/454123003
2014-08-11 08:08:43 -07:00
mtklein
f4078ad1ec SkRecord: Strip out cull-skipping and y-only drawPosTextH skipping.
These optimizations are outclassed by a general bounding-box hierarchy,
and are just going to make plugging that into SkRecordDraw more complicated.

BUG=skia:
R=robertphillips@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/452983002
2014-08-08 10:05:20 -07:00
Mike Klein
c11530ea73 Tick off some TODOs:
- support fRecord in copy constructor
  - support SkDrawPictureCallback

Moved SkDrawPictureCallback to its own header so
SkRecordDraw can include it without pulling in all of
SkPicture.

Adding an SkAutoSaveRestore to SkRecordDraw was the easiest
way to match the balance guarantees of the callback, and
probably not a bad idea in general.  Updated its tests.

BUG=skia:
R=robertphillips@google.com

Review URL: https://codereview.chromium.org/349973008
2014-06-24 11:29:06 -04:00
commit-bot@chromium.org
a095041f51 Remove SkRecorder's kWriteOnly mode.
I'm soon going to have SkRecorder start calling getTotalMatrix(), which
would be broken in write-only mode.  That change is big and nebulous,
but it's clear kWriteOnly needs to go, so we might as well kill it now.

My notes in bench_playback about kWriteOnly mode being important were
probably overly cautious.  I now think this is a fair enough comparison
even re-recording into a read-write canvas.

BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/290653004

git-svn-id: http://skia.googlecode.com/svn/trunk@14963 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-05-29 16:52:40 +00:00
commit-bot@chromium.org
0a98d87044 Don't clobber initial transform with SetMatrix.
BUG=skia:2378
R=reed@google.com, mtklein@google.com, robertphillips@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/290883004

git-svn-id: http://skia.googlecode.com/svn/trunk@14778 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-05-19 15:15:24 +00:00
commit-bot@chromium.org
8dac8b18ee Backfill unit tests for SkRecord
BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/251133008

git-svn-id: http://skia.googlecode.com/svn/trunk@14455 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-30 13:18:12 +00:00
commit-bot@chromium.org
705b1ff617 Don't bother doing the empty clip check in SkRecordDraw.
On Mike's suggestion, I tested out not doing any empty-clip check at all in
SkRecordDraw, given that mostly we'll do that again anyway inside SkCanvas.

Most SKPs are identical to the status quo, whether bot or silk, played back in tiles
or full.  Average playback performance, both arithmetic and geometric mean, is also
unchanged.

A handful of SKPs do draw faster or slower reliably, particularly when tiled.  E.g. a
cnn tile draws about 40% faster, a cuteoverload tile about 20% slower.  Their profiles
look pretty much the same before and after, so I can't really explain the changes.

I'd say, given that performance is mostly identical and very identical in bulk,
we might as well remove this code.  It's nice to keep SkRecordDraw as dumb as possible.

BUG=skia:2378
R=reed@google.com, fmalita@chromium.org, mtklein@google.com, borenet@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/258183002

git-svn-id: http://skia.googlecode.com/svn/trunk@14433 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-29 15:34:03 +00:00
commit-bot@chromium.org
2e0c32af05 Start using type traits in src/record instead of macros.
Simplified skip logic by always running clip commands.  No performance difference on bot or silk SKPs.

BUG=skia:2378
R=bungeman@google.com, fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/258693006

git-svn-id: http://skia.googlecode.com/svn/trunk@14410 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-28 16:19:45 +00:00
commit-bot@chromium.org
ad8ce572f6 anticipate more optimizations by renaming some files and methods
also, call the new SkRecordOptimize in bench_playback

BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/243243003

git-svn-id: http://skia.googlecode.com/svn/trunk@14277 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-21 15:03:36 +00:00
commit-bot@chromium.org
c4b21e6c03 Mark our territory with (C).
BUG=skia:
R=reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/235253002

git-svn-id: http://skia.googlecode.com/svn/trunk@14158 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-11 18:33:31 +00:00
commit-bot@chromium.org
ff2de7cb94 SkRecordDraw: don't bother clipping an empty clip down further
BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com, fmalita@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/231933003

git-svn-id: http://skia.googlecode.com/svn/trunk@14126 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-10 02:26:33 +00:00
commit-bot@chromium.org
d9ce2be6b2 SkRecordDraw: skip draw ops when the clip is empty
- Adds tests for SkRecordDraw's two main features: cull- and clip- based skipping.
   - Adds full SkCanvas semantic mode to SkRecorder, so it can be used as a target for SkRecordDraw.

BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/231653002

git-svn-id: http://skia.googlecode.com/svn/trunk@14124 2bbb7eff-a529-9590-31e7-b0007b416f81
2014-04-09 23:30:28 +00:00