Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(711)

Issue 245853002: Refactor SkRecord opts, converting playback optimizations where possible. (Closed)

Created:
6 years, 8 months ago by mtklein_C
Modified:
6 years, 8 months ago
Reviewers:
f(malita), mtklein
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlesource.com/skia.git@master
Visibility:
Public.

Description

Refactor SkRecord opts, converting playback optimizations where possible. This adds back two optimizations from SkPicture: drawPosText strength reduction to drawPosTextH, and pointless save-foo-restore blocks are noop'd away. The small-T optimization in SkRecord gets in the way of implementing replace(), so I removed it. Just to keep the API focused, I removed the methods on SkRecord that iterate over i for you; it's just as efficient to do it yourself, and all of the interesting code does its own custom iteration. BUG=skia:2378 Committed: http://code.google.com/p/skia/source/detail?r=14300

Patch Set 1 #

Patch Set 2 : notes #

Total comments: 2

Patch Set 3 : apply #

Unified diffs Side-by-side diffs Delta from patch set Stats (+402 lines, -200 lines) Patch
M src/record/SkRecord.h View 1 5 chunks +39 lines, -60 lines 0 comments Download
M src/record/SkRecordDraw.cpp View 4 chunks +41 lines, -68 lines 0 comments Download
M src/record/SkRecordOpts.h View 1 1 chunk +10 lines, -1 line 0 comments Download
M src/record/SkRecordOpts.cpp View 1 1 chunk +215 lines, -18 lines 0 comments Download
M src/record/SkRecorder.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/record/SkRecords.h View 6 chunks +59 lines, -38 lines 0 comments Download
M tests/RecordCullingTest.cpp View 1 2 2 chunks +15 lines, -9 lines 0 comments Download
M tests/RecordTest.cpp View 1 2 3 chunks +15 lines, -3 lines 0 comments Download
M tests/RecorderTest.cpp View 1 2 2 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
mtklein
I had originally developed a rather more aggressive refactoring that changed SkRecord's guts significantly, but ...
6 years, 8 months ago (2014-04-21 21:49:16 UTC) #1
f(malita)
lgtm https://codereview.chromium.org/245853002/diff/20001/tests/RecordTest.cpp File tests/RecordTest.cpp (right): https://codereview.chromium.org/245853002/diff/20001/tests/RecordTest.cpp#newcode49 tests/RecordTest.cpp:49: for (unsigned i = 0; i < record.count(); ...
6 years, 8 months ago (2014-04-22 12:16:09 UTC) #2
mtklein
lgtm https://codereview.chromium.org/245853002/diff/20001/tests/RecordTest.cpp File tests/RecordTest.cpp (right): https://codereview.chromium.org/245853002/diff/20001/tests/RecordTest.cpp#newcode49 tests/RecordTest.cpp:49: for (unsigned i = 0; i < record.count(); ...
6 years, 8 months ago (2014-04-22 15:36:27 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/245853002/40001
6 years, 8 months ago (2014-04-22 15:36:36 UTC) #4
commit-bot: I haz the power
6 years, 8 months ago (2014-04-22 16:57:52 UTC) #5
Message was sent while issue was closed.
Change committed as 14300

Powered by Google App Engine
This is Rietveld 408576698