Use all available texture units.

Use all available texture units.

Committed: http://code.google.com/p/skia/source/detail?r=9451

[bsalomon, 2013-06-05 14:38:38]

Removes a dependency on kNumStages.

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (left):

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGpuGL.cpp#oldcod...
src/gpu/gl/GrGpuGL.cpp:121: static bool fbo_test(const GrGLInterface* gl, int w,
int h) {
This would have to be update and hasn't been used in a long long time.

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGpuGL.cpp#oldcod...
src/gpu/gl/GrGpuGL.cpp:476: this->setSpareTextureUnit();
don't know why this was here.

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:2201: if (lastUnitIdx != fHWActiveTextureUnitIdx) {
fixes a bug here, the old comparison was wrong.

[jvanverth1, 2013-06-05 14:51:15]

https://codereview.chromium.org/16452007/diff/1/include/core/SkTArray.h
File include/core/SkTArray.h (right):

https://codereview.chromium.org/16452007/diff/1/include/core/SkTArray.h#newco...
include/core/SkTArray.h:22: }
Given that these are fixes in SkTArray, it might be better to move them to a
separate changelist.

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGLCaps.h#newcode221
src/gpu/gl/GrGLCaps.h:221: /// maximum number of textures units accessible in
the fragment shader.
Typo (texture*s*).

[bsalomon, 2013-06-05 15:00:57]

https://codereview.chromium.org/16452007/diff/1/include/core/SkTArray.h
File include/core/SkTArray.h (right):

https://codereview.chromium.org/16452007/diff/1/include/core/SkTArray.h#newco...
include/core/SkTArray.h:22: }
On 2013/06/05 14:51:15, JimVV wrote:
> Given that these are fixes in SkTArray, it might be better to move them to a
> separate changelist.

https://codereview.chromium.org/15949021

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGLCaps.h
File src/gpu/gl/GrGLCaps.h (right):

https://codereview.chromium.org/16452007/diff/1/src/gpu/gl/GrGLCaps.h#newcode221
src/gpu/gl/GrGLCaps.h:221: /// maximum number of textures units accessible in
the fragment shader.
On 2013/06/05 14:51:15, JimVV wrote:
> Typo (texture*s*). 

Done.

[jvanverth1, 2013-06-05 15:15:25]

lgtm + nits/questions

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:2192: if (fHWActiveTextureUnitIdx != unit) {
Change to (unit != fHWActiveTextureUnitIdx) to match pattern below?

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:2205: // clear out the this field so that if a program
does use this unit it will rebind the correct
I assume GrRefCnt will take care of any leaks here.

[bsalomon, 2013-06-05 15:45:10]

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:2192: if (fHWActiveTextureUnitIdx != unit) {
On 2013/06/05 15:15:25, JimVV wrote:
> Change to (unit != fHWActiveTextureUnitIdx) to match pattern below?

Done.

https://codereview.chromium.org/16452007/diff/8/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:2205: // clear out the this field so that if a program
does use this unit it will rebind the correct
On 2013/06/05 15:15:25, JimVV wrote:
> I assume GrRefCnt will take care of any leaks here.

These are raw ptrs and don't own a ref. If this ref'ed a texture then it could
prevent that texture from ever being deleted. Instead, when the texture is
deleted it notifies GrGpuGL via notifyTextureDelete() and that function will
clear out any pointers to the texture being deleted.

Answering this caused me to notice that there were a couple other spots that
needed to be updated (loops on fHWBoundTextures that used kNumStages).

[jvanverth1, 2013-06-05 15:58:40]

https://codereview.chromium.org/16452007/diff/9002/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

https://codereview.chromium.org/16452007/diff/9002/src/gpu/gl/GrGpuGL.cpp#new...
src/gpu/gl/GrGpuGL.cpp:2200: int lastUnitIdx =
this->glCaps().maxFragmentTextureUnits() - 1;
Your last change leads me to think that it would be more consistent to use
fHWBoundTextures.count() here instead.

[bsalomon, 2013-06-05 17:16:45]

https://codereview.chromium.org/16452007/diff/9002/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

https://codereview.chromium.org/16452007/diff/9002/src/gpu/gl/GrGpuGL.cpp#new...
src/gpu/gl/GrGpuGL.cpp:2200: int lastUnitIdx =
this->glCaps().maxFragmentTextureUnits() - 1;
On 2013/06/05 15:58:40, JimVV wrote:
> Your last change leads me to think that it would be more consistent to use
> fHWBoundTextures.count() here instead.

good point, will change before landing.

[commit-bot: I haz the power, 2013-06-05 17:20:07]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/bsalomon@google.com/16452007/9004

[commit-bot: I haz the power, 2013-06-05 23:29:28]

Change committed as 9451