Allow gpu ResourceHandle class to be shared for multiple purposes

Allow gpu ResourceHandle class to be shared for multiple purposes

Currently this class was just used for resource handles when building up
a shader. However, I want to now use this class for things like objects
owned/held by the GrVkResourceProvider which are used by other classes.
An example of this will be for GrVkRenderTargets to hold a handle to a
collection of compatible render passes without having to own/hold onto
the render passes themselves.

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

Committed: https://skia.googlesource.com/skia/+/167ab9198025e577efe033612d2e009633abb54b

[egdaniel, 2016-05-06 14:24:32]

Description was changed from

==========
Allow gpu ResourceHandle class to be shared for multiple purposes

Currently this class was just used for resource handles when building up
a shader. However, I want to now use this class for things like objects
owned/held by the GrVkResourceProvider which are used by other classes.
An example of this will be for GrVkRenderTargets to hold a handle to a
collection of compatible render passes without having to own/hold onto
the render passes themselves.

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

to

==========
Allow gpu ResourceHandle class to be shared for multiple purposes

Currently this class was just used for resource handles when building up
a shader. However, I want to now use this class for things like objects
owned/held by the GrVkResourceProvider which are used by other classes.
An example of this will be for GrVkRenderTargets to hold a handle to a
collection of compatible render passes without having to own/hold onto
the render passes themselves.

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

[egdaniel, 2016-05-06 14:35:24]

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

[bsalomon, 2016-05-06 14:54:53]

On 2016/05/06 14:35:25, egdaniel wrote:

Do we want some way to make this type safe? What I mean is a way to make it so
you can't accidentally use a handle for one resource type when you meant to use
a handle to another resource type.

[robertphillips, 2016-05-06 15:09:00]

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandle.h
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:1: /*
space these over 1 ?

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:20: 
1 line ?

[egdaniel, 2016-05-06 17:08:01]

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandle.h
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:1: /*
On 2016/05/06 15:09:00, robertphillips wrote:
> space these over 1 ?

Done.

https://codereview.chromium.org/1955893002/diff/20001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:20: 
On 2016/05/06 15:09:00, robertphillips wrote:
> 1 line ?

Done.

[bsalomon, 2016-05-06 17:37:54]

https://codereview.chromium.org/1955893002/diff/40001/src/gpu/GrResourceHandle.h
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/40001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:33: #define GR_CREATE_RESOURCE_HANDLE_CLASS(name) \
What do you think about simplifying this by making GrResourceHandle templated on
a const char* and then doing


#define GR_CREATE_RESOURCE_HANDLE_CLASS(name)
static const char k##name_tag[] = #name
using name = GrResourceKey<k##name_tag>

Also, I might prefer DEFINE to CREATE...

[egdaniel, 2016-05-06 18:35:54]

updated to use struct as template since we can't initialize a const char inside
of class.

[bsalomon, 2016-05-06 18:39:59]

lgtm w/ comment inline

[bsalomon, 2016-05-06 18:40:57]

https://codereview.chromium.org/1955893002/diff/80001/src/gpu/GrResourceHandle.h
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/80001/src/gpu/GrResourceHandl...
src/gpu/GrResourceHandle.h:14: // version of the class. No function should ever
take a generic GrResourceHandle and instead use
Maybe remove the last sentence? One of the benefits of the template approach is
that there no longer is a base class.

[egdaniel, 2016-05-06 18:46:55]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-05-06 18:46:56]

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/1955893002/#ps100001
(title: "review nit")

[commit-bot: I haz the power, 2016-05-06 18:47:02]

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

[commit-bot: I haz the power, 2016-05-06 18:53:32]

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

[commit-bot: I haz the power, 2016-05-06 18:53:33]

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

[robertphillips, 2016-05-09 13:13:11]

Does this deserve any sort of unit test ?

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:12: 
indexed version ?

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:31: 
type index ?

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
File src/gpu/glsl/GrGLSLProgramDataManager.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
src/gpu/glsl/GrGLSLProgramDataManager.h:49: // for nvpr only
We don't want a new VaryingHandle type here ?

[bsalomon, 2016-05-09 13:25:23]

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:12: 
On 2016/05/09 13:13:11, robertphillips wrote:
> indexed version ?

I agree that these comments are not totally clear.

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
File src/gpu/glsl/GrGLSLProgramDataManager.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
src/gpu/glsl/GrGLSLProgramDataManager.h:49: // for nvpr only
On 2016/05/09 13:13:11, robertphillips wrote:
> We don't want a new VaryingHandle type here ?

Good point

[egdaniel, 2016-05-09 17:19:54]

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:12: 
On 2016/05/09 13:25:23, bsalomon wrote:
> On 2016/05/09 13:13:11, robertphillips wrote:
> > indexed version ?
> 
> I agree that these comments are not totally clear.

Done.

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:31: 
On 2016/05/09 13:13:11, robertphillips wrote:
> type index ?

Done.

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
File src/gpu/glsl/GrGLSLProgramDataManager.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/glsl/GrGLSLPro...
src/gpu/glsl/GrGLSLProgramDataManager.h:49: // for nvpr only
On 2016/05/09 13:25:23, bsalomon wrote:
> On 2016/05/09 13:13:11, robertphillips wrote:
> > We don't want a new VaryingHandle type here ?
> 
> Good point

Done.

[bsalomon, 2016-05-09 17:26:51]

lgtm

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
File src/gpu/GrResourceHandle.h (right):

https://codereview.chromium.org/1955893002/diff/100001/src/gpu/GrResourceHand...
src/gpu/GrResourceHandle.h:12: 
On 2016/05/09 17:19:54, egdaniel wrote:
> On 2016/05/09 13:25:23, bsalomon wrote:
> > On 2016/05/09 13:13:11, robertphillips wrote:
> > > indexed version ?
> > 
> > I agree that these comments are not totally clear.
> 
> Done.

version->instantiation?

[egdaniel, 2016-05-09 17:41:43]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-05-09 17:41:44]

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/1955893002/#ps160001
(title: " ")

[commit-bot: I haz the power, 2016-05-09 17:41:54]

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

[commit-bot: I haz the power, 2016-05-09 18:03:54]

Description was changed from

==========
Allow gpu ResourceHandle class to be shared for multiple purposes

Currently this class was just used for resource handles when building up
a shader. However, I want to now use this class for things like objects
owned/held by the GrVkResourceProvider which are used by other classes.
An example of this will be for GrVkRenderTargets to hold a handle to a
collection of compatible render passes without having to own/hold onto
the render passes themselves.

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

to

==========
Allow gpu ResourceHandle class to be shared for multiple purposes

Currently this class was just used for resource handles when building up
a shader. However, I want to now use this class for things like objects
owned/held by the GrVkResourceProvider which are used by other classes.
An example of this will be for GrVkRenderTargets to hold a handle to a
collection of compatible render passes without having to own/hold onto
the render passes themselves.

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/+/167ab9198025e577efe033612d2e009633abb54b
==========

[commit-bot: I haz the power, 2016-05-09 18:03:56]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/167ab9198025e577efe033612d2e009633abb54b