Make GrGLProgramDesc's key variable length by compacting the effect key array

Make GrGLProgramDesc's key variable length by compacting the effect key array

R=robertphillips@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9239

[bsalomon, 2013-05-21 17:54:29]

Probably should start with GrGLProgramDesc.h, the rest should make more sense
after groking that.

[robertphillips, 2013-05-21 19:22:37]

lgtm

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgram.h
File src/gpu/gl/GrGLProgram.h (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgram.h#n...
src/gpu/gl/GrGLProgram.h:122: * This is the heavy initialization routine for
building a GLProgram. stages is all the enabled
stags -> stages

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
File src/gpu/gl/GrGLProgramDesc.cpp (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:75: void* newKey =
desc->fKey.reset(newKeyLength);
Does oldKey survive reset?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:81: // write the key length
reset doesn't do this?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:235: 
Do we need the first clear to 0?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc.h
File src/gpu/gl/GrGLProgramDesc.h (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.h:41: 
hache -> cache

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.h:85: GrAssert(fInitialized && other.fInitialized);
Is the ~0x3 necessary here?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h
File src/gpu/gl/GrGpuGL.h (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h#newco...
src/gpu/gl/GrGpuGL.h:190: 
Do we need this pre-declare?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h#newco...
src/gpu/gl/GrGpuGL.h:192: 
of -> for?

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL_program...
File src/gpu/gl/GrGpuGL_program.cpp (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL_program...
src/gpu/gl/GrGpuGL_program.cpp:66: int cacheHits = fTotalRequests -
fCacheMisses;
fTotalRequests -> cacheHits?

[bsalomon, 2013-05-22 14:11:25]

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
File src/gpu/gl/GrGLProgramDesc.cpp (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:75: void* newKey =
desc->fKey.reset(newKeyLength);
On 2013/05/21 19:22:37, robertphillips wrote:
> Does oldKey survive reset?

I just wanted to check whether reset() actually allocated new memory or not... I
guess this isn't strictly correct as malloc could return the same ptr that
free() just deleted and have the contents may have changed.

I modified reset() to report this directly.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:81: // write the key length
On 2013/05/21 19:22:37, robertphillips wrote:
> reset doesn't do this?

reset() was called on an SkAutoSMalloc.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.cpp:235: 
On 2013/05/21 19:22:37, robertphillips wrote:
> Do we need the first clear to 0?
Yeah, the memory where the checksum will live is also being checksummed.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc.h
File src/gpu/gl/GrGLProgramDesc.h (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.h:41: 
On 2013/05/21 19:22:37, robertphillips wrote:
> hache -> cache

Done. (actually hash)

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGLProgramDesc...
src/gpu/gl/GrGLProgramDesc.h:85: GrAssert(fInitialized && other.fInitialized);
On 2013/05/21 19:22:37, robertphillips wrote:
> Is the ~0x3 necessary here?

No... but I was hoping the compiler would see that the length is guaranteed to
be 4 byte aligned... the dissasembly on windows is disappointing though.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h
File src/gpu/gl/GrGpuGL.h (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h#newco...
src/gpu/gl/GrGpuGL.h:190: 
On 2013/05/21 19:22:37, robertphillips wrote:
> Do we need this pre-declare?

It has to have access to GrGpuGL::ProgramCache::Entry, so must either be a
friend or nested class of GrGpuGL::ProgramCache.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL.h#newco...
src/gpu/gl/GrGpuGL.h:192: 
On 2013/05/21 19:22:37, robertphillips wrote:
> of -> for?

Done.

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL_program...
File src/gpu/gl/GrGpuGL_program.cpp (right):

https://codereview.chromium.org/15252004/diff/4001/src/gpu/gl/GrGpuGL_program...
src/gpu/gl/GrGpuGL_program.cpp:66: int cacheHits = fTotalRequests -
fCacheMisses;
On 2013/05/21 19:22:37, robertphillips wrote:
> fTotalRequests -> cacheHits?

Done.

[bsalomon, 2013-05-22 14:34:35]

Committed patchset #4 manually as r9239 (presubmit successful).