whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.18k stars 2.69k forks source link

Float16Array + HTML Canvas #8708

Open palemieux opened 1 year ago

palemieux commented 1 year ago

FYI. Below is input from the Color on the Web CG regarding Float16Array in the context of HTML Canvas. This input was prompted by https://github.com/w3c/ColorWeb-CG/issues/87

-- W3C Color on the Web CG chairs https://www.w3.org/community/colorweb/ public-colorweb@w3.org

====================

CG proposal re: Float16Array + HTML Canvas

The ColorWeb CG is seeking consensus with parties interested in discussing floating-point backing stores for canvas rendering contexts.

The CG spec proposal is: https://github.com/w3c/ColorWeb-CG/blob/main/canvas_float.md

The primary outstanding issue at present is how to represent values which are read back from the canvas into typed arrays which can be manipulated from ECMAScript.

The current spec proposal supports readbacks into both Uint8ClampedArray and Float32Array via ImageDataColorType.

It has been suggested to also add support for readbacks into Float16Array (https://github.com/w3c/ColorWeb-CG/issues/87). This would make the specification dependent on the Float16Array type currently being defined by the ECMAScript committee (ISO/TC 39).

The CG supports the idea of adding support for Float16Array to ECMAScript. However, the CG feels it is most appropriate to decouple the ongoing improvements to ECMAScript from the work on floating-point canvases. Users of the web platform are expressing the need for floating-point canvas backing stores now; in Chromium, implementation is underway in crbug.com/1230619. Moreover, readback and CPU-side manipulation paths are not high-performance, so the additional memory bandwidth of using Float32Array for this task will not critically impact applications.

The ColorWeb CG therefore suggests:

1) Moving ahead with https://github.com/w3c/ColorWeb-CG/blob/main/canvas_float.md in its current form, with unorm8 and float32 as options for readback.

2) In parallel, working on adding Float16Array to ECMAScript, and prototyping it in browsers.

3) Specifying a "float16" enum in ImageDataColorType once implementation experience has been achieved with Float16Array, . Applications can feature-detect its support in browsers by catching exceptions raised by getImageData or createImageData.

Background

For context, introducing a Float16Array into the Typed Array hierarchy was discussed several years ago among members of TC39, as well as the Khronos Group where Typed Arrays originated. At the time, the costs of supporting this type natively in ECMAScript engines outweighed the benefits for the following reasons:

1) For practical purposes, Float16Array could be sufficiently polyfilled in ECMAScript:

https://github.com/petamoriken/float16

Corner cases existed regarding denormalized values, NaNs, and infinities - but applications could generally achieve their desired results, with good performance on all ECMAScript engines, using existing primitives.

2) Direct CPU support for half-float numbers did not seem to exist. At the time, C libraries which worked with this data type universally emulated it, rather than using compiler intrinsics on certain platforms.

3) A significant amount of work would be required in every ECMAScript engine to make this typed array type perform well.

4) A significant amount of work would be required to specify the behavior of this numeric type in ECMAScript.

The landscape has changed in recent years. Per https://en.wikipedia.org/wiki/Half-precision_floating-point_format#Hardware_support, ARM and x86 processors either support FP16 natively, or will in the near future. FP16 formats are increasingly heavily used in neural network evaluation and image processing, including on the web. The ECMAScript editors may be able to save spec work by referencing IEEE specifications (which, to be fair, already existed when Float16Array was originally considered).

annevk commented 1 year ago

I'm worried that this removes the incentive to get the work done on Float16Array.

It also ends up cementing the wrong default:

If the CanvasImageData has a colorType of "float16", then the ImageData's colorType will be "float32"

We don't want float32 there in the future.

annevk commented 1 year ago

cc @whatwg/canvas

palemieux commented 1 year ago

@annevk Can you confirm that your concern is specifically with selecting Float32Array as the default readback type? If so, we can use Uint8ClampedArray as the default readback type instead.

We are also happy to support the Float16Array proposal in TC 39. Let us know how to best coordinate the effort/proceed.

In the meantime, are there objections to supporting Float32Array as one of the (non-default) readback types and adding Float16Array once the proposal makes its way through the TC 39 process?

annevk commented 1 year ago

