Refactor how we store and use samplers in Ganesh

Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the samplers,
then when we get new textures, we just assume they come in in the same order as we
set the samplers on the descriptor sets. Thus no need to save direct vulkan info.

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

Committed: https://skia.googlesource.com/skia/+/45b61a1c4c0be896e7b12fd1405abfece799114f

Committed: https://skia.googlesource.com/skia/+/09aa1fce69b214714171db12c341aebd78dd29ea

[egdaniel, 2016-04-13 20:54:52]

Description was changed from

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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

to

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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

[egdaniel, 2016-04-13 20:57:57]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com, cdalton@nvidia.com, jvanverth@google.com

[egdaniel, 2016-04-13 20:57:58]

This is a first draft which doesn't include any vulkan changes yet (and probably
has many compile bugs, nit things, etc.). I just want to get initial feedback on
this overall design for samplers.

Chris I just want to make sure this doesn't mess up any of the sampler buffer
stuff you've been working on.

[Chris Dalton, 2016-04-13 21:56:40]

So does this mean Vulkan and GL want to use different sampler fuction names for
the same sampler types?

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler.h
File src/gpu/glsl/GrGLSLSampler.h (right):

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler....
src/gpu/glsl/GrGLSLSampler.h:29: virtual const char* getSamplerFunctionName()
const = 0;
This function is a bit problematic because there is nothing to return for
kSamplerBuffer. It appears to be used for doing a filtered, wrapped, mipmapped
sampling of a 2d image, which of course doesn't apply to a 1d texel array.

The functionality used to have a name more indicative of that, i.e.
GrGLSLGetTexture2DFunctionName. There it was easy enough to just assert on
GrSLTypeIs2DTextureType(). Maybe we could just change the name to make it clear
we are asking for the 2d texture sample name?

(Right now it is an error for kSamplerBuffer samplers to call
"appendTextureLookup" and they have to use "appendTexelFetch" instead.
appendTexelFetch can be used on any sampler.)

[egdaniel, 2016-04-13 23:44:45]

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler.h
File src/gpu/glsl/GrGLSLSampler.h (right):

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler....
src/gpu/glsl/GrGLSLSampler.h:29: virtual const char* getSamplerFunctionName()
const = 0;
On 2016/04/13 21:56:40, Chris Dalton wrote:
> This function is a bit problematic because there is nothing to return for
> kSamplerBuffer. It appears to be used for doing a filtered, wrapped, mipmapped
> sampling of a 2d image, which of course doesn't apply to a 1d texel array.
> 
> The functionality used to have a name more indicative of that, i.e.
> GrGLSLGetTexture2DFunctionName. There it was easy enough to just assert on
> GrSLTypeIs2DTextureType(). Maybe we could just change the name to make it
clear
> we are asking for the 2d texture sample name?
> 
> (Right now it is an error for kSamplerBuffer samplers to call
> "appendTextureLookup" and they have to use "appendTexelFetch" instead.
> appendTexelFetch can be used on any sampler.)

Okay so this function may just have been poorly named, mostly cause I wasn't
really sure what to call it. GrGLSLGetTexture2DFunctionName still exists. What
this functions returns for GL is the name of the variable for the sampler2D. In
vulkan, I may have a sampler2D variable or I could have a separate sampler and
texture. In the later case I need to "combine" the sampler and texture together
to make a sampler2D object. So the final functino call looks something like
"texture2D(sampler2D('samplerName', 'textureName'), ...);" This function would
return "sampler2D('samplerName', 'textureName')". Or in GL's case
"sampler2DName".

So maybe this should be something like getSampler2DName(), but this function
also works for sampler2DRect and such so I would prefer a name that was more
general, but still specific to what it's used for.

