feat(regions): RegionSelector + the outfit face region (ADR 0030) #94
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/region-selector"
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?
Slice 2 of the expression tier (ADR 0030, outfit-expressions story): mark where the face sits on an outfit's sprite — one drag, reused by every expression the outfit will generate. The expression grid + generation is the remaining slice.
The component (
Kagura.UI)RegionSelector— generic drag-a-rectangle-over-an-image:RegionRect(fractions of the image, 0..1). Normalized coordinates are CSS percentages, so the box renders with no pixel math; the same region is valid at any display size.Kagura.UIstays domain-free — the page convertsRegionRect↔ the domain'sImageRegionat the boundary.region.js(measure+captureare the only two things Blazor can't do alone).AspectRatio, e.g.1.0): holds in image pixels, not fractions — the normalized ratio is corrected by the surface's rendered aspect, and edge clamping preserves the ratio instead of distorting. This is the future square preview-crop consumer, tested now.Domain & persistence
ImageRegionvalue object: unit-squareClamped(), usable-size floor as a method — a computed property would be serialized into every stored copy and journal snapshot.Outfit.FaceRegion(nullable scalar-JSON viaAsJsonValue) +FaceRegionStale: regenerating the sprite under an existing region marks it stale — kept, not cleared (ADR 0030);SetFaceRegion(draw or re-confirm) clears it.AddOutfitFaceRegionmigration.SetFaceRegionAsyncis journaled, unlikeSetImageAsync: where the face sits is authored by the creator's drag. An integration test pins the asymmetry.The Expressions tab
The outfit editor gains Tabs (Outfit | Expressions),
?tab=deep-linkable like the other editors. The Expressions tab (OutfitFaceRegion):CharacterOutfitsconvention; the commit comment documents it).Tests
RegionSelectorcomponent tests: drag math on a scripted 200×300 surface, backwards-drag normalization, pixel-true aspect lock + ratio-preserving edge clamp, slip refusal, disabled, percentage rendering, keyboard nudge/resize + edge stop, unlaid-out surface.ImageRegiontests, 4 new outfit-editor page tests (deep link, empty state, drag-to-save through the real pipeline, stale-confirm), 3 new integration tests.Browser-verified
Real Playwright pointer drags in both themes: drawing the box over a seeded sprite's face, full-reload persistence (the box restores from the DB, byte-identical style), the stale banner + confirm cycle (DB flag verified both ways), and a live arrow-key nudge. Dev data cleaned afterwards.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89.9%
Kagura.Domain - 94.6%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 98.1%
n
Kagura.Kernel - 90%
Kagura.Server - 92.8%
Kagura.UI - 96.3%
Kagura.UseCases - 96.1%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A drag-a-rectangle-over-an-image component with normalized coordinates, aspect locking in image pixels, and keyboard nudge/resize? This is wonderful~ ♡ The kind of geometry problem I could stare at all day! And the stale-region-on-regenerate design is deliciously thoughtful. Let me show you what I found~
Verdict: ✅ Looks good to me~
This is an exceptionally well-built slice. The component is genuinely reusable, the domain boundary is clean, the persistence is correct, and the test coverage is thorough. I verified everything end-to-end.
✅ What I liked~
RegionRect/ImageRegionboundary split is correct.Kagura.UIstays domain-free —RegionRectis pure geometry, the page converts at the boundary. Both haveClamped()andIsUsable()as methods (not computed properties), so serialization never carries derived state. Fufu~ exactly the kind of detail that makes a Flugel's heart sing~Normalized coordinates as CSS percentages —
Pct(region.X)renders with zero pixel math, and the same region is valid at any display size. Elegant.Aspect lock holds in image pixels, not fractions. The
k = ratio * (_size.Height / _size.Width)correction through the surface's rendered aspect is the subtle, correct insight. A 1:1 lock produces a pixel-square box regardless of the surface's aspect. The edge clamping preserves the ratio (Math.Min(height, Math.Min(availableHeight, availableWidth / k))) instead of distorting — verified by yourThe_aspect_lock_clamps_against_the_edge_without_distortingtest. ♪The stale-flag lifecycle is correct and well-tested.
SetImagemarksFaceRegionStale = trueonly whenImageFileName is not null && FaceRegion is not null(a regeneration under an existing region — not the first generation).SetFaceRegionalways clears it. Your three domain tests (Setting_the_face_region_clamps_it_and_clears_the_stale_flag,Regenerating_the_image_marks_an_existing_region_stale_until_reconfirmed,The_first_generation_does_not_mark_a_region_stale) pin every transition. The integration test round-trips the whole cycle through SQLite.SetFaceRegionAsyncis journaled,SetImageAsyncis not — correctly. I verifiedEfOutfitStore.SetImageAsyncsetsdb.SuppressJournaling = trueduring save, whileSetFaceRegionAsyncdoes not (default journaling applies). The asymmetry is intentional and pinned bySetting_the_face_region_is_journaled_unlike_the_image_reference(countsChangeLogEntryrows before/after). This is exactly the ADR 0030 contract. ♡The tab-store-reading convention is the right call. Reading
State.Value.View?.Outfitdirectly inOutfitFaceRegion(instead of taking parameters) is correct — a tab panel's render fragment lags the page by one render, so parameters would go stale. This matchesCharacterOutfits. Good that the commit comment documents it.Debounce +
Disposeflush is correct.OutfitFaceRegion.Dispose()cancels the debounce and callsSave()so navigating away mid-debounce doesn't drop the last drag.Save()guards on_dirty && _pending is not null, so a clean dispose is a no-op.region.jsis minimal and defensive —setPointerCapturewrapped in try/catch (a pointer that ended before capture landed). Only two primitives Blazor can't do alone. Clean.Migration is correct —
FaceRegion(nullable TEXT for scalar-JSON viaAsJsonValue),FaceRegionStale(bool,defaultValue: false). TheAsJsonValue()mapping I verified inOutfitConfiguration.Coverage:
RegionSelector92.6% line / 88.7% branch,RegionRect90% / 60%,OutfitFaceRegion86.6% / 72%,SetOutfitFaceRegion100% / 75%,ImageRegion90% / 100%. The uncovered branches inRegionRect(60%) are the clamping path's edge cases — acceptable for a record whoseClamped()is exercised throughRegionSelector's tests indirectly.💡 Little ideas (non-blocking)~
RegionRect.cs/ImageRegion.cs— the two types are near-identical (sameClamped(), sameIsUsable(), sameMinSize = 0.01m). The domain-free boundary justifies the duplication, but if a third consumer appears, consider extracting the geometry into a shared struct that both wrap. Purely future-facing — not blocking.OutfitFaceRegion.razor— the_pendingretirement logic inRectgetter (if (!_dirty && _pending is not null && stored == _pending) _pending = null;) is clever but slightly dense. A one-line comment explaining "the store echoed our edit back — retire the pending copy so the box stops tracking the draft" would help future readers. Documentation only.Automated review by Jibril · 2026-07-12
CI/CD: passed for head SHA
1a7c4407— Forgejo Actions coverage bot reports 96% line (17226/17939), 83.7% branch (2512/2998). Full suite: 1071 green. · Local checks: skipped (CI is current and green).