Add ability to add general layout qualifiers GrGLSLShaderVar

Add ability to add general layout qualifiers GrGLSLShaderVar

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

Committed: https://skia.googlesource.com/skia/+/ae47418936933c9dd7f4be93fab7c3a42d7a7bd9

[egdaniel, 2016-01-20 21:33:34]

Description was changed from

==========
Add ability to add general layout qualifiers GrGLSLShaderVar

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

to

==========
Add ability to add general layout qualifiers GrGLSLShaderVar

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

[egdaniel, 2016-01-20 21:36:23]

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

[egdaniel, 2016-01-20 21:36:24]

This makes it simpler to add layout qualifiers when create shader vars. Need
this flexibility in vk. The current origin object was simply used in deciding to
add a layout qualifier which seemed more complex than needed.

[bsalomon, 2016-01-20 21:40:03]

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

https://codereview.chromium.org/1610853002/diff/1/src/gpu/glsl/GrGLSLShaderVa...
src/gpu/glsl/GrGLSLShaderVar.h:227: const char*     fLayoutQualifier;
It feels a bit dangerous to remember a char* unless we have some ability to
ensure that it is a literal.

[egdaniel, 2016-01-21 15:39:04]

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

https://codereview.chromium.org/1610853002/diff/1/src/gpu/glsl/GrGLSLShaderVa...
src/gpu/glsl/GrGLSLShaderVar.h:227: const char*     fLayoutQualifier;
On 2016/01/20 21:40:03, bsalomon wrote:
> It feels a bit dangerous to remember a char* unless we have some ability to
> ensure that it is a literal.

switched to SkString

[egdaniel, 2016-01-21 17:45:49]

ping

[joshualitt, 2016-01-21 18:37:08]

On 2016/01/21 17:45:49, egdaniel wrote:
> ping

will textures not have origins in Vulkan?  How will we handle the general case
of "layout(qualifier1​, qualifier2​ = value, ...) variable definition"?  seems
like this model always assumes we always add qualifiers all in one place, which
might be a fine assumption, but I figured I'd ask the question.

[egdaniel, 2016-01-21 18:39:50]

On 2016/01/21 18:37:08, joshualitt wrote:
> On 2016/01/21 17:45:49, egdaniel wrote:
> > ping
> 
> will textures not have origins in Vulkan?  How will we handle the general case
> of "layout(qualifier1​, qualifier2​ = value, ...) variable definition"?  seems
> like this model always assumes we always add qualifiers all in one place,
which
> might be a fine assumption, but I figured I'd ask the question.

Well we weren't using the origin for any real origin related thing outside of
the one layout, so until we need it no reason for it to be there.

You are right about the current assumption that we only set a layout once on a
variable. In our current world this is true. However, in the future if we end up
wanting to add general layouts it is not hard to adapt this to append an
arbitrary number of layout qualifiers.

[joshualitt, 2016-01-21 18:40:20]

On 2016/01/21 18:39:50, egdaniel wrote:
> On 2016/01/21 18:37:08, joshualitt wrote:
> > On 2016/01/21 17:45:49, egdaniel wrote:
> > > ping
> > 
> > will textures not have origins in Vulkan?  How will we handle the general
case
> > of "layout(qualifier1​, qualifier2​ = value, ...) variable definition"? 
seems
> > like this model always assumes we always add qualifiers all in one place,
> which
> > might be a fine assumption, but I figured I'd ask the question.
> 
> Well we weren't using the origin for any real origin related thing outside of
> the one layout, so until we need it no reason for it to be there.
> 
> You are right about the current assumption that we only set a layout once on a
> variable. In our current world this is true. However, in the future if we end
up
> wanting to add general layouts it is not hard to adapt this to append an
> arbitrary number of layout qualifiers.

lgtm

[bsalomon, 2016-01-21 18:41:38]

On 2016/01/21 18:40:20, joshualitt wrote:
> On 2016/01/21 18:39:50, egdaniel wrote:
> > On 2016/01/21 18:37:08, joshualitt wrote:
> > > On 2016/01/21 17:45:49, egdaniel wrote:
> > > > ping
> > > 
> > > will textures not have origins in Vulkan?  How will we handle the general
> case
> > > of "layout(qualifier1​, qualifier2​ = value, ...) variable definition"? 
> seems
> > > like this model always assumes we always add qualifiers all in one place,
> > which
> > > might be a fine assumption, but I figured I'd ask the question.
> > 
> > Well we weren't using the origin for any real origin related thing outside
of
> > the one layout, so until we need it no reason for it to be there.
> > 
> > You are right about the current assumption that we only set a layout once on
a
> > variable. In our current world this is true. However, in the future if we
end
> up
> > wanting to add general layouts it is not hard to adapt this to append an
> > arbitrary number of layout qualifiers.
> 
> lgtm

Can you remove the unneeded inits to nullptr?

[egdaniel, 2016-01-21 18:57:11]

On 2016/01/21 18:41:38, bsalomon wrote:
> On 2016/01/21 18:40:20, joshualitt wrote:
> > On 2016/01/21 18:39:50, egdaniel wrote:
> > > On 2016/01/21 18:37:08, joshualitt wrote:
> > > > On 2016/01/21 17:45:49, egdaniel wrote:
> > > > > ping
> > > > 
> > > > will textures not have origins in Vulkan?  How will we handle the
general
> > case
> > > > of "layout(qualifier1​, qualifier2​ = value, ...) variable definition"? 
> > seems
> > > > like this model always assumes we always add qualifiers all in one
place,
> > > which
> > > > might be a fine assumption, but I figured I'd ask the question.
> > > 
> > > Well we weren't using the origin for any real origin related thing outside
> of
> > > the one layout, so until we need it no reason for it to be there.
> > > 
> > > You are right about the current assumption that we only set a layout once
on
> a
> > > variable. In our current world this is true. However, in the future if we
> end
> > up
> > > wanting to add general layouts it is not hard to adapt this to append an
> > > arbitrary number of layout qualifiers.
> > 
> > lgtm
> 
> Can you remove the unneeded inits to nullptr?

done. ptal

[bsalomon, 2016-01-21 19:02:57]

lgtm

[egdaniel, 2016-01-21 19:03:14]

The CQ bit was checked by egdaniel@google.com

[egdaniel, 2016-01-21 19:03:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1610853002/#ps40001
(title: "review update")

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

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

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

Description was changed from

==========
Add ability to add general layout qualifiers GrGLSLShaderVar

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

to

==========
Add ability to add general layout qualifiers GrGLSLShaderVar

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/+/ae47418936933c9dd7f4be93fab7c3a42d7a7bd9
==========

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

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