[egdaniel, 2016-04-13 23:51:43]

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler.h
File src/gpu/glsl/GrGLSLSampler.h (right):

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLSampler....
src/gpu/glsl/GrGLSLSampler.h:29: virtual const char* getSamplerFunctionName()
const = 0;
On 2016/04/13 23:44:45, egdaniel wrote:
> On 2016/04/13 21:56:40, Chris Dalton wrote:
> > This function is a bit problematic because there is nothing to return for
> > kSamplerBuffer. It appears to be used for doing a filtered, wrapped,
mipmapped
> > sampling of a 2d image, which of course doesn't apply to a 1d texel array.
> > 
> > The functionality used to have a name more indicative of that, i.e.
> > GrGLSLGetTexture2DFunctionName. There it was easy enough to just assert on
> > GrSLTypeIs2DTextureType(). Maybe we could just change the name to make it
> clear
> > we are asking for the 2d texture sample name?
> > 
> > (Right now it is an error for kSamplerBuffer samplers to call
> > "appendTextureLookup" and they have to use "appendTexelFetch" instead.
> > appendTexelFetch can be used on any sampler.)
> 
> Okay so this function may just have been poorly named, mostly cause I wasn't
> really sure what to call it. GrGLSLGetTexture2DFunctionName still exists. What
> this functions returns for GL is the name of the variable for the sampler2D.
In
> vulkan, I may have a sampler2D variable or I could have a separate sampler and
> texture. In the later case I need to "combine" the sampler and texture
together
> to make a sampler2D object. So the final functino call looks something like
> "texture2D(sampler2D('samplerName', 'textureName'), ...);" This function would
> return "sampler2D('samplerName', 'textureName')". Or in GL's case
> "sampler2DName".
> 
> So maybe this should be something like getSampler2DName(), but this function
> also works for sampler2DRect and such so I would prefer a name that was more
> general, but still specific to what it's used for.

Also I just noticed I didn't upload the change that had the appendTexelFetch in
it. I'll have to see how the Vulkan variables are used for buffers, but I'm
thinking I may need a similar function to this for texelFetch to get the sampler
name. In GL it will probably just call directly the function above (aka just
gets the normal variable name). For vulkan I'll probably have to do some branch
of if it is a normal sampler (call function above) or do whatever vulkan does
for buffer samplers.

[bsalomon, 2016-04-14 01:40:17]

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLUniformH...
File src/gpu/glsl/GrGLSLUniformHandler.h (right):

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLUniformH...
src/gpu/glsl/GrGLSLUniformHandler.h:57: virtual const GrGLSLSampler&
getSampler(SamplerHandle handle) const = 0;
It seems a bit unfortunate to make every processor jump through this function to
insert a texture lookup. From a FP writer's perspective it seems like just
another piece of odd boilerplate code to add to my implementation. Is there a
way around that? Could we pass the processors an opaque type that has the
GrGLSLSampler and the handle under the hood? Or have the builder do this lookup?
Or...

[egdaniel, 2016-04-14 03:11:50]

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLUniformH...
File src/gpu/glsl/GrGLSLUniformHandler.h (right):

https://codereview.chromium.org/1885863004/diff/1/src/gpu/glsl/GrGLSLUniformH...
src/gpu/glsl/GrGLSLUniformHandler.h:57: virtual const GrGLSLSampler&
getSampler(SamplerHandle handle) const = 0;
On 2016/04/14 01:40:16, bsalomon wrote:
> It seems a bit unfortunate to make every processor jump through this function
to
> insert a texture lookup. From a FP writer's perspective it seems like just
> another piece of odd boilerplate code to add to my implementation. Is there a
> way around that? Could we pass the processors an opaque type that has the
> GrGLSLSampler and the handle under the hood? Or have the builder do this
lookup?
> Or...

Yeah I think it would be fine for appendTexLookup to just take a SamplerHandle
instead of a GrGLSLSampler. We'll do the get the sampler inside of
appendTexLookup.

I hesitate making another opaque type to hold the handle/sampler under the hood.
That is basically what the current version of GrGLSLSampler is and to me it
added more confusion when trying to figure out what all the various sampler
objects did (GrGLSLSampler, Sampler Uniforms of Uniformhandler, the fSamplers
stored on programbuilder and program). I think its cleaner to just have the
GrGLSLSamplers and handles to them.

