Implement texel buffers

Implement texel buffers

Adds a mechanism for processors to add buffer accesses and implements
them in the GL backend.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1870893002

Committed: https://skia.googlesource.com/skia/+/74b8d323323c8533e3e5cc7719e0bd127aacd829

[Chris Dalton, 2016-04-08 00:13:15]

Description was changed from

==========
Implement texel buffers

Adds a mechanism for processors to add buffer accesses and implements
them in the GL backend.

BUG=skia:
==========

to

==========
Implement texel buffers

Adds a mechanism for processors to add buffer accesses and implements
them in the GL backend.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[Chris Dalton, 2016-04-08 00:56:42]

cdalton@nvidia.com changed reviewers:
+ bsalomon@google.com

[Chris Dalton, 2016-04-08 00:56:43]

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrProcessor.cpp
File src/gpu/GrProcessor.cpp (right):

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrProcessor.cpp#new...
src/gpu/GrProcessor.cpp:109: void GrProcessor::addBufferAccess(const
GrBufferAccess* access) {
Is there a kosher way to get a hold of a caps object here to assert texel buffer
support? If not, the program builder will do it later.

[bsalomon, 2016-04-11 16:51:38]

lgtm with some minor things inline

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrBufferAccess.h
File src/gpu/GrBufferAccess.h (right):

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrBufferAccess.h#ne...
src/gpu/GrBufferAccess.h:38: bool operator!=(const GrBufferAccess& other) const
{ return !(*this == other); }
other->that? just for consistency with op==

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrProcessor.cpp
File src/gpu/GrProcessor.cpp (right):

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrProcessor.cpp#new...
src/gpu/GrProcessor.cpp:109: void GrProcessor::addBufferAccess(const
GrBufferAccess* access) {
On 2016/04/08 00:56:43, Chris Dalton wrote:
> Is there a kosher way to get a hold of a caps object here to assert texel
buffer
> support? If not, the program builder will do it later.

Not really. I wouldn't bend over backwards for that assert.

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

https://codereview.chromium.org/1870893002/diff/1/src/gpu/gl/GrGLBuffer.h#new...
src/gpu/gl/GrGLBuffer.h:36: void setHasAttachedToTexture() {
fHasAttachedToTexture = true; }
Maybe (set)isAttachedToTexture? More clear that is is now not just has been at
some point.

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

https://codereview.chromium.org/1870893002/diff/1/src/gpu/gl/GrGLProgram.h#ne...
src/gpu/gl/GrGLProgram.h:97: void setData(const GrPrimitiveProcessor&, const
GrPipeline&, bool allowSRGBInputs);
Is this bool not just gettable from the GrPipeline now?

[Chris Dalton, 2016-04-11 19:48:19]

Just let me know if you would prefer the counting approach instead of
"hasAttachedToTexture" (in the comments).

Also, have you had a chance to look at
https://codereview.chromium.org/1869063005/ ? That is a dependency for this
patch.

Thanks!

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrBufferAccess.h
File src/gpu/GrBufferAccess.h (right):

https://codereview.chromium.org/1870893002/diff/1/src/gpu/GrBufferAccess.h#ne...
src/gpu/GrBufferAccess.h:38: bool operator!=(const GrBufferAccess& other) const
{ return !(*this == other); }
On 2016/04/11 16:51:38, bsalomon wrote:
> other->that? just for consistency with op==

Done.

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

https://codereview.chromium.org/1870893002/diff/1/src/gpu/gl/GrGLBuffer.h#new...
src/gpu/gl/GrGLBuffer.h:36: void setHasAttachedToTexture() {
fHasAttachedToTexture = true; }
On 2016/04/11 16:51:38, bsalomon wrote:
> Maybe (set)isAttachedToTexture? More clear that is is now not just has been at
> some point.

Actually the intention of this bool is to know if it has *ever* been attached to
a
texture. Then upon deletion, it only tries to detach it from any possible buffer
textures if it has been attached at some point in the past.

We could instead be more elaborate and complex, and maintain a count of how many
buffer
textures it is attached to. Do you prefer one way or the other? I don't have a
preference myself.

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

https://codereview.chromium.org/1870893002/diff/1/src/gpu/gl/GrGLProgram.h#ne...
src/gpu/gl/GrGLProgram.h:97: void setData(const GrPrimitiveProcessor&, const
GrPipeline&, bool allowSRGBInputs);
On 2016/04/11 16:51:38, bsalomon wrote:
> Is this bool not just gettable from the GrPipeline now?

Done.

[bsalomon, 2016-04-11 20:37:11]

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

https://codereview.chromium.org/1870893002/diff/1/src/gpu/gl/GrGLBuffer.h#new...
src/gpu/gl/GrGLBuffer.h:36: void setHasAttachedToTexture() {
fHasAttachedToTexture = true; }
On 2016/04/11 19:48:19, Chris Dalton wrote:
> On 2016/04/11 16:51:38, bsalomon wrote:
> > Maybe (set)isAttachedToTexture? More clear that is is now not just has been
at
> > some point.
> 
> Actually the intention of this bool is to know if it has *ever* been attached
to
> a
> texture. Then upon deletion, it only tries to detach it from any possible
buffer
> textures if it has been attached at some point in the past.
> 
> We could instead be more elaborate and complex, and maintain a count of how
many
> buffer
> textures it is attached to. Do you prefer one way or the other? I don't have a
> preference myself.

Got it. I'm ok with it as is.

[Chris Dalton, 2016-04-11 20:41:40]

The CQ bit was checked by cdalton@nvidia.com

[Chris Dalton, 2016-04-11 20:41:41]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1870893002/#ps20001
(title: "rebase + comments")

[commit-bot: I haz the power, 2016-04-11 20:41:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870893002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870893002/20001

[commit-bot: I haz the power, 2016-04-11 20:43:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 20:43:20]

Try jobs failed on following builders:
  Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)

[Chris Dalton, 2016-04-11 21:30:05]

The CQ bit was checked by cdalton@nvidia.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 21:30:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870893002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870893002/40001

[commit-bot: I haz the power, 2016-04-11 21:45:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 21:45:07]

Dry run: This issue passed the CQ dry run.

[Chris Dalton, 2016-04-11 21:45:57]

The CQ bit was checked by cdalton@nvidia.com

[Chris Dalton, 2016-04-11 21:45:58]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1870893002/#ps40001
(title: "GrBuffer(Access) into include/gpu")

[commit-bot: I haz the power, 2016-04-11 21:46:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1870893002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1870893002/40001

[commit-bot: I haz the power, 2016-04-11 21:47:31]

Description was changed from

==========
Implement texel buffers

Adds a mechanism for processors to add buffer accesses and implements
them in the GL backend.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Implement texel buffers

Adds a mechanism for processors to add buffer accesses and implements
them in the GL backend.

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/74b8d323323c8533e3e5cc7719e0bd127aacd829
==========

[commit-bot: I haz the power, 2016-04-11 21:47:33]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/74b8d323323c8533e3e5cc7719e0bd127aacd829