Conversation
Handwavy things that need fleshing out are marked with 👋
Kaiido
left a comment
Kaiido
left a comment
There was a problem hiding this comment.
Glad to see this being worked on, thanks.
Not quite sure how much discussion should be held at this stage. So to note, this doesn't seem to fully match the latest state of https://github.com/WICG/html-in-canvas. e.g. the rename to drawHTMLElement. The layoutsubtree attribute is also missing along with the implications to the existing fallback contents.
Still, thanks for making this move.
foolip
changed the title
|
@Kaiido thank you for the review! I've fleshed things out more, renaming to There's still some handwaving going on of course, in particular what causes the subtree to be laid out but not painted. |
foolip
changed the title
|
I've fleshed this out some more now, in particular the hit testing. |
There was a problem hiding this comment.
One common complain with the use of dictionaries in the Canvas2D API is that this makes GC kick in very often during animations which has a non-negligible performance cost.
This API shape makes a big use of such dictionaries with one for the wrapper CanvasElementHitTestRegion and then a nested one for the CanvasHitTestRect, and I guess there will be scenarios where multiple of these will need to be updated at every frame. Since the values are copied over from the passed objects to new internal objects, it's unclear if even a careful author, who would try to reuse the same objects, could avoid GC at all here.
On the other hand, I really like how this API shape enables future additions like using a Path2D, or even a bitmap mask, instead of a CanvasHitTestRect. (btw can we bikeshed on rect for that purpose?)
It's not my area of expertise, but would an actual exposed interface allow for non copy from JS, so that authors can just update the regions instead of setting new ones?
|
|
||
| <div algorithm> | ||
| <p>The <code data-x="dom-context-2d-drawElementImage">drawElementImage(element, sx, sy, sw, sh, | ||
| dx, dy)</code> method, when invoked, must <span>draw an element</span> with <span>this</span>, |
There was a problem hiding this comment.
What if dw or dh are negative. In chrome we are clamping to zero.
drawImage is not explicit but it does say "The destination rectangle is the rectangle whose corners are the four points (dx, dy), (dx+dw, dy), (dx+dw, dy+dh), (dx, dy+dh)." so maybe negative is allowed and mirrors.
Add the 4-arg overload ctx.drawElementImage(el, dx, dy, dwidth, dheight)
on top of TB1b's 2-arg path. The caller specifies destination size in
canvas-grid (backing-store) pixels; the snapshot is scaled to fit.
Pipeline:
* CanvasDrawElementImage.idl gains a second overload with unrestricted
double dwidth/dheight, matching Chrome's canvas_draw_element_image.idl.
unrestricted because non-finite values are handled in C++ as silent
no-ops rather than rejected by the binding layer.
* CanvasRenderingContext2DBase: the existing 2-arg body is refactored
into a private drawElementImageInternal taking std::optional<FloatSize>
for the explicit destination size. The 2-arg public method forwards
with std::nullopt (-> natural boxSize); the new 4-arg public method
forwards with FloatSize { dwidth, dheight } and runs no validation
itself, so eligibility (canvas type, layoutsubtree, parent, snapshot)
is checked BEFORE geometry. This matches Blink's
VerifyDrawElementImageEligibility ordering and ensures a non-canvas
context with NaN dwidth throws InvalidStateError rather than silently
returning identity.
* Replay sequence (4-arg): clip(destRect) -> translate(dx, dy) ->
scale(dwidth/boxSize.w, dheight/boxSize.h) when scaling -> drawDisplayList
-> didDraw. The 2-arg path skips the scale step. For the 4-arg path,
defensive guards skip the divide when boxSize is degenerate (cannot
occur in practice; the recording walk skips non-box children).
Edge cases (4-arg):
* Non-finite dx/dy/dwidth/dheight -> silent no-op, identity matrix.
* Zero or negative dwidth or dheight -> silent no-op, identity matrix.
* No exception thrown on degenerate geometry. Matches Chrome
BaseRenderingContext2D::DrawElementInternal (rect_f.IsEmpty() falls
through to DOMMatrix::Create()). Deliberately diverges from drawImage
9-arg's normalizeRect() mirroring for negative widths because the spec
PR (whatwg/html#11588) is silent and Chrome is the de-facto reference.
Open question for the spec: should negative dwidth/dheight mirror
(drawImage-style) or no-op (drawElementImage / Chrome)? TB2a follows
Chrome.
Returned matrix:
Identity DOMMatrix, same as TB1b. TB3a (issue #8) returns the real
alignment matrix and MUST cover the 4-arg destination-scale case
(scale factor (dwidth/boxSize.w, dheight/boxSize.h) on top of CTM and
the (dx, dy) translation).
Tests:
* drawElementImage-4arg-basic{,-expected}.html — reftest with half-size
(50x50 -> 25x25 at (10,10)) and double-size (50x50 -> 100x100 at
(60,60)) draws on a 200x200 canvas. Expected file uses canvas+fillRect
so test and reference go through the same canvas blit path.
* drawElementImage-4arg-css-vs-backing{,-expected}.html — reftest on a
canvas with backing 400x400 / CSS 200x200 (2x downsample), draws at
(100, 100, 200, 200) in canvas-grid pixels. Confirms that destination
parameters are interpreted as canvas-grid (backing-store) pixels, not
CSS pixels. Reference uses an identically configured canvas with
fillRect to keep the device-pixel comparison filter-equivalent.
* drawElementImage-4arg-edge-cases.html — testharness covering zero,
negative, NaN, +/-Infinity for dwidth/dheight (and non-finite dx/dy).
Asserts no-throw, identity matrix, and unchanged pixels (sentinel
blue fill remains intact). Second subtest creates a main-thread
OffscreenCanvas, calls drawElementImage with NaN dwidth, asserts
InvalidStateError "drawElementImage requires an HTMLCanvasElement
context" — eligibility runs before geometry.
Imported corpus tests for TB2a (scale.html, draw-element-image-scale-
variant.html, basic-rect-zoom.html, percent-sizing.html) remain Skipped
because they depend on infrastructure outside TB2a's scope:
* scale.html / basic-rect-zoom.html / percent-sizing.html call
ResizeObserver entry's devicePixelContentBoxSize, which is not
implemented in WebKit.
* draw-element-image-scale-variant.html (and TB1b's basic-rect.html
etc.) call canvas.requestPaint(), which is TB5 / issue #16.
Both blockers are infrastructure gaps outside TB2a; the destination-
scale path itself is verified by the three native tests above. The Skip
lines stay; each will be removed when its blocker ships.
Out of scope:
* 6/8-arg overloads with source rect (TB2b / issue #7).
* Real alignment matrix return (TB3a / issue #8).
* Full eligibility validator (TB4 / issue #9).
* requestPaint and the paint event (TB5 / issues #10, #16).
* ElementImage / captureElementImage / Worker transfer (TB6/TB7).
* hasInvertibleTransform check on the replay path — also missing in
TB1b 2-arg; belongs in TB4.
Closes #6.
Chrome's gfx::SizeF constructor incidentally clamps any dwidth/dheight input below 8 * FLT_EPSILON (≈ 9.5e-7) to zero, so values like drawElementImage(el, 0, 0, 1e-10, 50) route through the IsEmpty() -> no-op path. The behaviour is an artifact of Chrome's storage type, not spec text — but matching it explicitly keeps the no-op contract byte-identical across engines for visually-degenerate destination sizes. Replace `width <= 0 || height <= 0` with `width < 8.0f * std::numeric_limits<float>::epsilon() || height < ...` in drawElementImageInternal. The new check still subsumes negative, zero, and now sub-epsilon-positive inputs in one branch. Test: drawElementImage-4arg-edge-cases.html gains two subtests for dwidth=1e-10 and dheight=1e-10, confirming no-throw and identity matrix return. The visible-pixel assertion (sentinel blue unchanged) was already passing before this commit because tiny scales produce no observable pixels — but the new subtests pin the API contract so a future regression that *does* leak pixels at sub-epsilon scales is caught. Open question for the spec PR (whatwg/html#11588): the sub-epsilon clamp is undocumented in the explainer and is incidental to Chrome's gfx::SizeF. Worth pinning down in spec text whether sub-epsilon positive dimensions are no-op or drawn; until then, both engines agree by way of this clamp.
9 participants
(See WHATWG Working Mode: Changes for more details.)
/canvas.html ( diff )
/dom.html ( diff )
/index.html ( diff )
/indices.html ( diff )
/infrastructure.html ( diff )
/references.html ( diff )
/rendering.html ( diff )
/webappapis.html ( diff )