Update effect key for descriptor to not record offsets.

Update effect key for descriptor to not record offsets.

BUG=skia:

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

[egdaniel, 2014-11-25 22:20:04]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com, joshualitt@google.com

[egdaniel, 2014-11-25 22:20:04]

A follow up to the previous key CL's to update the actually recorded key

[bsalomon, 2014-11-26 13:41:55]

Looking at this now, it seems like there is extra complexity just to keep the
sizes before the keys (e.g. passing around keyIndex). Now that we don't read
them anymore, it seems like it'd be simpler to write key,size,key,size,... and
not have to do the address calculations.

[bsalomon, 2014-11-26 13:42:25]

On 2014/11/26 13:41:55, bsalomon wrote:
> Looking at this now, it seems like there is extra complexity just to keep the
> sizes before the keys (e.g. passing around keyIndex). Now that we don't read
> them anymore, it seems like it'd be simpler to write key,size,key,size,... and
> not have to do the address calculations.

I mean sizeIndex not keyIndex.

[egdaniel, 2014-11-26 15:19:40]

new version should be a lot simpler

[bsalomon, 2014-11-26 15:25:06]

looks much better than the old code. One question about the meta key and request
to clarify the comments describing the key structure.

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
File src/gpu/gl/GrGLProgramDesc.cpp (left):

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
src/gpu/gl/GrGLProgramDesc.cpp:151: struct GeometryProcessorKeyBuilder {
good riddance.

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
File src/gpu/gl/GrGLProgramDesc.cpp (right):

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
src/gpu/gl/GrGLProgramDesc.cpp:142: key[1] = (classID << 16 | processorKeySize);
fail if key size and classID overlap?

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
File src/gpu/gl/GrGLProgramDesc.h (right):

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
src/gpu/gl/GrGLProgramDesc.h:32: // 5. per-effect keys and key length pairs.
Hmm....

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
src/gpu/gl/GrGLProgramDesc.h:39: // This is the offset in the overall key to the
array of effect keys.
can we just say "to the effect keys". "Array" is slightly misleading.

Actually, this should probably be the offset to the backend-specific part of the
key, which include per-processor keys. Similar updates to the enumerated list
above.

[egdaniel, 2014-11-26 15:47:51]

updated

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
File src/gpu/gl/GrGLProgramDesc.cpp (right):

https://codereview.chromium.org/760593005/diff/20001/src/gpu/gl/GrGLProgramDe...
src/gpu/gl/GrGLProgramDesc.cpp:142: key[1] = (classID << 16 | processorKeySize);
On 2014/11/26 15:25:06, bsalomon wrote:
> fail if key size and classID overlap?

In practice the keys would have never overlapped, but there was a bug if the
size was sooo large that some bits > 32 were 1, bits 17-32 were 0's, and the
lower bits were whatever, it wouldn't have been caught by the previous check
above

[bsalomon, 2014-11-26 15:49:55]

lgtm

[egdaniel, 2014-11-26 15:53:09]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-11-26 15:53:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/760593005/60001

[commit-bot: I haz the power, 2014-11-26 16:50:54]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/c67870cfc1e854f1b8a4142bcbc52414beb4207d