[egdaniel, 2016-04-14 14:58:53]

Okay so this has been updated so that processors only ever deal with
SamplerHandles.

I've added in the the texel stuff and renamed the name getters on GrGLSLSampler
to getSamplerNameForTexture2D and getSamplerNameForTexelFetch. Functionally
though these functions haven't changed.

[egdaniel, 2016-04-14 19:53:24]

Vulkan stuff added. For this change vulkan still is using a combined sampler and
texture. In a follow up CL I will break them apart.

Also added GL sampler file which I forgot to add to my previous git commits...

[bsalomon, 2016-04-19 14:42:21]

lgtm

[egdaniel, 2016-04-19 20:16:32]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-04-19 20:16:33]

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/1885863004/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-04-19 20:16:42]

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

[commit-bot: I haz the power, 2016-04-19 20:17:52]

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

[commit-bot: I haz the power, 2016-04-19 20:17:53]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[egdaniel, 2016-04-19 20:29:15]

The CQ bit was checked by egdaniel@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-19 20:29:21]

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

[commit-bot: I haz the power, 2016-04-19 20:44:48]

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

[commit-bot: I haz the power, 2016-04-19 20:44:49]

Dry run: This issue passed the CQ dry run.

[egdaniel, 2016-04-19 21:44:32]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-04-19 21:44:33]

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/1885863004/#ps120001
(title: "dtor")

[commit-bot: I haz the power, 2016-04-19 21:44:37]

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

[commit-bot: I haz the power, 2016-04-19 21:46:00]

Description was changed from

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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

to

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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/+/45b61a1c4c0be896e7b12fd1405abfece799114f
==========

[commit-bot: I haz the power, 2016-04-19 21:46:01]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/45b61a1c4c0be896e7b12fd1405abfece799114f

[egdaniel, 2016-04-19 22:24:14]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1896013003/ by egdaniel@google.com.

The reason for reverting is: breaking bots.

[egdaniel, 2016-04-20 13:40:02]

Description was changed from

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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/+/45b61a1c4c0be896e7b12fd1405abfece799114f
==========

to

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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/+/45b61a1c4c0be896e7b12fd1405abfece799114f
==========

[egdaniel, 2016-04-20 13:57:40]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-04-20 13:57: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/1885863004/#ps140001
(title: "Remove unneeded assert")

[commit-bot: I haz the power, 2016-04-20 13:57:58]

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

[commit-bot: I haz the power, 2016-04-20 14:09:49]

Description was changed from

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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/+/45b61a1c4c0be896e7b12fd1405abfece799114f
==========

to

==========
Refactor how we store and use samplers in Ganesh

The main goal of this refactorization is to allow Vulkan to use separate
sampler and texture objects in the shader and descriptor sets and combine
them into a sampler2d in the shader where needed.

A large part of this is separating how we store samplers and uniforms in the
UniformHandler. We no longer need to store handles to samplers besides when
we are initially emitting code. After we emit code all we ever do is loop over
all samplers and do some processor independent work on them, so we have no need
for direct access to individual samplers.

In the GLProgram all we ever do is set the sampler uniforms in the ctor and
never
touch them again, so no need to save sampler info there. The texture access on
program reuse just assume that they come in the same order as we set the texture
units for the samplers

For Vulkan, it is a similar story. We create the descriptor set layouts with the
samplers,
then when we get new textures, we just assume they come in in the same order as
we
set the samplers on the descriptor sets. Thus no need to save direct vulkan
info.

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/+/45b61a1c4c0be896e7b12fd1405abfece799114f

Committed:
https://skia.googlesource.com/skia/+/09aa1fce69b214714171db12c341aebd78dd29ea
==========

[commit-bot: I haz the power, 2016-04-20 14:09:50]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/09aa1fce69b214714171db12c341aebd78dd29ea