code review 137630043: go.mobile/gl: f32 utilities and extra debugging

go.mobile/gl: f32 utilities and extra debugging

Split constants out into their own file. Hide the
values in named types in a struct, so that when
using the gldebug tag we can attach extra
information (such as the name of a uniform and
attribute).

[crawshaw, 2014-09-13 20:05:07]

Hello golang-codereviews@googlegroups.com (cc: alsodave, bryanturley, nigeltao),

I'd like you to review this change to
https://code.google.com/p/go.mobile

[bryanturley, 2014-09-13 21:10:21]

Are you aware of 

http://www.khronos.org/registry/gles/extensions/EXT/EXT_debug_label.txt

Which appears to not be available everywhere.

Also the less related but more available
http://www.khronos.org/registry/gles/extensions/EXT/EXT_debug_marker.txt

I assume they spit out to logcat and something similar to the missing
ARB_debug_output is always forced on.

https://codereview.appspot.com/137630043/diff/60001/gl/gldebug.go
File gl/gldebug.go (right):

https://codereview.appspot.com/137630043/diff/60001/gl/gldebug.go#newcode67
gl/gldebug.go:67: switch v {
I do a map[uint32] string for this, forgot to mention it previously.
I also provide a map[string] uint32 but have no idea what I would use it for...

[bryanturley, 2014-09-13 21:26:26]

I think this could get very confusing, having structs that change definition.
It will also not work with uniform blocks.

UBOs are supported in gles2.0 with extensions
  GL_NV_uniform_buffer_object
  GL_IMG_uniform_buffer_object
  (so practically everyone that isn't running Mali)
Which I cannot find documentation for at 
http://www.khronos.org/registry/gles/ oddly...

They are in vanilla gles >= 3.0

[crawshaw, 2014-09-13 21:38:54]

I am unclear on how uniform blocks will cause a problem. There's still
a handle for the uniform, which would be represented by gl.Uniform.

I realize there a much better diagnostic tools, but gldebug is meant
to be an easy-to-setup tool.

On Sat, Sep 13, 2014 at 5:26 PM,  <bryanturley@gmail.com> wrote:
> I think this could get very confusing, having structs that change
> definition.
> It will also not work with uniform blocks.
>
> UBOs are supported in gles2.0 with extensions
>   GL_NV_uniform_buffer_object
>   GL_IMG_uniform_buffer_object
>   (so practically everyone that isn't running Mali)
> Which I cannot find documentation for at
> http://www.khronos.org/registry/gles/ oddly...
>
> They are in vanilla gles >= 3.0
>
>
>
>
> https://codereview.appspot.com/137630043/

[bryanturley, 2014-09-13 22:29:36]

On 2014/09/13 21:38:54, crawshaw wrote:
> I am unclear on how uniform blocks will cause a problem. There's still
> a handle for the uniform, which would be represented by gl.Uniform.

You are correct I haven't thought about stand alone uniforms for a while.
I am so used to uniform blocks now.

In your spinnable gopher example you have 

var (
  ...
  lightDirection gl.Uniform
  lightAmbientColor gl.Uniform
  lightDiffuseColor gl.Uniform
  materialAmbientFactor gl.Uniform
  materialDiffuseFactor gl.Uniform
  materialShininess gl.Uniform
  model gl.Uniform
  view gl.Uniform
  projection gl.Uniform
  ...
) 
And you send each of them up with one call to gl.UniformX().

With a uniform block and uniform buffer object you could.

type MyUniforms struct {
  LightDirection Vec4
  LightAmbientColor Vec4 // Col4?
  ... etc ..
  View Mat4
  Projection Mat4
}

Then you upload the entire (carefully aligned) struct with one gl.BufferData()
to the uniform buffer object.
One call, one copy.
In opengl 4.4 (guessing also gles3.1 + AEP) you can keep the uniform buffer
object mmap()'ed indefinitely. (gl.MapBuffer() with GL_MAP_PERSISTANT)
At this point you can write directly into the drivers storage for the uniform,
which may in fact be actual video memory.
Now you can update all of your uniforms with zero opengl calls every frame :)
You just have to use gl.FenceSync() and gl.ClientWaitSync().

Part of what I am trying to do with reflectgraph is to make that whole process
as safe as normal go.
You can use that pattern for all of the buffer types (vertex arrays, element
arrays, pixel (un)pack buffers, etc...)

> 
> I realize there a much better diagnostic tools, but gldebug is meant
> to be an easy-to-setup tool.
> 

Yeah I agree.

> On Sat, Sep 13, 2014 at 5:26 PM,  <mailto:bryanturley@gmail.com> wrote:
> > I think this could get very confusing, having structs that change
> > definition.
> > It will also not work with uniform blocks.
> >
> > UBOs are supported in gles2.0 with extensions
> >   GL_NV_uniform_buffer_object
> >   GL_IMG_uniform_buffer_object
> >   (so practically everyone that isn't running Mali)
> > Which I cannot find documentation for at
> > http://www.khronos.org/registry/gles/ oddly...
> >
> > They are in vanilla gles >= 3.0
> >
> >
> >
> >
> > https://codereview.appspot.com/137630043/

[bryanturley, 2014-09-13 22:41:31]

On 2014/09/13 22:29:36, bryanturley wrote:
> On 2014/09/13 21:38:54, crawshaw wrote:
> > I am unclear on how uniform blocks will cause a problem. There's still
> > a handle for the uniform, which would be represented by gl.Uniform.
> 
> You are correct I haven't thought about stand alone uniforms for a while.
> I am so used to uniform blocks now.
> 
> In your spinnable gopher example you have 
> 
> var (
>   ...
>   lightDirection gl.Uniform
>   lightAmbientColor gl.Uniform
>   lightDiffuseColor gl.Uniform
>   materialAmbientFactor gl.Uniform
>   materialDiffuseFactor gl.Uniform
>   materialShininess gl.Uniform
>   model gl.Uniform
>   view gl.Uniform
>   projection gl.Uniform
>   ...
> ) 
> And you send each of them up with one call to gl.UniformX().
> 
> With a uniform block and uniform buffer object you could.
> 
> type MyUniforms struct {
>   LightDirection Vec4
>   LightAmbientColor Vec4 // Col4?
>   ... etc ..
>   View Mat4
>   Projection Mat4
> }
> 
> Then you upload the entire (carefully aligned) struct with one gl.BufferData()
> to the uniform buffer object.
> One call, one copy.
> In opengl 4.4 (guessing also gles3.1 + AEP) you can keep the uniform buffer
> object mmap()'ed indefinitely. (gl.MapBuffer() with GL_MAP_PERSISTANT)
> At this point you can write directly into the drivers storage for the uniform,
> which may in fact be actual video memory.
> Now you can update all of your uniforms with zero opengl calls every frame :)

Also from multiple threads and/or goroutines since it is just memory.

> You just have to use gl.FenceSync() and gl.ClientWaitSync().
> 
> Part of what I am trying to do with reflectgraph is to make that whole process
> as safe as normal go.
> You can use that pattern for all of the buffer types (vertex arrays, element
> arrays, pixel (un)pack buffers, etc...)
> 
> > 
> > I realize there a much better diagnostic tools, but gldebug is meant
> > to be an easy-to-setup tool.
> > 
> 
> Yeah I agree.
> 
> > On Sat, Sep 13, 2014 at 5:26 PM,  <mailto:bryanturley@gmail.com> wrote:
> > > I think this could get very confusing, having structs that change
> > > definition.
> > > It will also not work with uniform blocks.
> > >
> > > UBOs are supported in gles2.0 with extensions
> > >   GL_NV_uniform_buffer_object
> > >   GL_IMG_uniform_buffer_object
> > >   (so practically everyone that isn't running Mali)
> > > Which I cannot find documentation for at
> > > http://www.khronos.org/registry/gles/ oddly...
> > >
> > > They are in vanilla gles >= 3.0
> > >
> > >
> > >
> > >
> > > https://codereview.appspot.com/137630043/

[nigeltao, 2014-09-14 23:45:16]

LGTM.

https://codereview.appspot.com/137630043/diff/60001/gl/types.go
File gl/types.go (right):

https://codereview.appspot.com/137630043/diff/60001/gl/types.go#newcode29
gl/types.go:29: Value uint
uint32?

[crawshaw, 2014-09-17 22:08:07]

*** Submitted as
https://code.google.com/p/go/source/detail?r=547bbce45af3&repo=mobile ***

go.mobile/gl: f32 utilities and extra debugging

Split constants out into their own file. Hide the
values in named types in a struct, so that when
using the gldebug tag we can attach extra
information (such as the name of a uniform and
attribute).

LGTM=nigeltao
R=golang-codereviews, bryanturley, nigeltao
CC=davidday, golang-codereviews
https://codereview.appspot.com/137630043