What are the use cases for Float32Array? And would it not be weird if the canvas is float16 and you get uint8 data out even if Float16Array is a thing?

palemieux commented 1 year ago

The use case for Float32Array that is of concern to the Color CG is the manipulation of HDR pixels, for which UInt8 is not sufficient.

The idea is for getImageData() to continue returning UInt8 data by default, even if the contents of the Canvas is higher bit-depth. An application could request higher bit depths, e.g., Float16Array, by passing a configuration parameter to getImageData().

annevk commented 1 year ago

Right, but Float16Array meets that need as well and shouldn't be difficult to add to the platform. And would allow for more useful defaults as well.

palemieux commented 1 year ago

Please provide a concrete proposal and the steps needed to get there, and the CG can consider it.

annevk commented 1 year ago
  1. The Color CG or one of its participants follows through on https://github.com/tc39/proposals/issues/453 to get Float16Array in ECMAScript/JavaScript done.
  2. We add Float16Array to Web IDL.
  3. We incorporate it in this API (and others).
palemieux commented 1 year ago

@annevk Are you ready to be a proponent to the work in TC 39, and are ready to implement the feature?

bakkot commented 1 year ago

I've made a repo to track the proposal in TC39. I'm not very familiar with the space, so while I'd be willing to champion the proposal in TC39 it would be helpful if someone from ColorWebCG could lay out why it would be helpful to have this (and, specifically, why Uint16Array and Float32Array aren't sufficient). The best thing would be if you could open an issue on that repository with your use case, but if you respond here or point me to a summary I can try to copy that into an issue myself.

palemieux commented 1 year ago

@bakkot Thanks for getting this started. I am happy to coordinate input from the Color CG. In the meantime:

Do you think the explainer should include background on HDR and WCG imagery?

bakkot commented 1 year ago

A little bit of background on why Uint16Array is not sufficient would be very helpful, yes. Most people on the committee (myself included) don't have any particular expertise here. I'm happy to take it on faith, but if there's a short explanation you can give to a layperson that would be great.

I've signed myself up to present the proposal for stages 2 and 3 (essentially skipping a stage due to the fact that the design is entirely straightforward) at the March meeting, which begins on March 21, so that's the timeline. If it gets approved at that point - which is not entirely a sure thing, but I'm hopeful - it will be ready for browsers to begin developing and shipping implementations at that point.

ccameron-chromium commented 1 year ago

Perhaps we should split this proposal into two proposals: (1) that adds floating-point backing for CanvasRenderingContext2D and (2) that adds whatever formats are considered useful for ImageData.

As was mentioned earlier, I now consider it a bug that I proposed Float32Array as a default format for getImageData. We should not change the default type for that function.

annevk commented 1 year ago

