search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.08k stars 1.56k forks source link

CanvasElement for WebGL2 RenderingContext2.canvas.canvas is null but should be the same as the CanvasElement it was created from #48469

Open denniskaselow opened 2 years ago

denniskaselow commented 2 years ago

The CanvasElement associated with a RenderingContext2 when using webgl2 is missing:

final actualCanvas = querySelector('canvas')! as CanvasElement;
final gl = actualCanvas.getContext('webgl2')! as RenderingContext2

// prints canvas as in an instance of Canvas
print(gl.canvas);
// prints null but should be the same instance of CanvasElement as the variable actualCanvas 
print(gl.canvas?.canvas);

// Furthermore, its declared as non nullable, so it should not be null
@Native("WebGLCanvas")
class Canvas extends JavaScriptObject {
  ...
  @JSName('canvas')
  CanvasElement get canvas native;
  ...
}

The CanvasElement is missing with ddc and dart2js.

Dart SDK version: 2.17.0-132.0.dev (dev) (Sun Feb 20 15:40:08 2022 -0800) on "windows_x64"

Markzipan commented 2 years ago

The current behavior is expected according to MDN, so it's possible the Dart side needs to be updated. Adding @srujzs for more info here.

denniskaselow commented 2 years ago

Yes you are right that it should be nullable, in case the rendering context was created by an OffscreenCanvas, but I created the RenderingContext2 from an existing CanvasElement and as I understand the MDN article, if the RenderingContext2 was created from an existing CanvasElement then that one should be returned.

It might be null if it is not associated with a <canvas> element or an OffscreenCanvas object.

EDIT: It may not be nullable at the moment, because no browser supports OffscreenCanvas, so if an associated canvas exists and thus gl.canvas is not null, then it must have been created from a CanvasElement-object and gl.canvas.canvas must be that object.

srujzs commented 2 years ago

It's unclear what can be done on the dart:html side here. These methods route to their native definitions (querySelector, getContext, and both definitions of canvas), so if there's unexpected behavior, it's likely coming from the browser afaict. Do you have a complete repro we can try out?

If canvas is returning null in a legitimate usage, though, I think this warrants a breaking change in dart:html to make the getter nullable. We may need to version this to avoid excessive breaks - not sure.

denniskaselow commented 2 years ago

Okay, I tried to narrow down whats happening and it's a bit more weird than I thought.

index.html

<!DOCTYPE html>

<script defer src="main.dart.js"></script>

<canvas id='webgl1'></canvas>
<canvas id='webgl2'></canvas>

<script>
  let canvas1 = document.querySelector('#webgl1');
  let webgl1ctx = canvas1.getContext('webgl');
  console.log(`Javascript webgl1ctx.canvas = ${webgl1ctx.canvas}`); // CanvasElement

  let canvas2 = document.querySelector('#webgl2');
  let webgl2ctx = canvas2.getContext('webgl2');
  console.log(`Javascript webgl2ctx.canvas = ${webgl2ctx.canvas}`); // CanvasElement
</script>

main.dart

import 'dart:html';

import 'dart:web_gl';

void main() {
  final canvas1 = querySelector('#webgl1') as CanvasElement;
  final canvas2 = querySelector('#webgl2') as CanvasElement;

  final webgl1ctx = canvas1.getContext('webgl') as RenderingContext;
  final webgl2ctx = canvas2.getContext('webgl2') as RenderingContext2;

  print('Dart webgl1ctx.canvas = ${webgl1ctx.canvas.runtimeType}'); // CanvasElement

  print('Dart webgl2ctx.canvas = ${webgl2ctx.canvas?.runtimeType}'); // signature says Canvas, but runtimeType is CanvasElement
  print('Dart webgl2ctx.canvas.canvas = ${webgl2ctx.canvas?.canvas}'); // null
  print('Dart webgl2ctx.canvas.offscreenCanvas = ${webgl2ctx.canvas?.offscreenCanvas}'); // null
}

The only strange result is webgl2ctx.canvas. The method signature in Dart does not return a CanvasElement but a Canvas?: https://github.com/dart-lang/sdk/blob/6f4f2f5f129500a3671a4af78463b5b34df3c478/sdk/lib/web_gl/dart2js/web_gl_dart2js.dart#L1367

