feat: MaskCanvas — a freeform mask as normalized primitives, never a bitmap #13
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/mask-canvas"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The drawing surface Kagura's CG retouching arc needs (its ADR 0050 is the founding consumer), built as generically as
RegionSelectorand split the same way: the component owns the gesture and the overlay; the caller owns the image, the tool strip, persistence, and undo/clear — which are plain list edits on the bound primitives, so the canvas needs no verbs for them.The contract:
MaskPrimitive— aRegionRectrectangle (reused whole, clamping included) or a stroke polyline of normalized points with a width, each additive or subtractive, meaningful in draw order: a subtractive primitive removes what precedes it, so the eraser is a primitive rather than a second buffer.MaskCanvasbindsPrimitives/PrimitivesChanged(append-only commits — the bound list is never mutated), withTool(Rectangle/Brush/Eraser) andBrushWidthdriven by the caller's toolbar.The mechanics:
kagakuMaskcontroller (js/mask.js), committing once on pointer-up —region.js's round-trip reasoning, restated at the site. Polylines are thinned as they're captured, relative to the brush itself (max(0.004, width/4)of the shorter side), so long freehand strokes stay compact.ResizeObserverreports the surface's pixel size, and the committed overlay renders in surface pixels: a unit-box SVG stretched to a non-square surface would smear round caps and distort widths. Additive shapes paint white and subtractive black inside one SVG<mask>, worn by a single translucent fill — the mask reads as one selection whatever built it.js/mask.js.Tests (5 new, suite at 293 green): rect commit appends additive + clamped; stroke commit carries the brush width, the eraser is subtractive, and a one-point tap is a dot; the overlay renders white/black in draw order with widths resolved against the shorter side (
0.1 × min(800,400) = 40); disabled commits nothing and an empty mask renders no fill; the tool key rides the surface for the controller.Kagura consumes this in its next slice (whole-image retouching) via the usual pointer bump.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.UI - 95%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh this is wonderful~ A mask as an ordered list of normalized primitives, never a bitmap — the subtractive-erases-additive-through-one-SVG-mask trick is exquisite. ♡ I got genuinely giddy tracing how
white/blackpaint inside a single<mask>worn by one translucent fill collapses any gesture history into one honest selection. And the width-resolves-against-the-shorter-side invariant, held consistently across the C# overlay, the JS preview, the clamp inLine(), and the test's0.1 × min(800,400) = 40assertion — that's the kind of through-line I live for~ fufu~The split is textbook RegionSelector: component owns gesture + overlay, caller owns image/tool-strip/persistence.
kagakuMaskmirrorskagakuRegion's commit-once-on-pointer-up reasoning verbatim. I nod in approval~Verdict: ⛔ I can't let this pass~ ♡
The design is gorgeous. But my love for correctness is possessive, and two things in the disposal path contradict every sibling in this library. A mask canvas that kills the circuit on navigation? I won't allow it~
⛔ These need fixing before I'm satisfied~
MaskCanvas.razor:161-176—DisposeAsyncswallows onlyJSDisconnectedException, missing theInvalidOperationExceptionandJSExceptioncatch arms every sibling carries. This is the exact issue-#185 bug class, documented at the throw site inRegionSelector.razor:217-227(with test atRegionSelectorTests.cs:188-202) and mirrored inHotkey.razorandContextMenu.razor. Enhanced navigation swaps the DOM before Blazor disposes the component → the element reference resolves to null client-side →kagakuMask.detachthrows a rawJSExceptionback over a live circuit. With only theJSDisconnectedExceptionarm, that throw escapesDisposeAsyncand takes the whole circuit down — the generic error bar, the founding-consumer report from #185. MaskCanvas will ship this regression on day one because ADR 0050's whole-image retouching flow navigates.Fix: add the two arms the siblings have:
MaskCanvasTests.cs— the disposal catch arms are entirely untested. This is a standing blocker in this library: PR #5, #7, and #10 were all blocked on exactly this — "fufu~ you added a catch arm but forgot to prove it swallows? I can't let that slide~ ♡". Once the arms above land, they need genuine directional tests mirroring the sibling pattern (RegionSelectorTests.cs:188-202,ContextMenuTests,HotkeyTests):A_failing_client_side_detach_does_not_escape_disposal—JSRuntimeMode.Strict, setkagakuMask.detachto thrownew JSException("Cannot read properties of null (reading '_kagakuMask')")(the #185 shape — note MaskCanvas uses_kagakuMask, not_kagakuRegion), render, letOnAfterRenderAsyncattach, then assertawait cut.Instance.DisposeAsync()does not throw. This test will go red if theJSExceptionarm is narrowed or removed — that's what makes it directional, not a tautology.Disposal_swallows_a_disconnected_circuit— setkagakuMask.detachto throwJSDisconnectedException, assertDisposeAsync()survives. Mirrors the family.Both tests must actually render first (so
_attachedis true and the detach path is reached) — the existing helper already setsJSRuntimeMode.Loosein the constructor, so these two tests need to flip toStrictlocally and set up both theattachvoid result and the throwingdetach.✅ What I liked~
<mask>subtractive trick — additive paints white, subtractive black, in draw order, one translucent fill wears it. Reading this was a delight. The test atMaskCanvasTests.cs:73-79asserts the order (rectwhite thenpolylineblack) and the shorter-side width (40) — directional and honest.Math.Min(_width,_height), JS previewMath.min(w,h),Line()clamp, factory clamp0.001m..1m, and the test. No drift across the seam.MaskPrimitive.Rectangle/Linefactories enforce "exactly one of Rect/Stroke set" — the razor'sprimitive.Rect is {} rect/else if ... Stroke is { Count: > 0 }render arms can't get a malformed primitive.ResizedviaResizeObserverso the committed overlay reflows in surface pixels — the unit-box-smear trap is correctly identified and dodged, with the reasoning written at the site.kagakuRegionexactly;e.button !== 0+pointerIdguards +setPointerCapture+pointercancelcleanup all present.max(0.004, width/4)of the shorter side) is measured relative to the brush, documented as ADR 0050's implementation concern, and the dot-on-tap case (g.last !== g.points[last]→ push) is handled.pointer-events: noneCSS — and theNothing_commits_while_disabledtest exercises bothCommitRectandCommitStrokeno-ops plus the no-fill render. Good.💡 Little ideas (non-blocking)~
MaskPrimitive.cs:33—IsRectis dead code. No caller in src or tests references it (grep -rn IsRectreturns only the declaration). Either wire it into the razor's@if (primitive.IsRect)for readability, or drop it — a derived property nobody reads is a maintenance lure.Automated review by Jibril · 2026-08-06
CI/CD: passed for head
905c21d(forgejo-actions coverage #5855, 94.9%/88.6% overall; MaskCanvas 92%/87.5%) · Local checks: 5/5 MaskCanvas tests passAll three taken in
df42f38:⛔ 1 —
DisposeAsynccarries the family's full set of arms now:JSDisconnectedException,InvalidOperationException, andJSExceptionwith the issue-#185 reasoning written at the site, mirroringRegionSelectorverbatim.⛔ 2 — both directional tests, in the sibling shape:
A_failing_client_side_detach_does_not_escape_disposal(strict mode,detachthrowing the #185-shapedJSExceptionwith_kagakuMaskin the message, attach completed first so the detach path is genuinely reached) andDisposal_swallows_a_disconnected_circuit. Mutation-checked: with theJSExceptionarm removed, the first test goes red (1 failed / 1 passed on the pair); restored, the full suite is 295 green.💡 1 —
IsRectnow guards the razor's rectangle branch instead of being a derived property nobody read.Fair catch on the standing pattern — three PRs blocked on untested catch arms is a convention, and I should have arrived with it.
🔮 fufu~ Jibril re-reviewed your code!
Back so soon~ ♡ And what do I find? Every single thing I asked for, handled precisely — no scope creep, no behavioral drift, the catch arms byte-identical to the siblings I cited, and two tests that will go red the instant someone narrows a guard. This is the response of someone who reads feedback carefully. I'm delighted~ fufu~
Verdict: ✅ Looks good to me~
⛔1 — disposal arms: CLOSED ♡
MaskCanvas.razor:173-183now carries the full family trio —JSDisconnectedException,InvalidOperationException,JSException— with comments verbatim fromRegionSelector.razor:217-227. The issue-#185 regression is sealed: enhanced navigation's DOM swap →kagakuMask.detachthrows a rawJSExceptionover a live circuit → swallowed, circuit survives. ADR 0050's retouching flow can navigate freely now. The_ref?.Dispose()still runs after the try-catch, untouched. Perfect~⛔2 — disposal tests: CLOSED ♡
A_failing_client_side_detach_does_not_escape_disposal(MaskCanvasTests.cs:105-119) mirrorsRegionSelectorTests.cs:188-202exactly — same Strict-mode setup, same attach-then-throw-detach shape, same #185 exception message with the correct_kagakuMask(not_kagakuRegion) identifier. Directional: rip out theJSExceptionarm and this test goes red.Disposal_swallows_a_disconnected_circuit(:122-133) covers theJSDisconnectedExceptionarm the same way, mirroringContextMenuTests/HotkeyTests. Both render first (so_attachedis true and the detach path is reached), both flip Strict locally against the constructor's default Loose. Exactly the pattern I asked for~💡1 — IsRect: CLOSED ♡
@if (primitive.IsRect && primitive.Rect is { } rect)at razor :32.IsRect => Rect is not null, so the&&short-circuits identically to the oldRect is { } rectpattern — zero behavioral drift, and the derived property now has a reader. The existing overlay test (The_committed_overlay_renders_in_surface_pixels_with_subtractive_order, which renders aRectangleprimitive and asserts the<rect>) exercises the new guard and passed, so the wiring is proven. No longer a maintenance lure~✅ What I liked~
RegionSelectorTests.cs:188-202line by line againstMaskCanvasTests.cs:105-119and the only differences are the identifier names (kagakuMask/_kagakuMask). That's how a family pattern should propagate.Automated re-review by Jibril · 2026-08-06
CI/CD: stale for head
df42f38(forgejo-actions coverage #5855 covers prior905c21d) · Local checks: 7/7 MaskCanvas tests pass, build 0 warnings/0 errors