@ccameron-chromium can you explain? getImageData() using the same backing as the 2D context it is invoked upon makes sense to me. (While looking for other impacted features I found #8917.)

ccameron-chromium commented 1 year ago

It's a trade-off about what the "best" default behavior is. What is "best" here is a bit subjective, and I can understand people disagreeing here. I'll outline my reasoning below. Note that this is just for the default behavior -- with ImageDataSettings, the developer is in full control of the format in which data is read back.

The guiding principle that leads me to "we should use Uint8ClampedArray" is as follows: Suppose someone has written naïve code that is unaware of the existence of different CanvasRenderingContext2D colorTypes. What should happen to them? If someone writes code assuming Uint8ClampedArray, and they unexpectedly get a Float32Array or Float16Array instead, the result will be catastrophic. E.g, the developer blindly writes 255 for SDR white and instead gets a color value that is 524,947 times brighter than SDR white. E.g, the developer blindly reads the colors and gets values in 0..1, and treats them all as being black.

The guiding principle that originally lead me to "we should use Float32Array" would be that a naive getImageData/putImageData pair that has no pixel manipulation in between should not lose precision. The more I think about this, the more I think that it does not represent a realistic use case, because of the "no pixel manipulation in between" part.

I also see an analogous behavior in toDataURL. If one tries to round-trip data using that, they will have values clamped to 0..1 in the canvas' color space.

WRT #8917, I think there is a misunderstanding there with respect to the color space of the backing bitmap of a CanvasRenderingContext2D versus the parameterization of a CSS color. I put some details on that bug. Feel free to reach out if I misunderstood the question myself.

annevk commented 1 year ago

Presumably if you opt-in to a float16 context you do so to be able to do float16-based manipulation. Having to opt-in again for each getImageData() call seems cumbersome.

Kaiido commented 1 year ago

On the other hand I wonder if it's backward compatible to return something new here. There are cases where you want to grab the ImageData out of a context that you didn't initialize, e.g. to extend from a library. Having a variable format for the default call to getImageData might break any script that were expecting an Uint8 array there when the library is updated to use HDR.

kenrussell commented 1 year ago

Float16Array meets that need as well and shouldn't be difficult to add to the platform

Adding Float16Array to the web platform is far from trivial. Every ECMAScript engine must be updated to support it, and it must be implemented as efficiently as all of the other Typed Array types from the start, because web authors already have expectations of high performance from the other typed array variants.

In each engine this involves revising an interpreter and one or more tiers of compilers to understand the float16 data type, register allocate it, define efficient conversions to and from other ECMAScript numeric representations, perform good per-CPU-architecture instruction selection to keep these values in the float16 representation as long as possible (again, for efficiency and performance), tightly specify and test these conversions, and finally ship them. Having participated in V8's implementation of Typed Arrays I would estimate this as a minimum of six months of dedicated work, and likely more.

In my opinion as a ColorWeb CG member, it's not acceptable for the WHATWG to require Float16Array as a hard dependency for the specification changes the ColorWeb CG is incubating and proposing. I respectfully request that the WHATWG members evaluating this issue agree with the ColorWeb CG's evaluation, and allow the specs to move forward using Float32Array as an ECMAScript-exposed representation of the canvas's float16 backing store.

If this can be agreed upon, then even if it were later changed to Float16Array (without requiring an application-level opt-in), that would likely not be a breaking change. The values read back from the canvas would be exactly the same in both representations - 0 bits are added to the mantissa and exponent when expanding float16 to float32. float32 adds more precision and range, but both would already be silently dropped / truncated when writing values to the canvas. If the application must explicitly choose the representation, then there is no risk of breakage.

domenic commented 1 year ago

In my opinion as a ColorWeb CG member, it's not acceptable for the WHATWG to require Float16Array

Let's be clear here that in no part of this thread is "the WHATWG" "requiring" anything. We have individuals expressing opinions.

Implicitly, those opinions often represent "what the browser engine that X individual represents would require, in order to implement the proposal", but I haven't seen anyone be explicit about speaking for any given company or browser project so even that is not clear yet.

But there's no reason to treat comments on an issue thread as some sort of organizational position. That's never really the case in the WHATWG. We specify what is implemented and shipped in browsers. If you get 2+ browsers to ship Float32Array-based or Float16Array-based or Float1337Array-based canvases, that's what will make it into the spec, per our working mode.

othermaciej commented 1 year ago

Setting aside differences in amount of spec work and implementation, is it a better design if we have Float16Array and can build on it? It seems to me it is. From what I see here, the main arguments against are the expediency of being able to ship somewhat faster, by avoiding the ECMAScript standards process and JS engine implementation work. If that was a very large difference, like multiple years, then maybe cutting corners in the design would be warranted. On the other hand, the future is big, and web APIs tend to last a long time, so if the delta relatively small (like less than a year), then it's probably better to do the extra work.

The above is a personal opinion, now, with more of an Apple hat on:

That's much less than the 6 month estimate above, and tentatively seems more credible to me since it comes from an expert on the relevant VM internals.

bakkot commented 1 year ago

The spec work for Float16Array seems relatively straightforward to us, we're looking into whether any Apple folks who have TC-39 experience can help.

The spec work to add it to JS is entirely trivial; I have already done it as part of putting together the proposal linked above.

ccameron-chromium commented 1 year ago

I want to get back to the discussion of "what is the best default format for getImageData"?

I still think that changing that to match the canvas is a bad idea, for the reasons that Kaiido articulated above.

To be clear, I'm a huge fan of Float16Array, and I consider it to be strictly better than Float32Array for use with ImageData. But the only place that it touches this spec is the default ImageDataSettings for getImageData, and I don't think it should touch that part of the spec.

othermaciej commented 1 year ago

Changing the type that getImageData returns is extremely likely to break (like, render nonfunctional) libraries. Any library that exists to day that is passed a canvas for any reason is broken if it gets passed a higher precision canvas.

Some questions that might highlight how significant this risk is:

I’d expect the answer to both questions to be “no”; even one of them being “no” would, I think, reduce the force of the quoted argument. The first I’d expect to be somewhat uncommon because canvas readback is slow. The second because it seems this would break HDR or other intended color effects over a read-write-modify cycle. Perhaps it’s common to do pure reads and do math on them without writing back; and it’s ok if that math uses the wrong colors, but it would totally explode if given float16 values instead of uint8 values. But that doesn’t seem all that likely.

Still, if there actually is substantial compatibility risk to changing the default format at all, then perhaps the existing method should be forever uint8 and there should be a new one that gets the canvas’s most preferred representation. Leaving it to the library or the developer to figure out the best format for the backing store seems like a stumbling block, especially if the mapping is going to be non-obvious for future formats.

Kaiido commented 1 year ago

Is it actually common for general-purpose canvas libraries to use getImageData / putImageData?

Sure, given that as of today Safari still doesn't support ctx.filter using getImageData() / putImageData() is the only way to have some kind of cross-browser CSS filters. All "polyfills" out there are doing getImageData() / putImageData(). Many non-standard filters are also done this way. Image analysis like the ones provided by tensorflow also use getImageData(). Some very bad code are still doing collision detection by checking the pixel values. Flood fill algos are using getImageData()/ putImageData(). All color histogram scripts from more than 2 years ago are using getImageData(). All these (and probbaly many more examples) are expecting Uint8, and passing anything else will simply break them because all of the sudden they'll see just almost transparent black images when they receive a Float16 instead.

is it likely that such functionality would actually give correct behavior blindly operating on uint8 data when the underlying backing store is float16?

That sounds like a moot point. Losing HDR precision when applying a filter or doing image analysis isn't as bad as working on an almost transparent black image. In one case, at worst you lose some "cool feature", in the other, you have a broken product. Also, remember that HDR canvas is still relatively new and still not that widespread.

perhaps the existing method should be forever uint8 and there should be a new one that gets the canvas’s most preferred representation.

That's what is being proposed already: getImageData(x, y, width, height, settings)'s settings would provide a way to define the output format. All that's being discussed here is the default format.

syg commented 1 year ago

The spec work to add it to JS is entirely trivial; I have already done it as part of putting together the proposal linked above.

It should be very explicitly noted that the triviality of the spec changes here is not at all indicative of the implementation challenges. And each browser JS engine may have fairly different tradeoffs to consider around what architectures to support, because half-precision support on CPUs is limited and relatively new.

Implementing fully optimized Float16Array (on all JIT tiers) takes from 2 weeks to 1 month. I think 6 months was reasonable for typed arrays because it is bringing up all typed array infrastructure from scratch. Adding one Float16Array does not require this level of thing.

V8's estimate is more conservative: we think a quarter at least (exclusive of standardization time). Our supported hardware run a wider gamut and the software emulation that may be required play into the longer estimate.

kdashg commented 1 year ago

I think:

Put another way, we still don't have a core float16 type in C++ and this situation is tolerated by developers. I think the feature is nice-to-have, but not extremely compelling, even in this use case here.

If the memory bandwidth story is found to be compelling, I think it would even be tolerable to return a uint16array. Some amount of decode logic is already needed for the uint8array path, since e.g. the decode function is v => v/255.0, so the difference would be in the complexity of the encode/decode func, not a new extra step per se. (bonus points for a toFloat16(Number) js builtin)

I appreciate the drive to make it ideal immediately, but unfortunately I don't think all the dependencies are low-hanging-fruit enough. :(

othermaciej commented 1 year ago

is it likely that such functionality would actually give correct behavior blindly operating on uint8 data when the underlying backing store is float16?

That sounds like a moot point. Losing HDR precision when applying a filter or doing image analysis isn't as bad as working on an almost transparent black image. In one case, at worst you lose some "cool feature", in the other, you have a broken product. Also, remember that HDR canvas is still relatively new and still not that widespread.

I think filters breaking in an obvious way might actually be better than breaking in the subtle way of filters destroying HDR precision, because the dev is likely to notice the breakage when adopting HDR canvas. If adoption is low, that means few will be broken already.

perhaps the existing method should be forever uint8 and there should be a new one that gets the canvas’s most preferred representation.

That's what is being proposed already: getImageData(x, y, width, height, settings)'s settings would provide a way to define the output format. All that's being discussed here is the default format.

No, sorry, these are not the same proposal. Your proposal does not provide any way to get whatever is the best backing store type for any given canvas, without having to know its format and what type that maps to. It enshrines a uint8-forever method, and gives no method that automatically gets the best format for a given canvas. It also would make the code for getting the correct backing store format verbose and awkward to write.

What I am suggesting is more like introducing a getPrecisionImageData() that gets the most appropriate format by default, which would be Float16Array for canvases with HDR backing stores. I don’t know if that’s the best plan on the whole, but I don’t see an obvious reason to omit this operation if getImageData() is forever locked to uint8.

othermaciej commented 1 year ago

I think:

  • we should leave getImageData as uint8array, and that
  • we should absolutely not block on having float16array, which is non-trivial implementation work according to multiple implementers.

Put another way, we still don't have a core float16 type in C++ and this situation is tolerated by developers. I think the feature is nice-to-have, but not extremely compelling, even in this use case here.

C++ allows sufficiently low-level access that it’s not as necessary to have a core type. C++ devs work around this when necessary by using potentially CPU-specific vector types and operations, and certainly not by representing float16 buffers as unit8 buffers scaled by 255.

If the memory bandwidth story is found to be compelling, I think it would even be tolerable to return a uint16array. Some amount of decode logic is already needed for the uint8array path, since e.g. the decode function is v => v/255.0, so the difference would be in the complexity of the encode/decode func, not a new extra step per se. (bonus points for a toFloat16(Number) js builtin)

That just feels like an obviously goofy workaround. No one would design a system that way if not for expediency.

I appreciate the drive to make it ideal immediately, but unfortunately I don't think all the dependencies are low-hanging-fruit enough. :(

There’s really only one dependency and even the most conservative estimates do not make it all that high-hanging, as they are significantly less than a year. Is that amount of extra speed worth introducing a quirky pitfall into the Web Platform forever?

(I understand there are separate arguments for why always returning uint8 is actually better, but this seems to be solely an expediency-based argument to standardize and ship something a quarter or two sooner.)

ccameron-chromium commented 1 year ago

Fleshing out some of the current-and-future behaviors here.

[...] gives no method that automatically gets the best format for a given canvas. It also would make the code for getting the correct backing store format verbose and awkward to write.

What I am suggesting is more like introducing a getPrecisionImageData() that gets the most appropriate format by default, which would be Float16Array for canvases with HDR backing stores. I don’t know if that’s the best plan on the whole, but I don’t see an obvious reason to omit this operation if getImageData() is forever locked to uint8.

In the proposal as it exists, there is an obvious "best match" (assuming the existence of Float16Array). But that may not always be the case.

Another extremely useful format for backing WCG and HDR canvases would be GL_RGB10_A2 aka MTLPixelFormatRGB10A2Unorm. Maybe we'll want to add that. But what's the "best match" for that? I could see an argument for Uint16Array, Float16Array, or Uint32Array-with-some-packing. That's another reason why I want to stay out of the "default is best match" game -- "best mach" isn't always obvious.

How awkward is it for an application to do a canvas<->imageData conversion? It would look like this:

  function getPreferredImageDataColorType(c) {
    switch (c.getContextAttributes().colorType) {
       case 'unorm8': return 'unorm8';
       case 'float16': return 'float16';
       default: return 'float32'; // Maybe a new type was added!
    }
  }
  let myImageData = c.getImageData(x, y, w, h, {colorType:getPreferredImageDataColorType(c)});

WRT getPrecisionImageData, if we were to go that route, I'd imagine it being agetBestMatchImageDataSettings function (for use with the ImageData ctor and canvas createImageData functions as well). In that case, we'd do something like

  let myImageData = c.getImageData(x, y, w, h, c.getBestMatchImageDataSettings());
othermaciej commented 1 year ago

Fleshing out some of the current-and-future behaviors here.

[...] gives no method that automatically gets the best format for a given canvas. It also would make the code for getting the correct backing store format verbose and awkward to write. What I am suggesting is more like introducing a getPrecisionImageData() that gets the most appropriate format by default, which would be Float16Array for canvases with HDR backing stores. I don’t know if that’s the best plan on the whole, but I don’t see an obvious reason to omit this operation if getImageData() is forever locked to uint8.

In the proposal as it exists, there is an obvious "best match" (assuming the existence of Float16Array). But that may not always be the case.

Another extremely useful format for backing WCG and HDR canvases would be GL_RGB10_A2 aka MTLPixelFormatRGB10A2Unorm. Maybe we'll want to add that. But what's the "best match" for that? I could see an argument for Uint16Array, Float16Array, or Uint32Array-with-some-packing. That's another reason why I want to stay out of the "default is best match" game -- "best mach" isn't always obvious.

For many formats there is an obvious single best choice. For others there is an at least reasonable choice. Picking a default doesn't prevent thoughtful developers from using different types that are more useful for their use case. In this case, UInt16Array seems like a reasonable default choice.

I'll add that this particular example means array type is not enough to represent a backing store format. There's also the conversion method. UInt32Array would presumably not mean UInt32 per channel, rather pack it all into 32 bits. That's different than a format that's UInt32-per-channel. It would need to both be a different colorType

How awkward is it for an application to do a canvas<->imageData conversion? It would look like this:


  function getPreferredImageDataColorType(c) {
    switch (c.getContextAttributes().colorType) {
       case 'unorm8': return 'unorm8';
       case 'float16': return 'float16';
       default: return 'float32'; // Maybe a new type was added!

This list expands for every color type added. Even ones with the same most preferred backing store.

But also, would this code work in what's proposed to initially ship and go into the spec? I thought float16 was initially not going to be an option until later ECMA spec and implementation work gets done. Does this imply that c.getContextAttributes().colorType would not return float16 until a browser supports Float16Array?

}

} let myImageData = c.getImageData(x, y, w, h, {colorType:getPreferredImageDataColorType(c)});


WRT `getPrecisionImageData`, if we were to go that route, I'd imagine it being a`getBestMatchImageDataSettings` function (for use with the `ImageData` ctor and canvas `createImageData` functions as well). In that case, we'd do something like

let myImageData = c.getImageData(x, y, w, h, c.getBestMatchImageDataSettings());

Seems needlessly verbose, especially in cases where it would be useful to customize other settings besides the color format (if there ever are any).

Besides that, I don't find the case for uint8 as the forever default very convincing. Setting aside arguments about implementation expediency, what's left it allowing canvases with newer color formats to be used with old unmodified libraries in a way that will cause subtle bugs instead of blatant bugs. That doesn't sound like a big improvement.

A colleague suggested getImageData on a float16-backed canvas should throw in implementations that don't yet support Float16Array. I am not sure that is the best option but I think it's better than uint8 default.

petamoriken commented 1 year ago

Put another way, we still don't have a core float16 type in C++ and this situation is tolerated by developers. I think the feature is nice-to-have, but not extremely compelling, even in this use case here.

C++ allows sufficiently low-level access that it’s not as necessary to have a core type. C++ devs work around this when necessary by using potentially CPU-specific vector types and operations, and certainly not by representing float16 buffers as unit8 buffers scaled by 255.

FYI: Support for float16_t type will be coming in C++23. https://en.cppreference.com/w/cpp/types/floating-point

kdashg commented 1 year ago

I think:

  • we should leave getImageData as uint8array, and that
  • we should absolutely not block on having float16array, which is non-trivial implementation work according to multiple implementers.

Put another way, we still don't have a core float16 type in C++ and this situation is tolerated by developers. I think the feature is nice-to-have, but not extremely compelling, even in this use case here.

C++ allows sufficiently low-level access that it’s not as necessary to have a core type. C++ devs work around this when necessary by using potentially CPU-specific vector types and operations, and certainly not by representing float16 buffers as unit8 buffers scaled by 255.

If the memory bandwidth story is found to be compelling, I think it would even be tolerable to return a uint16array. Some amount of decode logic is already needed for the uint8array path, since e.g. the decode function is v => v/255.0, so the difference would be in the complexity of the encode/decode func, not a new extra step per se. (bonus points for a toFloat16(Number) js builtin)

That just feels like an obviously goofy workaround. No one would design a system that way if not for expediency.

I appreciate the drive to make it ideal immediately, but unfortunately I don't think all the dependencies are low-hanging-fruit enough. :(

There’s really only one dependency and even the most conservative estimates do not make it all that high-hanging, as they are significantly less than a year. Is that amount of extra speed worth introducing a quirky pitfall into the Web Platform forever?

(I understand there are separate arguments for why always returning uint8 is actually better, but this seems to be solely an expediency-based argument to standardize and ship something a quarter or two sooner.)

Nevertheless my preferences stand, here.

kdashg commented 1 year ago

Put another way, we still don't have a core float16 type in C++ and this situation is tolerated by developers. I think the feature is nice-to-have, but not extremely compelling, even in this use case here.

C++ allows sufficiently low-level access that it’s not as necessary to have a core type. C++ devs work around this when necessary by using potentially CPU-specific vector types and operations, and certainly not by representing float16 buffers as unit8 buffers scaled by 255.

FYI: Support for float16_t type will be coming in C++23. https://en.cppreference.com/w/cpp/types/floating-point

This is not compelling in any direction for me.

kdashg commented 1 year ago

There are two main threads of discussion here, and I think they need two issues, because this is hard to follow.

palemieux commented 1 year ago

@bakkot Quick update. I am working on a simplified explainer re: HDR in HTML Canvas. In the meantime, some background information on why >8 bits per color channel is needed for HDR imagery.

As detailed, for example, at Ultra HD Blu-ray Format Video Characteristics , 8-bit quantization (bit depth) results in contouring and banding, even for traditional standard dynamic range (SDR) imagery, like sRGB, which covers a typical luminance range between 0 and 100 cd/m2. These quantization artifacts become unacceptable with High-Dynamic Range (HDR) imagery, supports luminance ranges between 0 and up to 10,000 cd/m2.

The table below illustrates how larger quantization step size results in visible step sizes, i.e. the gap between successive quantization levels exceeds the threshold of visibility for the human visual system (Barten threshold). The green entries are less than or equal to the Barten threshold, the yellow entries are 2.5 times the Barten threshold, and the red entries are five times the Barten threshold.

In summary, at very least 10 bits per color channels are needed for HDR imagery.

image

@michaeldsmith can provide more information.

bakkot commented 1 year ago

Update: the proposal to add Float16Array to JavaScript today reached stage 3, meaning the design is finished, the committee is in favor, and engines can start implementing and shipping it.

litherum commented 1 year ago

Ideally, while we're here, we should add support for 10-10-10-2 canvas backing store formats (either RGBA or BGRA) so that authors can accomplish wide color without having to use fp16 and doubling the size of their back buffer. We don't have opinions about what the data type returned by getImageData() for such backing stores.

ccameron-chromium commented 1 year ago

Ideally, while we're here, we should add support for 10-10-10-2 canvas backing store formats (either RGBA or BGRA) so that authors can accomplish wide color without having to use fp16 and doubling the size of their back buffer. We don't have opinions about what the data type returned by getImageData() for such backing stores.

That sounds like a great idea. In fact, it might be a better first step to land that (compared with float, which has many more things attached to it). What are your thoughts on the idea of adding:

    enum CanvasPixelFormat {
    "rgba8unorm",
    "rgb10alpha2unorm",
  };
  partial dictionary CanvasRenderingContext2DSettings {
    CanvasPixelFormat pixelFormat = "rgba8unorm";
  };

If that sounds good on its own, I can put up a PR!

I can also add a parameter to ImageDataSettings that allows the user to request a higher precision, if that's something that would be an important complement (I would default to just adding an option for Uint16Array, but that isn't a position that I hold very strongly).

