Html: Float compatibility issues on implementations of CanvasContext2D getImageData()

[fserb]

I want to address a compatibility issue on the implementation of CanvasContext2D getImageData() regarding non-int inputs. There are 3 different implementations out there (Chrome/Safari/Opera, Firefox and Edge) and they all differ in what they return for non-integer inputs.

For a summary of the results:
https://cdn.rawgit.com/fserb/a22ab8bd708f0a2ea3c5b4ede4c6156e/raw/3411bdd4a7150445c4c1873117a24c8d46d8d39c/getimagedata-diff.html

Here is the relevant standard:
https://html.spec.whatwg.org/multipage/scripting.html#dom-context-2d-getimagedata

The implementations diverge on how to handle non-int inputs, as can be seen on the first link. But in summary:

Gecko: clamp all values to ToInt32(), but having to force a value of 1 for size dimensions in (0,1) - since it can't throw an error for those values.
Blink/WebKit: include all pixels that touch the rectangle, but also forcing a value of 1 for size dimensions in (0,1), even if the resulting rectangle would include a result anyway.
Edge: Close to Gecko, afaik, but round() instead of ToInt32().

Historically, this interface was originally getPixels(int, int, int, int), but it changed to getImageData(float, float, float, float) during a conversation trying to make it clear those were canvas pixels and not CSS/Display units. After that, a new topic over getImageDataHD was brought up also trying to represent backing stores resolution, but we backed down on this.

I'm not particularly attached to any single solution, as long as we have one. But for conversation purposes, I'll suggest some:

Option 1: make getImageData(int, int, int, int).
Option 2: keep getImageData as is but clarify that partial pixels must be included.
Option 3: keep getImageData as is but clarify that only full pixels must be included.

I started this thinking Option2 was the best, but after spending a couple days looking at this, I think Option1 is the sanest one.

Anyway, my proposal is to clean up the language, clarify the implementation and create web-platform-tests to cover the new spec. I'd be willing to do the leg work, after people agree on a direction.

[annevk]

Did you (also) test on macOS or iOS with a 2x/3x screen? It shouldn't matter I think, but just in case.

I kinda agree that as we moved away from ImageData potentially representing more or less pixels than the the number of canvas pixels it no longer makes sense to extract ImageData from subpixels.

[junov]

+1
Given that:
a) the motivation for using double no longer exists, and
b) fractional values are not currently interoperable
I think it would make sense to just revert back to "long" in the IDL.

Did you verify whether negative values of sw and sh are interoperable? According to the spec, inverted rectangles should work fine. I'm wondering whether that is true in practice.

[fserb]

@annevk, I did test on retina Mac, not on iOS. Same results (on Chrome, Safari and Firefox).

@junov, I did test negative numbers. Everyone does the same thing for negative values of sw/sh. The only way they are different is how they round negative numbers, which is a even bigger mess than positive rounding.

[annevk]

Given that Edge and Firefox effectively have long behavior already, perhaps a good way forward here is to file a bug against WebKit to make sure they're aware there's a plan on changing this and then start working on tests and standard changes. This seems relatively uncontroversial, but better safe than sorry.

[fserb]

Just to be clear. Edge and Firefox have long behavior-ish (edge is round(), firefox is ToInt32()/clamp-ish), but they would still need to slightly adapt. Forcing to int/long, would slightly change the behavior on the "round to 1" issue for every implementation. Which I think is the correct thing, but just to be clear that there will be a very small change everywhere (namely, removing the "if (w < 1) w = 1" code).

For example: getImageData(0, 0, 0.1, 1.5). Right now, the considered sizes on each browser would be: Edge: (1,2), Firefox: (1,1), Chrome/Safari/Opera: (1, 1.5). Defining getImageData as long, which is good, would mean to consider this (0,1) and throw an exception.

That said, I'd be ok with filing a heads up bug on Webkit, Blink and Gecko.

[annevk]

Fair enough, yeah, please file those bugs now and mention them here. I can help with triage for Gecko and maybe WebKit too. If you have a Microsoft account you can file issues for Edge at https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/, but I'm happy to do that too after I can copy-and-paste from your reports.

[fserb]

Heads up bugs:

WebKit: https://bugs.webkit.org/show_bug.cgi?id=169622
Blink: https://bugs.chromium.org/p/chromium/issues/detail?id=701490
Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1347268
Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11273296/

Feel free to triage them as you see fit or add any relevant info that I may have missed.

[fserb]

Just FYI, the browser bugs have all been updated with the commits. :) thanks.

[jdm]

Based on the results of wpt.fyi (https://wpt.fyi/results/2dcontext/pixel-manipulation/2d.imageData.create2.nonfinite.html?label=experimental&label=master&aligned&q=create2.nonfinite), it appears as though none of the browsers actually updated their IDL to match the specification since this change was made 3 years ago. Infinite values still generate a type error instead of 0. I've opened https://github.com/whatwg/html/issues/5336 in the hopes that we can make the specification reflect what is actually being shipped by all browsers.