This is the Canvas class (which is not a superclass of CanvasElement): https://github.com/dart-lang/sdk/blob/6f4f2f5f129500a3671a4af78463b5b34df3c478/sdk/lib/web_gl/dart2js/web_gl_dart2js.dart#L81-L93

That's why I tried to access the canvas-getter of that class, but the runtimeType when translated to javascript is CanvasElement, not Canvas. Casting webgl2ctx.canvas to CanvasElement actually works and is what I want to access (but will probably break at some point in the future once it returns what the signature says).

  1. Why does the signature of RenderingContext2.canvas say Canvas but actually is a CanvasElememt?
  2. RenderingContext.canvas still returns CanvasElement. Will that change at some point in the future to be the same as for RenderingContext2 to include OffscreenCanvas via that intermediate Canvas-class?
  3. Just discovered OffscreenCanvasRenderingContext2D.canvas which returns OffscreenCanvas so the canvas-getter for CanvasRenderingContext2D will probably remain as is.
  4. Once that intermediate Canvas class works as advertised and OffscreenCanvas is supported by browser would CanvasElement need to be CanvasElement? as far as I can tell.
srujzs commented 2 years ago

Thanks for the repro! Sorry for the late response.

Why does the signature of RenderingContext2.canvas say Canvas but actually is a CanvasElememt?

I suspect this may be a relic. Canvas corresponds to the native type WebGLCanvas, which from what I can tell, no longer exists. This should shift to CanvasElement.

RenderingContext.canvas still returns CanvasElement. Will that change at some point in the future to be the same as for RenderingContext2 to include OffscreenCanvas via that intermediate Canvas-class?

Just discovered OffscreenCanvasRenderingContext2D.canvas which returns OffscreenCanvas so the canvas-getter for CanvasRenderingContext2D will probably remain as is.

https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/canvas https://caniuse.com/mdn-api_offscreencanvasrenderingcontext2d

It isn't clear to me in what context I should expect OffscreenCanvasRenderingContext2D to occur. Is it simply when getContext is called on an OffscreenCanvas object? In that case, addressing your previous question, wouldn't I expect RenderingContext and RenderingContext2 to only return CanvasElements? If they returned an OffscreenCanvas, I would expect them to be OffscreenCanvasRenderingContexts instead. The MDN page does say it can return an OffscreenCanvas only for Firefox in a very specific case, but supporting that might be quite complicated.

Once that intermediate Canvas class works as advertised and OffscreenCanvas is supported by browser would CanvasElement need to be CanvasElement? as far as I can tell.

I still think we should get rid of Canvas in favor of CanvasElement, but yes, if it can be nullable, it should be written as such. When would the condition "It might be null if it is not associated with a canvas element or an OffscreenCanvas object." hold true?

It seems like the only major change here is moving RenderingContext.canvas and RenderingContext2.canvas to CanvasElement? types. It's a breaking change, so I'm not sure what we might do here in the near future. To unblock yourself, you can use workarounds using interop (@staticInterop might work well here to wrap a RenderingContext type): https://github.com/dart-lang/sdk/blob/main/sdk/lib/html/doc/WORKAROUNDS.md

denniskaselow commented 2 years ago

I suspect this may be a relic. Canvas corresponds to the native type WebGLCanvas, which from what I can tell, no longer exists. This should shift to CanvasElement.

I see, I was assuming the Canvas-class is a workaround because there are no union types in Dart to represent CanvasElement | OffscreenCanvas and dynamic would be an ugly return type.

It isn't clear to me in what context I should expect OffscreenCanvasRenderingContext2D to occur. Is it simply when getContext is called on an OffscreenCanvas object?

Yes, that's what I understand. Wouldn't make sense otherwise.

In that case, addressing your previous question, wouldn't I expect RenderingContext and RenderingContext2 to only return CanvasElements? If they returned an OffscreenCanvas, I would expect them to be OffscreenCanvasRenderingContexts instead.

Yes, that sounds reasonable, though I don't see any OffscreenCanvasRenderingContext other than the 2D one, even in the spec: https://html.spec.whatwg.org/multipage/canvas.html#the-offscreencanvas-interface

typedef (OffscreenCanvasRenderingContext2D or ImageBitmapRenderingContext or WebGLRenderingContext or WebGL2RenderingContext or GPUCanvasContext) OffscreenRenderingContext;

I still think we should get rid of Canvas in favor of CanvasElement, but yes, if it can be nullable, it should be written as such. When would the condition "It might be null if it is not associated with a canvas element or an OffscreenCanvas object." hold true?

