Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(282)

Issue 6503073: Simplified stencil buffer caching (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by robertphillips
Modified:
12 years ago
Reviewers:
bsalomon
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

This CL simplifies caching of stencil buffers by: removing the GrResourceCache::kSingle_LockType replacing GrStencilBuffer::fRTAttachmentCnt with nested locks from the reffing render targets This CL also fixes a lock leak in the old system as can be observed in the 'Unlocked Entry Count' fields in the following. BEFORE config: gpu 4cad70 Budget: 256 items 16777216 bytes Entry Count: current 9 high 163 Unlocked Entry Count: current 0 high 162 Entry Bytes: current 11591600 high 198836800 Detached Entry Count: current 0 high 13 Detached Bytes: current 0 high 14133056 config: msaa16 12ad70 Budget: 256 items 16777216 bytes Entry Count: current 12 high 164 Unlocked Entry Count: current 0 high 162 Entry Bytes: current 217151232 high 317456128 Detached Entry Count: current 0 high 13 Detached Bytes: current 0 high 14133056 AFTER config: gpu 6a2140 Budget: 256 items 16777216 bytes Entry Count: current 14 high 168 Unlocked Entry Count: current 13 high 167 Entry Bytes: current 9576980 high 198556800 Detached Entry Count: current 0 high 13 Detached Bytes: current 0 high 14133056 config: msaa16 802140 Budget: 256 items 16777216 bytes Entry Count: current 7 high 163 Unlocked Entry Count: current 7 high 162 Entry Bytes: current 14256000 high 216988800

Patch Set 1 #

Patch Set 2 : Removed change in GrConfig.h #

Total comments: 1

Patch Set 3 : Added comment #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -106 lines) Patch
M include/gpu/GrContext.h View 1 chunk +3 lines, -4 lines 0 comments Download
M include/gpu/GrRenderTarget.h View 1 chunk +4 lines, -0 lines 0 comments Download
M src/gpu/GrContext.cpp View 3 chunks +12 lines, -8 lines 0 comments Download
M src/gpu/GrGpu.cpp View 2 chunks +3 lines, -6 lines 0 comments Download
M src/gpu/GrRenderTarget.cpp View 1 chunk +14 lines, -2 lines 0 comments Download
M src/gpu/GrResourceCache.h View 2 chunks +1 line, -9 lines 0 comments Download
M src/gpu/GrResourceCache.cpp View 2 chunks +2 lines, -4 lines 0 comments Download
M src/gpu/GrStencilBuffer.h View 4 chunks +1 line, -23 lines 0 comments Download
M src/gpu/GrStencilBuffer.cpp View 1 chunk +0 lines, -39 lines 0 comments Download
M src/gpu/gl/GrGLRenderTarget.h View 1 chunk +2 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLRenderTarget.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/gl/GrGLStencilBuffer.h View 2 chunks +4 lines, -4 lines 0 comments Download
M src/gpu/gl/GrGLStencilBuffer.cpp View 1 chunk +0 lines, -2 lines 2 comments Download
M src/gpu/gl/GrGpuGL.cpp View 1 2 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 3
robertphillips
http://codereview.appspot.com/6503073/diff/2001/src/gpu/GrGpu.cpp File src/gpu/GrGpu.cpp (left): http://codereview.appspot.com/6503073/diff/2001/src/gpu/GrGpu.cpp#oldcode159 src/gpu/GrGpu.cpp:159: rt->width(), rt->height())) { These two lines were moved into ...
12 years ago (2012-09-05 13:22:31 UTC) #1
bsalomon
LGTM (w/ 1 question) http://codereview.appspot.com/6503073/diff/4001/src/gpu/gl/GrGLStencilBuffer.cpp File src/gpu/gl/GrGLStencilBuffer.cpp (left): http://codereview.appspot.com/6503073/diff/4001/src/gpu/gl/GrGLStencilBuffer.cpp#oldcode31 src/gpu/gl/GrGLStencilBuffer.cpp:31: INHERITED::onRelease(); Did we have to ...
12 years ago (2012-09-05 13:47:45 UTC) #2
robertphillips
12 years ago (2012-09-05 15:47:13 UTC) #3
committed as r5400

https://codereview.appspot.com/6503073/diff/4001/src/gpu/gl/GrGLStencilBuffer...
File src/gpu/gl/GrGLStencilBuffer.cpp (left):

https://codereview.appspot.com/6503073/diff/4001/src/gpu/gl/GrGLStencilBuffer...
src/gpu/gl/GrGLStencilBuffer.cpp:31: INHERITED::onRelease();
That is why I had to remove them and I think it is wrong too. I will fix that
(and restore these) in a separate CL.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b