annevk commented 1 year ago

Could we get away with returning the same uint8 view and essentially require bitwise math? I'd like our switch to bigger views to essentially be a switch to f16.

annevk commented 1 year ago

For the high-precision backing buffer, we shouldn't forget about https://github.com/whatwg/html/issues/5173#issuecomment-962045106 from @junov:

let buffer = new Uint8ClampedArray(w*h*4);  // Any type of ArrayBuffer or ArrayBufferView should work
canvas.width = w
canvas.height = h
let ctx = canvas.getContext('2d', {storage: buffer});  // Throws if buffer is too small.

That API shape still looks quite good and addresses a number of shortcomings with the current getImageData() API. (Though not entirely sure about accepting all view types.)

ccameron-chromium commented 1 year ago

I've updated the proposal such that the type returned by getImageData() does not change.

Example usages are:

  // Returns an ImageData with a Uint8ClampedArray
  ctx.getImageData(1, 2, 3, 4);

  // Returns an ImageData with a Uint8ClampedArray
  ctx.getImageData(1, 2, 3, 4, {pixelFormat:'rgba8unorm'});

  // Returns an ImageData with a Float32Array
  ctx.getImageData(1, 2, 3, 4, {colorSpace:'display-p3', pixelFormat:'rgba32float'}); 