I have absolutely no idea when it can be null. It's just the MDN page (https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/canvas) saying it can be null, and the current RenderingContext2 implementation that already says Canvas?. The spec doesn't say anything about null either: https://www.khronos.org/registry/webgl/specs/latest/1.0/#WebGLRenderingContext.

Using the names from the spec, both WebGLRenderingContext and WebLG2RenderingContext use WebGLRenderingContextBase, which has the line readonly attribute (HTMLCanvasElement or OffscreenCanvas) canvas;, same goes for WebGPU https://gpuweb.github.io/gpuweb/#canvas-context. So yeah, no clue where that nullability in the MDN or the relic Canvas? is even coming from.

It seems like the only major change here is moving RenderingContext.canvas and RenderingContext2.canvas to CanvasElement? types.

As there doesn't seem to be nullability according to the spec RenderingContext.canvas seems fine like it currently is except that OffscreenCanvas is also a possible return type, like defined in the spec. I haven't used OffscreenCanvas yet, so I have not run into that issue, but I just tried the example/repro code with an OffscreenCanvas (final webgl1ctx = canvas1.transferControlToOffscreen().getContext('webgl') as RenderingContext;), and both webgl and webgl2 return a runtimeType of OffscreenCanvas$ in Chrome when accessing canvas.

So.. solution one is to have the signature for both to be CanvasElement get canvas until something like CanvasElement | OffscreenCanvas may be possible in the far future. Or second solution have some workaround like that Canvas class. The first solution would be less breaking, as it's just a signature change for webgl2 but current code would keep working, while the second solution would break current code but would be clearer how/that it's possible to get a OffscreenCanvas.

I'm glad it's not my decision to make ;).

EDIT: I'm totally fine with solution one. Get rid of that Canvas thing and have RenderingContext2 simply return a non nullable CanvasElement. No need for additional OffscreenCanvasRenderingContext-variations that don't exist in the spec. If I ever need to use a OffscreenCanvas, I'm now aware how to access that one. When I need the canvas, I only need it for accessing width and height or drawing it onto another canvas, and that works for both even without casting to OffscreenCanvas, so it doesn't really matter that the returned object may be an OffscreenCanvas instead of a CanvasElement. At least to me, and not seeing any other issues concerning OffscreenCanvas this way, probably to no one else either.

denniskaselow commented 1 year ago

Can/will this be fixed with Dart 3.0?

For completeness I've added 2d and OffscreenCanvas as a repro (spoiler: only the 2d context has the correct signature for CanvasElement and OffscreenCanvas):

index.html

<!DOCTYPE html>

<script defer src="main.dart.js"></script>

<canvas id='webgl1'></canvas>
<canvas id='webgl2'></canvas>
<canvas id='ctx2d'></canvas>

<script>
  const canvas1 = document.querySelector('#webgl1');
  const webgl1ctx = canvas1.getContext('webgl');
  console.log(`Javascript webgl1ctx.canvas = ${webgl1ctx.canvas}`); // HTMLCanvasElement

  const canvas2 = document.querySelector('#webgl2');
  const webgl2ctx = canvas2.getContext('webgl2');
  console.log(`Javascript webgl2ctx.canvas = ${webgl2ctx.canvas}`); // HTMLCanvasElement

  const canvas2d = document.querySelector('#ctx2d');
  const ctx2d = canvas2d.getContext('2d');
  console.log(`Javascript ctx2d.canvas = ${ctx2d.canvas}`); // HTMLCanvasElement

  const offscreen1 = new OffscreenCanvas(256, 256);
  const offscreenWebgl1ctx = offscreen1.getContext('webgl');
  console.log(`Javascript offscreenWebgl1ctx.canvas = ${offscreenWebgl1ctx.canvas}`); // OffscreenCanvas

  const offscreen2 = new OffscreenCanvas(256, 256);
  const offscreenWebgl2ctx = offscreen2.getContext('webgl2');
  console.log(`Javascript offscreenWebgl2ctx.canvas = ${offscreenWebgl2ctx.canvas}`); // OffscreenCanvas

  const offscreen2d = new OffscreenCanvas(256, 256);
  const offscreenCtx2d = offscreen2d.getContext('2d');
  console.log(`Javascript offscreenCtx2d.canvas = ${offscreenCtx2d.canvas}`); // OffscreenCanvas
