FBFetch refactor + arm support

FBFetch refactor + arm support

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/5816233d237e5c214f14f1c92eda4a87c898ea11

[krajcevski, 2014-07-30 21:50:19]

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

https://codereview.chromium.org/433603002/diff/1/src/gpu/gl/GrGLShaderBuilder...
src/gpu/gl/GrGLShaderBuilder.cpp:333: this->addFSFeature(1 <<
fGpu->glCaps().fbFetchType(),
Doesn't this clash with kFragCoordConventions_GLSLPrivateFeature. (It has a
value of one, same as kEXT_FBFetchType)?

[joshua.litt, 2014-07-30 22:15:17]

good catch.
On 2014/07/30 21:50:19, krajcevski wrote:
>
https://codereview.chromium.org/433603002/diff/1/src/gpu/gl/GrGLShaderBuilder...
> File src/gpu/gl/GrGLShaderBuilder.cpp (right):
> 
>
https://codereview.chromium.org/433603002/diff/1/src/gpu/gl/GrGLShaderBuilder...
> src/gpu/gl/GrGLShaderBuilder.cpp:333: this->addFSFeature(1 <<
> fGpu->glCaps().fbFetchType(),
> Doesn't this clash with kFragCoordConventions_GLSLPrivateFeature. (It has a
> value of one, same as kEXT_FBFetchType)?

[krajcevski, 2014-07-30 22:38:25]

1gtm

+Brian

[bsalomon, 2014-07-31 13:45:05]

I don't really have a preference between the old and new system for installing
the extension name in the shader, but I'd rather have consistency. Does it make
sense to do a similar refactor of the frag coord conventions stuff?

https://codereview.chromium.org/433603002/diff/50001/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/433603002/diff/50001/src/gpu/gl/GrGLCaps.cpp#...
src/gpu/gl/GrGLCaps.cpp:250: GR_GL_CALL(gli,
Enable(GR_GL_FETCH_PER_SAMPLE_ARM));
I don't think that this is the right place for this call. We don't set any other
state through the caps class. We would enable this once at startup but we are
not always the only user of the GL context. It could be disabled by other code.
It should happen in a function called when we flush the graphics state so that
it gets set again when we get a GrContext::resetContext() call (which is how our
client lets us know that someone else was messing with the GL context).

[joshua.litt, 2014-07-31 17:17:20]

Feedback incorporated.  Their was a try failure, but not in GM, something called
run decoding tests.

[bsalomon, 2014-07-31 17:28:01]

lgtm, with a comment about comments. The failure surely is unrelated to your
change.

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

https://codereview.chromium.org/433603002/diff/70001/src/gpu/gl/GrGpuGL.cpp#n...
src/gpu/gl/GrGpuGL.cpp:279: // On arm we need to reset fetch per sample, used fb
fetch extension.
This sentence is a little unclear. Also, maybe unnecessary given the explanation
in the next comment.

[joshua.litt, 2014-08-01 13:05:45]

The CQ bit was checked by joshualitt@chromium.org

[commit-bot: I haz the power, 2014-08-01 13:06:21]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/joshualitt@chromium.org/433603002/90001

[commit-bot: I haz the power, 2014-08-01 13:45:07]

Change committed as 5816233d237e5c214f14f1c92eda4a87c898ea11