I support the idea of subsequently adding ImageData support for rgba16unorm, rgba16float, and (probably) rgb10a2-packed (which would return use a Uint32Array).

Regarding alignment with #5173, it should be emphasized that getImageData() does not ever return the actual backing buffer data from the canvas (ImageData is always tightly packed and always un-premultiplied, whereas almost all canvas backings don't have these properties).

annevk commented 1 year ago

When we had the in-person meeting, my impression was that we only needed one of these APIs going forward, not both.

It certainly seems like a lot of complexity to overload ImageData in this way when it's not quite clear that will be the path most web developers will use going forward.

ccameron-chromium commented 1 year ago

When we had the in-person meeting, my impression was that we only needed one of these APIs going forward, not both.

Sorry for being confused, which two APIs are under consideration?

annevk commented 1 year ago

I thought that instead of getImageData() we were considering just exposing the backing buffer (and making the backing type of that Float16Array so it can handle the new pixel formats).

ccameron-chromium commented 1 year ago

I thought that instead of getImageData() we were considering just exposing the backing buffer (and making the backing type of that Float16Array so it can handle the new pixel formats).

Sorry, I must have misunderstood things during the meeting.

I don't think that it is feasible to directly expose the true backing buffer of a canvas.

The main reason for this is that the true format of a canvas's backing buffer doesn't line up with ImageData in several ways:

The consequence is that if something is going to access the data from a canvas as an ImageData, that data will have gone through some sort of conversion and/or copy.

The getImageData and putImageData functions already support several conversions. The only big deficiency with getImageData is that it is synchronous -- I'd love for there to be a getImageDataAsync function.

annevk commented 1 year ago

Even if it's not a true backing buffer, it would be the new forced-asynchronous way to access pixel data. (Potentially into an existing buffer.) Which would also allow us to switch to Float16Array. I don't think we should be doing both.

ccameron-chromium commented 1 year ago

That hypothetical new function would still incur a copy-and-convert of the data, for the aforementioned reasons.

Are we proposing that we restrict the types of conversions that will be supported for that new function?

E.g getImageData can be used to

If we are to introduce any of these restrictions, what would be the motivation?

annevk commented 1 year ago

I think we only really need to restrict things that we were unhappy with, such as synchronous access. But it might be wise to start out small and only add parity for features when there is clear demand and need.

ccameron-chromium commented 1 year ago

Back in another turn of the wheel of reincarnation, desktop OpenGL supported type conversions. When the "lower level" APIs (starting with OpenGL ES) removed those abilities, it was a big pain point for many developers. Lots of people just wanted to always specify or read data in the format that the rest of their code wanted (often floats), and lamented that TexImage2D and ReadPixels suddenly didn't support conversion anymore.

Having been through that, I wouldn't want introduce restrictions of that sort.

Also, the penalty for removing conversions is higher now, as GPUs are now much faster relative to CPUs than was the case during that previous episode, and removing conversions forces the user to do the conversions themselves, on the CPU.

But I think we're getting a bit far afield. With respect to this proposal, I hope I've made a convincing case that we shouldn't break getImageData.

annevk commented 1 year ago

I don't think anyone is suggesting breaking getImageData though?