</script>

main.dart

import 'dart:html';

import 'dart:web_gl';

void main() {
  // CanvasElement
  final canvas1 = querySelector('#webgl1') as CanvasElement;
  final webgl1ctx = canvas1.getContext('webgl') as RenderingContext;
  print('Dart webgl1ctx.canvas = ${webgl1ctx.canvas.runtimeType}'); // correct: CanvasElement

  final canvas2 = querySelector('#webgl2') as CanvasElement;
  final webgl2ctx = canvas2.getContext('webgl2') as RenderingContext2;
  print('Dart webgl2ctx.canvas = ${webgl2ctx.canvas.runtimeType}'); // wrong: signature says Canvas, but runtimeType is CanvasElement
  print('Dart webgl2ctx.canvas.canvas = ${webgl2ctx.canvas?.canvas}'); // null
  print('Dart webgl2ctx.canvas.offscreenCanvas = ${webgl2ctx.canvas?.offscreenCanvas}'); // null

  final canvas2d = querySelector('#ctx2d') as CanvasElement;
  final ctx2d = canvas2d.context2D;
  print('Dart ctx2d.canvas = ${ctx2d.canvas.runtimeType}'); // correct: CanvasElement

  // OffscreenCanvas
  final offscreen1 = OffscreenCanvas(256, 256);
  final offscreenWebgl1ctx = offscreen1.getContext('webgl') as RenderingContext;
  print('Dart offscreenWebgl1ctx.canvas = ${offscreenWebgl1ctx.canvas.runtimeType}'); // wrong: signature says CanvasElement, but runtimeType is OffscreenCanvas$

  final offscreen2 = OffscreenCanvas(256, 256);
  final offscreenWebgl2ctx = offscreen2.getContext('webgl2') as RenderingContext2;
  print('Dart offscreenWebgl2ctx.canvas = ${offscreenWebgl2ctx.canvas.runtimeType}'); // wrong: signature says Canvas, but runtimeType is OffscreenCanvas$
  print('Dart offscreenWebgl2ctx.canvas.canvas = ${offscreenWebgl2ctx.canvas?.canvas}'); // null
  print('Dart offscreenWebgl2ctx.canvas.offscreenCanvas = ${offscreenWebgl2ctx.canvas?.offscreenCanvas}'); // null

  final offscreenCanvas2d = OffscreenCanvas(256, 256);
  final offscreenCtx2d = offscreenCanvas2d.getContext('2d') as OffscreenCanvasRenderingContext2D;
  print('Dart offscreenCtx2d.canvas = ${offscreenCtx2d.canvas.runtimeType}'); // correct: OffscreenCanvas$
}

Possible fixes I can think of:

  1. Either introduce a OffscreenRenderingContext and OffscreenRenderingContext2 like the OffscreenCanvasRenderingContext2D but the latter actually exists in the spec, not so the former - and return CanvasElement for RenderingContext and RenderingContext2 and OffscreenCanvas for the newly introduced classes and remove the Canvas-class. Maybe even remove the nullability of CanvasElement, as that only exists in the MDN but not in the specs and I'm not aware of a way to create a context without a canvas.
  2. Make the canvas getter for the RenderingContext2 actually return a Canvas where you can either access the CanvasElement or the OffscreenCanvas (in this case the CanvasElement should be nullable because it can be null when created from an OffscreenCanvas). But then it would also be needed for the RenderingContext and Canvas may not need to be nullable for both for the same reason as solution 1.
srujzs commented 1 year ago

I seem to have missed responding to your previous comment, sorry! We've been busy with moving towards more interop features (with inline classes and expanding the functionality of the existing @staticInterop) and with it, an interop-based DOM library instead. Part of the motivation was issues like these where the Dart code surrounding the native calls became either stale or incorrect.

Can/will this be fixed with Dart 3.0?

This specific issue won't be addressed within dart:html in the next few months, but https://github.com/dart-lang/web (feel free to take a look, but it's very much a WIP to refactor/clean up/etc.) will hopefully be available in the next few months for people to use for Dart 3. It'll expose the DOM (and browser APIs in general) with interop and shy away from some of the Dart-esque decisions we've made with dart:html.

We'll be sticking to the WebRef specs largely to expose these types, so I expect we'll come closer to addressing your first solution. The nullability too will adhere to the spec instead. With the newer library, we should conform to the behavior of the JS code in your above html code.