whatwg / html

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

canvas2D setTransform and doubles #5657

Open Kaiido opened 4 years ago

Kaiido commented 4 years ago

The specs text define CanvasTransform.setTransform() as accepting "unrestricted double".
This corresponds to what the DOMMatrix interface also supports which is great.

Implementers seem to follow this definition too (FF, Chrome), which is also great...
... except that a few lines later they do clamp it to float anyway.

This means that if we use a double that doesn't fit as a float in a DOMMatrix, set the canvas transform using this DOMMatrix and get back the canvas' current DOMMatrix calling getTransform(), we receive a DOMMatrix with clamped values (live fiddle).

const mat = new DOMMatrix();
mat.a = 16777215.123;
ctx.setTransform( mat );
ctx.getTransform(); // -> { a: 16777215, ...

While not a big deal, as an user it's quite surprising (I am coming here after a StackOverflow question popped out).

I am not sure what should be done here, but I decided to open the issue directly here because the implementations actually, somehow follow the specs.
What I think would be acceptable changes from the top of my head:

cc @whatwg/canvas

domenic commented 4 years ago

I think the spec doesn't really allow this, since it has no step which clamps to float. Maybe it allows it in the sense that it doesn't really define "multiplication", so we could pretend this happens as part of the multiplication operation... but IMO we have a spec/implementation mismatch here.

I think

Changing the definition of CanvasTransform.setTransform() so it accepts floats directly

makes sense, unless implementers want to stop clamping to float. /cc @whatwg/canvas @mysteryDate

shengzheng1981 commented 4 years ago

wow. check this situation: use canvas to render a map (projection is web mecator), so when zoom is 19-20(256 * Math.pow(2, zoom)), means matrix(e,f) always exceed float, float is not enough!

shengzheng1981 commented 4 years ago

This is a good issue, check my repo: https://github.com/shengzheng1981/green-gis-js.

nornagon commented 1 year ago

Yeah, I'm also encountering this issue. Doing the transform "by hand" in JS retains double precision all the way through to line drawing, whereas using .scale() and friends causes odd glitches at the limits of single-precision FP accuracy.

kdashg commented 3 months ago

Implementation of full f64 here, in light of our increasing reliance on GPU-based canvas engines, is unfortunately relatively onerous. f32 is really pretty good for most use-cases here, and I think our preference would be to change the spec to be clear that these are f32 in practice. Yes, sorry that f32 is not enough for some use-cases, but I think that in such cases, there's enough else going on that polyfilling with your own Number/f64 matrix is not an unreasonable request, sorry!

annevk commented 3 months ago

Thanks @kdashg for updating this issue. WebKit's perspective is also to stick with f32. Updating the API seems reasonable, modulo the usual web compatibility caveats.

@Kaiido are you interested in addressing this?

Kaiido commented 3 months ago

I could try when I get time. However I'm not quite sure how to handle the setTransform(DOMMatrix) case, I guess this can't be tackled from WebIDL only, right?

annevk commented 3 months ago

Yeah, I'm not sure what @chrishtr and @dirkschulze would prefer. Either they provide some kind of opt-in so https://drafts.fxtf.org/geometry/ returns a f32 matrix for our use case or we clamp the f64 matrix afterwards ourselves. If we are the only f32-interested caller I guess it's on us. We already have some logic to discard matrixes with elements that are Infinite or NaN so that doesn't seem too bad.

(We should also clean setTransform(transform) up so it doesn't seem like we use a DOMMatrix internally. Perhaps by reusing the wording setTransform(a, b, c, d, e, f) uses in its last step.)

annevk commented 3 months ago

I might have spoken too soon here. WebKit does support f64 currently as far as I can tell. This will need more discussion then.

chrishtr commented 2 months ago

I might have spoken too soon here. WebKit does support f64 currently as far as I can tell. This will need more discussion then.

Blink also has double-accuracy transform matrices internally. However, SkMatrix and all other representations within Skia or our GPU rendering code uses float accuracy.

The Chrome graphics experts I've talked to say that for the purposes of graphics output, float is always sufficient, and that AAA games for example pretty much always get away with using floats internally as a result (which is good, because it uses less memory and is supported by all GPUs).

I also found one blink-dev thread that suggests double is important at least for the purposes of some engine-internal coordinate space transformations, to avoid loss of precision.

kdashg commented 2 months ago

While we do occasionally run into f32 precision limitations in Firefox for layout (like extremely long log pages, tens of megabytes+), like @chrishtr says about f32 being enough for [on-screen] games, I think the needs of canvas2d are fairly well-enough served by f32.

Kaiido commented 2 months ago

Blink also has double-accuracy transform matrices internally.

Though the canvas method itself seem to clamp, even before doing any actual transform right?
Anne is correct that Safari does return the passed double precision number when going through setTransform() -> getTransform(), Chrome and Firefox don't.