Closed Bug 1096633 Opened 5 years ago Closed 5 years ago

Allow webgl/experimental-webgl aliasing

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jgilbert, Assigned: jgilbert)

Details

Attachments

(3 files, 2 obsolete files)

In a spec revision, we added this to the WebGL 1 spec in $2.1 "Context Creation":

If the user agent supports both the webgl and experimental-webgl canvas context types, they shall be treated as aliases. For example, if a call to getContext('webgl') successfully creates a WebGLRenderingContext, a subsequent call to getContext('experimental-webgl') shall return the same context object.
Attachment #8520246 - Flags: review?(dglastonbury)
Attachment #8520246 - Flags: review?(dglastonbury) → review+
Updated the test.
Attachment #8520366 - Flags: review?(dglastonbury)
Attachment #8520366 - Flags: review?(dglastonbury) → review+
Let's do this right. I also simplified some things. (Sorry!)
Attachment #8520246 - Attachment is obsolete: true
Attachment #8523264 - Flags: review?(dglastonbury)
B2G hates fun. Also, moz enum class nested in another class.
Attachment #8523264 - Attachment is obsolete: true
Attachment #8523264 - Flags: review?(dglastonbury)
Attachment #8523341 - Flags: review?(dglastonbury)
Comment on attachment 8523341 [details] [diff] [review]
0001-Allow-webgl-experimental-webgl-aliasing.patch

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

Maybe a potential telemetry reporting nit, but otherwise LGTM.

::: dom/html/HTMLCanvasElement.cpp
@@ +702,5 @@
> +      return nullptr;
> +    break;
> +
> +  case CanvasContextType::WebGL2:
> +    Telemetry::Accumulate(Telemetry::CANVAS_WEBGL_USED, 1);

Should that be 2 instead of 1?
Attachment #8523341 - Flags: review?(dglastonbury) → review+
(In reply to Dan Glastonbury :djg :kamidphish from comment #7)
> Comment on attachment 8523341 [details] [diff] [review]
> 0001-Allow-webgl-experimental-webgl-aliasing.patch
> 
> Review of attachment 8523341 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Maybe a potential telemetry reporting nit, but otherwise LGTM.
> 
> ::: dom/html/HTMLCanvasElement.cpp
> @@ +702,5 @@
> > +      return nullptr;
> > +    break;
> > +
> > +  case CanvasContextType::WebGL2:
> > +    Telemetry::Accumulate(Telemetry::CANVAS_WEBGL_USED, 1);
> 
> Should that be 2 instead of 1?

I am not sure. I copied the existing behavior. If we want to do this, we should do it in a different bug.
I accidentally pushed this in two parts. There might be mochitest failures in the builds of just the change without the test update.

The change:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/7025a18fc63b

The test:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/f85d4a0211e9
This patch fixes adds missing namespacing tags.
Depends on: 1101623
No longer depends on: 1101623
You need to log in before you can comment on or make changes to this bug.