1097416 - WebGL2 - Feature checks

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[u480271]

Check for all required features and extensions to enable WebGL2 support.

Comment 1

[u480271]

Attachment: [WebGL2] enabled if all GLES 3 level features are available

Enable WebGL2 context support if all GLES3 features are available.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8524364 [details] [diff] [review]
[WebGL2] enabled if all GLES 3 level features are available

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

::: dom/canvas/WebGL2Context.cpp
@@ +63,5 @@
>      MOZ_ASSERT(IsWebGL2(), "WebGLContext is not a WebGL 2 context!");
>  
> +    const unsigned begin = unsigned(GLFeature::bind_buffer_offset);
> +    const unsigned end = unsigned(GLFeature::EnumMax);
> +    for (unsigned f = begin; f < end; f++) {

Put this logic in GLContext.

::: gfx/gl/GLContext.h
@@ +514,5 @@
>      bool IsSupported(GLFeature feature) const {
>          return mAvailableFeatures[size_t(feature)];
>      }
>  
> +    bool IsFeaturePartOfProfileVersion(GLFeature feature, 

ws

Comment 3

[u480271]

Attachment: [WebGL2] enabled if all GLES 3 level features are available

Updated with Jeff's review comments.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8527379 [details] [diff] [review]
[WebGL2] enabled if all GLES 3 level features are available

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

::: dom/canvas/WebGL2Context.cpp
@@ +62,5 @@
>  {
>      MOZ_ASSERT(IsWebGL2(), "WebGLContext is not a WebGL 2 context!");
>  
> +    if (!gl->AreAllFeaturesSupported(GLESVersion::ES3)) {
> +        GenerateWarning("Not all features availble to enable WebGL 2.");

This isn't actually true, since WebGL 2 support isn't exactly the same as ES3. I think doing it automagically will just cause pain. Sorry for not considering this previously.

We only have to do this in one place, so I don't think it should be a problem.

Comment 5

[u480271]

(In reply to Jeff Gilbert [:jgilbert] from comment #4)
> This isn't actually true, since WebGL 2 support isn't exactly the same as
> ES3. I think doing it automagically will just cause pain. Sorry for not
> considering this previously.

I don't understand. Do you think we'll have ES3 level features that aren't part of WebGL 2?  I can add a big list of features to check instead, but when I started looking it looked 1-to-1 to me.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Dan Glastonbury :djg :kamidphish from comment #5)
> (In reply to Jeff Gilbert [:jgilbert] from comment #4)
> > This isn't actually true, since WebGL 2 support isn't exactly the same as
> > ES3. I think doing it automagically will just cause pain. Sorry for not
> > considering this previously.
> 
> I don't understand. Do you think we'll have ES3 level features that aren't
> part of WebGL 2?  I can add a big list of features to check instead, but
> when I started looking it looked 1-to-1 to me.

DrawRangeElements might not make the final spec. MapBuffer almost certainly won't. It's much simpler and more explicit to just have a list.

Comment 7

[u480271]

Attachment: [WebGL2] enabled if all GLES 3 level features are available

Reverted auto-magic check and replaced with a long list of required features.

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8531794 [details] [diff] [review]
[WebGL2] enabled if all GLES 3 level features are available

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

::: dom/canvas/WebGL2Context.cpp
@@ +48,5 @@
>  // WebGL 2 initialisation
>  
> +// These WebGL 1 extensions are natively supported by WebGL 2.
> +static const WebGLExtensionID
> +nativelySupportedExtensions[] = {

kConstant, for static consts.

Also, variables don't need a linebreak between the type and identifier name, even in the base scope.

@@ +122,4 @@
>          return false;
>      }
>  
> +    for (size_t i = 0; i < size_t(MOZ_ARRAY_LENGTH(requiredFeatures)); i++) {

ArrayLength() from mfbt/ArrayUtils.h returns a size_t.

Comment 9

[u480271]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3cf43086fae

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e3cf43086fae