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

Issue 26557003: Add a GPU paths to resource cache of the context (Closed)

Created:
7 years, 2 months ago by Kimmo Kinnunen
Modified:
7 years, 1 month ago
Reviewers:
bsalomon, mtklein
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Make GrContext cache the gpu paths Creating paths for nv_path_rendering is costly. Try to reduce this cost by caching paths based on the SkPath "hash" (i.e. SkPathRef generation id) and stroke properties. Adds the paths to GrContext::fTextureCache instance. Later this should be renamed and the GrContext API should reflect the nature of the cache better. Committed: http://code.google.com/p/skia/source/detail?r=12083

Patch Set 1 #

Patch Set 2 : indent #

Total comments: 10

Patch Set 3 : reupload #

Patch Set 4 : making GrContext api correct #

Patch Set 5 : upload the correct patch #

Patch Set 6 : pre-empt #

Patch Set 7 : address review comments #

Patch Set 8 : a #

Total comments: 4

Patch Set 9 : address the friend -comment #

Patch Set 10 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -14 lines) Patch
M include/core/SkStrokeRec.h View 1 2 3 4 5 1 chunk +8 lines, -0 lines 0 comments Download
M include/gpu/GrContext.h View 1 2 3 4 5 6 7 8 9 3 chunks +11 lines, -0 lines 0 comments Download
M src/gpu/GrContext.cpp View 1 2 3 4 5 6 7 8 9 2 chunks +17 lines, -0 lines 0 comments Download
M src/gpu/GrPath.h View 1 2 3 3 chunks +14 lines, -3 lines 0 comments Download
M src/gpu/GrPath.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +23 lines, -0 lines 0 comments Download
M src/gpu/GrStencilAndCoverPathRenderer.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLPath.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLPath.cpp View 1 2 3 4 chunks +5 lines, -7 lines 0 comments Download

Messages

Total messages: 15 (0 generated)
Kimmo Kinnunen
Ready for review
7 years, 2 months ago (2013-10-15 10:57:23 UTC) #1
bsalomon
On 2013/10/15 10:57:23, kkinnunen wrote: > Ready for review Hi Kimmo, there are some issues ...
7 years, 2 months ago (2013-10-15 13:27:34 UTC) #2
mtklein
https://chromiumcodereview.appspot.com/26557003/diff/3001/include/core/SkPathRef.h File include/core/SkPathRef.h (right): https://chromiumcodereview.appspot.com/26557003/diff/3001/include/core/SkPathRef.h#newcode237 include/core/SkPathRef.h:237: nit: this->genID() https://chromiumcodereview.appspot.com/26557003/diff/3001/src/gpu/GrContext.cpp File src/gpu/GrContext.cpp (right): https://chromiumcodereview.appspot.com/26557003/diff/3001/src/gpu/GrContext.cpp#newcode138 src/gpu/GrContext.cpp:138: Seems ...
7 years, 2 months ago (2013-10-15 15:29:50 UTC) #3
Kimmo Kinnunen
Thanks for the comments. A new version is up. https://codereview.chromium.org/26557003/diff/3001/include/core/SkPathRef.h File include/core/SkPathRef.h (right): https://codereview.chromium.org/26557003/diff/3001/include/core/SkPathRef.h#newcode237 include/core/SkPathRef.h:237: ...
7 years, 2 months ago (2013-10-16 06:14:28 UTC) #4
mtklein
I see you've done a lot of refactoring that's somewhat incidental to caching GPU paths. ...
7 years, 2 months ago (2013-10-17 13:38:26 UTC) #5
Kimmo Kinnunen
On 2013/10/17 13:38:26, mtklein wrote: > I see you've done a lot of refactoring that's ...
7 years, 2 months ago (2013-10-18 09:20:05 UTC) #6
bsalomon
Mostly looking good to me, a couple comments inline. https://codereview.chromium.org/26557003/diff/178001/include/core/SkPath.h File include/core/SkPath.h (right): https://codereview.chromium.org/26557003/diff/178001/include/core/SkPath.h#newcode929 include/core/SkPath.h:929: ...
7 years, 2 months ago (2013-10-18 13:25:24 UTC) #7
Kimmo Kinnunen
https://codereview.chromium.org/26557003/diff/178001/include/core/SkPath.h File include/core/SkPath.h (right): https://codereview.chromium.org/26557003/diff/178001/include/core/SkPath.h#newcode929 include/core/SkPath.h:929: uint32_t getHash() const; On 2013/10/18 13:25:24, bsalomon wrote: > ...
7 years, 2 months ago (2013-10-21 13:40:48 UTC) #8
Kimmo Kinnunen
Any other comments or any other modifications I could do to get this reviewed again?
7 years, 1 month ago (2013-10-28 15:08:33 UTC) #9
bsalomon
On 2013/10/28 15:08:33, kkinnunen wrote: > Any other comments or any other modifications I could ...
7 years, 1 month ago (2013-10-28 19:59:09 UTC) #10
bsalomon
On 2013/10/28 19:59:09, bsalomon wrote: > On 2013/10/28 15:08:33, kkinnunen wrote: > > Any other ...
7 years, 1 month ago (2013-10-31 15:43:05 UTC) #11
Kimmo Kinnunen
On 2013/10/31 15:43:05, bsalomon wrote: > On 2013/10/28 19:59:09, bsalomon wrote: > > On 2013/10/28 ...
7 years, 1 month ago (2013-11-01 13:25:11 UTC) #12
bsalomon
lgtm
7 years, 1 month ago (2013-11-01 14:46:18 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/kkinnunen@nvidia.com/26557003/388001
7 years, 1 month ago (2013-11-01 14:57:15 UTC) #14
commit-bot: I haz the power
7 years, 1 month ago (2013-11-01 15:23:52 UTC) #15
Message was sent while issue was closed.
Change committed as 12083

Powered by Google App Engine
This is Rietveld 408576698