Closed Bug 902488 Opened 7 years ago Closed 7 years ago

WebGL2 Occlusion queries optimization on desktop

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: guillaume.abadie, Assigned: guillaume.abadie)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Summary: WebGL2 Improve occlusion queries → WebGL2 Occlusion queries optimization on desktop
Attached patch patch revision 1 (obsolete) — Splinter Review
Attachment #786967 - Flags: review?(jgilbert)
Attached patch patch revision 2Splinter Review
Attachment #786967 - Attachment is obsolete: true
Attachment #786967 - Flags: review?(jgilbert)
Attachment #787005 - Flags: review?(jgilbert)
Blocks: 903480
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`!
Attachment #787005 - Flags: review?(jgilbert) → review+
(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...
https://hg.mozilla.org/integration/mozilla-inbound/rev/59c344ec4caf
Target Milestone: --- → mozilla26
https://hg.mozilla.org/mozilla-central/rev/59c344ec4caf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.