deprecate_emscripten_set_get_canvas_size

[juj]

Deprecate emscripten_set/get_canvas_size() in favor of emscripten_set/get_canvas_element_size() which are slimmer to the point and allow specifying the target canvas element to resize, instead of relying on a singleton Module['canvas'].

[juj]

Looking at the implementation of emscripten_set_canvas_size, it reads:

emscripten_set_canvas_size: function(width, height) {
  Browser.setCanvasSize(width, height);
},

setCanvasSize: function(width, height, noUpdates) {
  var canvas = Module['canvas'];
  Browser.updateCanvasDimensions(canvas, width, height);
  if (!noUpdates) Browser.updateResizeListeners();
},

updateCanvasDimensions : function(canvas, wNative, hNative) {
  if (wNative && hNative) {
    canvas.widthNative = wNative;
    canvas.heightNative = hNative;
  } else {
    wNative = canvas.widthNative;
    hNative = canvas.heightNative;
  }
  var w = wNative;
  var h = hNative;
  if (Module['forcedAspectRatio'] && Module['forcedAspectRatio'] > 0) {
    if (w/h < Module['forcedAspectRatio']) {
      w = Math.round(h * Module['forcedAspectRatio']);
    } else {
      h = Math.round(w / Module['forcedAspectRatio']);
    }
  }
  if (((document['fullscreenElement'] || document['mozFullScreenElement'] ||
       document['msFullscreenElement'] || document['webkitFullscreenElement'] ||
       document['webkitCurrentFullScreenElement']) === canvas.parentNode) && (typeof screen != 'undefined')) {
     var factor = Math.min(screen.width / w, screen.height / h);
     w = Math.round(w * factor);
     h = Math.round(h * factor);
  }
  if (Browser.resizeCanvas) {
    if (canvas.width  != w) canvas.width  = w;
    if (canvas.height != h) canvas.height = h;
    if (typeof canvas.style != 'undefined') {
      canvas.style.removeProperty( "width");
      canvas.style.removeProperty("height");
    }
  } else {
    if (canvas.width  != wNative) canvas.width  = wNative;
    if (canvas.height != hNative) canvas.height = hNative;
    if (typeof canvas.style != 'undefined') {
      if (w != wNative || h != hNative) {
        canvas.style.setProperty( "width", w + "px", "important");
        canvas.style.setProperty("height", h + "px", "important");
      } else {
        canvas.style.removeProperty( "width");
        canvas.style.removeProperty("height");
      }
    }
  }
},

updateResizeListeners: function() {
  var canvas = Module['canvas'];
  Browser.resizeListeners.forEach(function(listener) {
    listener(canvas.width, canvas.height);
  });
},

In this flow, emscripten_set_canvas_size has the following roles:

1. emscripten_set_canvas_size is only able to operate on Module['canvas'] in a singleton manner.
2. If Module.forcedAspectRatio is set (default undefined), emscripten_set_canvas_size() will not (intentionally) resize to the requested size, but (arbitrarily) decreases the smaller of requested width/`height upwards to constrain to the desired aspect ratio.
3. If Browser.resizeCanvas is set, then not only does emscripten_set_canvas_size() set the canvas render target size, but it also sets the CSS size of the DOM element to match the low DPI (i.e. non-Retina, https://www.khronos.org/webgl/wiki/HandlingHighDPI) resolution of the canvas. Browser.resizeCanvas is undefined by default, but looks like f one calls Browser.requestFullscreen(resizeCanvas=true); once, then Browser.resizeCanvas will stick to true even after one exits fullscreen (fixable).
4. It is possible to add a callback listener via Browser.resizeListeners.push(myCanvasResizeListener) to listen to canvas render target size (not visible CSS size!) changes. However, this has the same limitation of only being able to listen to size changes on singleton Module['canvas'], but also that it is only able to fire events if one explicitly uses the emscripten_set_canvas_size() or Browser.setCanvasSize() API to specify the size of the canvas. If one directly uses canvas.width/.height = foo; code to change the size, the events will not fire.

It feels like most of this logic is misplaced, and we need primitive functionality that does only what is requested, without building more complex logic to the bottommost level.

The part 1. is fixed in the new API signature in this PR.

The part 2. comes from 94cb8f9, which added the Module.forcedAspectRatio to help implementing aspect ratio correct fullscreen transitions. The HTML5 API shows a better way to implement this that does not need adding fields on Module object, and, more imporantly, does not mutate the DOM.

Item 3. feels like caller responsibility as well. I.e. if one wants to set both the canvas render target size and the CSS size, it is cleaner for one to call two functions, without relying on boolean state like Browser.resizeCanvas. Originally, Browser.resizeCanvas was also introduced to do fullscreen mode fixes, a7fa285, so this should also be solved via a mechanism in the HTML5 API.

Finally, item 4. is something I am somewhat torn about. We could add similar listener logic to emscripten_set_canvas_size, although since that is not a real listener, and will preclude users from being able to do canvas.width/.height = foo in their on JS code or the listener will break, I would prefer to deprecate the listener machinery altogether, and rather find other ways to solve the root problem. Browser.resizeListeners was added way back in 9981061, and at minimum, it should be located as a non-singleton as Canvas.resizeListener for each canvas.

The intent of this PR is to deprecate the usage of emscripten_set_canvas_size and emscripten_get_canvas_size on C code side, and not immediately touch the JavaScript side Browser.setCanvasSize() API. The new functions emscripten_set_canvas_element_size and emscripten_get_canvas_element_size are easy to make OffscreenCanvas+multithreading compatible with OffscreenCanvas, but the overloaded roles in emscripten_set_canvas_size and emscripten_get_canvas_size makes it very difficult to be supported with OffscreenCanvas and multithreading.

Another way forward might be not to deprecate emscripten_set_canvas_size and emscripten_get_canvas_size at first at least, but document them to be only available in singlethreaded/non-OffscreenCanvas builds. So we could land this without the deprecation warnings added if so desired. Though I'd really prefer vocally advocating developers to be aware of emscripten_set/get_canvas_element_size instead of emscripten_set/get_canvas_size in C/C++ code. That will allow avoiding odd workarounds like https://github.com/emscripten-ports/SDL2/blob/master/src/video/emscripten/SDL_emscriptenevents.c#L579, which needs to get and set CSS size when setting the canvas render target size, in order to only change the render target size and not the CSS size.

Finally, there are tests for the new functions, but I have written them to be multithreading aware (1), so would prefer this to be merged without tests firsts, and testing to be introduced once the other multithreading capable bits land.

[kripken]

lgtm

[kripken]

Needs tests though.

[juj]

Added simple singlethreaded tests. There's a much larger multithreaded test for this in my another branch (incoming...juj:pthread_proxy_main), which I'm working on pulling in bit by bit in manageable review units.