Implement ClearBuffer family of functions for WebGL2.

Comment on attachment 8467498 [details] [diff] [review] WebGL2 - Implement ClearBufferXXX APIs Review of attachment 8467498 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGL2ContextMRTs.cpp @@ +24,5 @@ > + case LOCAL_GL_STENCIL: > + return 1; > + > + default: > + return 0; MOZ_CRASH @@ +95,5 @@ > + ClearBufferuiv_base(buffer, drawbuffer, value.Elements()); > +} > + > +void > +WebGL2Context::ClearBufferfv_base(GLenum buffer, GLint drawbuffer, const GLfloat* value) I'd rather you stick these _base functions next to eachother. ::: gfx/gl/GLContext.h @@ +879,5 @@ > AfterGLDrawCall(); > } > > + void fClearBufferfi(GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil) > + { Inlined member funcs should keep the { on the same line, unless decl is split across multiple lines. ::: gfx/gl/GLContextSymbols.h @@ +66,5 @@ > typedef void (GLAPIENTRY * PFNGLCLEARPROC) (GLbitfield); > PFNGLCLEARPROC fClear; > + typedef void (GLAPIENTRY * PFNGLCLEARBUFFERFIPROC) (GLenum buffer, GLint drawbuffer, GLfloat depth, GLint stencil); > + PFNGLCLEARBUFFERFIPROC fClearBufferfi; > + typedef void (GLAPIENTRY * PFNGLCLEARBUFFERFVPROC) (GLenum buffer, GLint drawbuffer, const GLfloat *value); Star to left, even if the GL headers have it to the right.

Comment on attachment 8467498 [details] [diff] [review] WebGL2 - Implement ClearBufferXXX APIs Review of attachment 8467498 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLContextFeatures.cpp @@ +74,5 @@ > { > + "clear_buffers", > + 300, // OpenGL version > + 300, // OpenGL ES version > + { You forgot `GLContext::Extension_None,` here.

(In reply to Jeff Gilbert [:jgilbert] from comment #3) > Comment on attachment 8467498 [details] [diff] [review] > WebGL2 - Implement ClearBufferXXX APIs > > Review of attachment 8467498 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/gl/GLContextFeatures.cpp > @@ +74,5 @@ > > { > > + "clear_buffers", > > + 300, // OpenGL version > > + 300, // OpenGL ES version > > + { > > You forgot `GLContext::Extension_None,` here. Strange. It's there in the change on my local disk.

(In reply to Jeff Gilbert [:jgilbert] from comment #2) > Comment on attachment 8467498 [details] [diff] [review] > WebGL2 - Implement ClearBufferXXX APIs > > Review of attachment 8467498 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/canvas/WebGL2ContextMRTs.cpp > @@ +24,5 @@ > MOZ_CRASH Scary, but done. ;-) > @@ +95,5 @@ > I'd rather you stick these _base functions next to eachother. Done. > ::: gfx/gl/GLContext.h > Inlined member funcs should keep the { on the same line, unless decl is > split across multiple lines. Done. > ::: gfx/gl/GLContextSymbols.h > Star to left, even if the GL headers have it to the right. Done.

Comment on attachment 8469720 [details] [diff] [review] Review Comments 1 Review of attachment 8469720 [details] [diff] [review]: ----------------------------------------------------------------- Cool, not a bad way to handle minor review updates.

https://hg.mozilla.org/users/dglastonbury_mozilla.com/mozilla-central/rev/3fdf50d41975 https://hg.mozilla.org/users/dglastonbury_mozilla.com/mozilla-central/rev/c1b4ad28ad33

https://hg.mozilla.org/users/dglastonbury_mozilla.com/mozilla-central/rev/fd773ecc59cf https://hg.mozilla.org/users/dglastonbury_mozilla.com/mozilla-central/rev/1577ccf078df

I looked at this patch and the spec, and it's almost good to land but I found some issues which are fixed by this patch. - don't MOZ_CRASH on bad inputs - validate the buffer parameter also in clearBufferfi - validate the GLint drawbuffer parameter - share identical validation code between different entry points - get a bit closer to Gecko coding style in switch statement even if we're 4-space indenting

Comment on attachment 8491624 [details] [diff] [review] clearbuffer-fixes Review of attachment 8491624 [details] [diff] [review]: ----------------------------------------------------------------- I like it.

Dan, should we rename this bug to "implement clearbuffer" and close it?