917046 - WebGL2 requires instanced drawing with only non-zero-divisor active attrib arrays to be allowed

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kelsey Gilbert [:jgilbert]]

ANGLE_instanced_arrays and NV_instanced_arrays disallow instanced drawing when there is not at least one active attrib array with a zero instancing divisor.

GLES3 (and thus WebGL2) does not appear to have this restriction.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 1: Add a `instanced_arrays2` feature to encapsulate this functionality.

Add a `instanced_arrays2` feature to encapsulate this functionality.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: patch 2: Require `instanced_arrays2` for WebGL2.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 805664 [details] [diff] [review]
patch 1: 
Add a `instanced_arrays2` feature to encapsulate this functionality.

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

::: gfx/gl/GLContextFeatures.cpp
@@ +158,5 @@
>              GLContext::Extensions_End
>          }
>      },
>      {
> +        "instanced_arrays2",

Have you thought of maybe giving this a more descriptive name?

@@ +169,5 @@
> +        /* This is an expanded version of `instanced_arrays` that allows for all
> +         * enabled active attrib arrays to have non-zero divisors.
> +         * ANGLE_instanced_arrays and NV_instanced_arrays forbid this, but GLES3
> +         * has no such restriction.
> +         */

Comments usually go _above_ what they refer to.

Comment 4

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #3)
> Comment on attachment 805664 [details] [diff] [review]
> patch 1: 
> Add a `instanced_arrays2` feature to encapsulate this functionality.
> 
> Review of attachment 805664 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/gl/GLContextFeatures.cpp
> @@ +158,5 @@
> >              GLContext::Extensions_End
> >          }
> >      },
> >      {
> > +        "instanced_arrays2",
> 
> Have you thought of maybe giving this a more descriptive name?
"instanced_non_arrays"? :P
Actually, that's as good a name as any.
> 
> @@ +169,5 @@
> > +        /* This is an expanded version of `instanced_arrays` that allows for all
> > +         * enabled active attrib arrays to have non-zero divisors.
> > +         * ANGLE_instanced_arrays and NV_instanced_arrays forbid this, but GLES3
> > +         * has no such restriction.
> > +         */
> 
> Comments usually go _above_ what they refer to.
I agree, but we should change this all at once for this file.

Comment 5

[Kelsey Gilbert [:jgilbert]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7ca634037700
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ae79bc0418e1

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7ca634037700
https://hg.mozilla.org/mozilla-central/rev/ae79bc0418e1