902488 - WebGL2 Occlusion queries optimization on desktop

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Comment 1

[Guillaume Abadie]

Attachment: patch revision 1

Comment 2

[Guillaume Abadie]

Attachment: patch revision 2

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 787005 [details] [diff] [review]
patch revision 2

Review of attachment 787005 [details] [diff] [review]:
-----------------------------------------------------------------

Nits.

::: content/canvas/src/WebGLContextAsyncQueries.cpp
@@ +42,5 @@
> +               target == LOCAL_GL_ANY_SAMPLES_PASSED_CONSERVATIVE,
> +               "unknown occlusion query target");
> +
> +    if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query_boolean))
> +    {

Brace to end of the previous line.

If this seems too cramped, use:
if (foo) {
  return bar;

} else if (qux) {
  return bax;
}

@@ +45,5 @@
> +    if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query_boolean))
> +    {
> +        return target;
> +    }
> +    else if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query2))

`} else`!

Comment 4

[Guillaume Abadie]

(In reply to Jeff Gilbert [:jgilbert] from comment #3)
> Comment on attachment 787005 [details] [diff] [review]
> patch revision 2
> 
> Review of attachment 787005 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nits.
> 
> ::: content/canvas/src/WebGLContextAsyncQueries.cpp
> @@ +42,5 @@
> > +               target == LOCAL_GL_ANY_SAMPLES_PASSED_CONSERVATIVE,
> > +               "unknown occlusion query target");
> > +
> > +    if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query_boolean))
> > +    {
> 
> Brace to end of the previous line.
Oups...
> 
> If this seems too cramped, use:
> if (foo) {
>   return bar;
> 
> } else if (qux) {
>   return bax;
> }
> 
> @@ +45,5 @@
> > +    if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query_boolean))
> > +    {
> > +        return target;
> > +    }
> > +    else if (gl->IsExtensionSupported(gl::GLContext::XXX_occlusion_query2))
> 
> `} else`!
Oups...

Comment 5

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59c344ec4caf

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59c344ec4caf