feat(pointer): add an outline tone with a transparent fill and white edge - #353
Merged
Merged
Conversation
…edge Issue #344 point 5: pointer only had filled tones (light/dark), but a walkthrough over a busy screenshot or mockup needs an arrow that marks a spot without covering it — a white outline over a transparent fill. Chose a third PointerTone variant (Outline) over a separate `fill` field. `tone` already exists precisely to let a scene pick one coherent word instead of juggling matched fill/outline hex pairs (see its own doc comment), and `colors()` already dispatches fill+outline from it in one match. A `fill` field would need its own semantics for what "no fill" means, interact awkwardly with `color`/`outline_color`, and reintroduce the two-hex-values problem `tone` was built to avoid. A third arm keeps the existing override mechanism (`color`/`outline_color`) working unchanged: a scenario can still ask for a translucent fill under an outline pointer by setting `color` explicitly. The click ring used to default to the arrow's fill color. For Outline, fill is "transparent" (parses to rgb (0,0,0), alpha only from ring draw time), so with the old default this would render a black ring under a white outline: no hardcoded bug, but not "coherent with the outline style" either. Added `ring_fallback_color`, which threads through the *already resolved* fill/outline strings instead of adding a second hardcoded white: outline tone falls back to `outline`, the two filled tones keep falling back to `fill` exactly as before (so existing light/dark scenarios render byte-identically — pinned by `filled_tones_still_paint_pointer_colour_in_both_interior_and_edge`). Tests measure actual pixels, not field values: an interior point (found via Path::contains with a margin, not hand-picked) must stay background-colored for an outline pointer since its fill paint has alpha 0, while an edge point sitting on the stroke must be opaque and pointer-colored. Confirmed the interior/edge test fails before the fix (temporarily reusing the filled arm's colors for Outline): assertion `left == right` failed, got `(255, 255, 255, 255)` where `(0, 128, 0, 255)` (the background) was expected. A second pixel test pins the click ring default: temporarily hardcoding the fallback to `fill` reproduces a (9, 9, 9, 255) near-black ring against a white outline, which the test catches and which `ring_fallback_color` fixes. Extended pointer-walkthrough.md with the new tone and the ring-default rule. SKILL.md doesn't enumerate PointerTone's values anywhere, so it needs no update for this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #344 — its point 5.
PointerTonewaslight | dark, both filled. A reference video uses a pointer that is a white outline over a transparent fill.PointerTone::Outlineadds it.Why a third tone rather than a
fillfieldtoneexists precisely so a scene picks one coherent word instead of two matched hex values, andcolors()already dispatches fill and outline from a single match arm — a third arm extends that uniformly. A separatefillfield would need its own "no fill" semantics, interact awkwardly with the existingcolor/outline_coloroverrides, and reintroduce exactly the two-hex-values problemtonewas built to avoid. The override mechanism keeps working: a scenario can still setcolorexplicitly for a translucent fill under an outline pointer.A bug found on the way
The click ring defaulted to the arrow's fill colour. Under
Outline, fill is"transparent"— which parses to rgb(0,0,0)with alpha supplied separately at draw time — so the ring would have rendered near-black under a white outline. Not a hard-coded bug, but visually incoherent, and it would have shipped looking like the feature was broken.ring_fallback_color(&self, fill, outline)threads the already-resolved strings:Outlinefalls back tooutline(white, or the author'soutline_coloroverride),LightandDarkkeep falling back tofillexactly as before.Verification
Four tests, all pixel-level:
deep_interior_point_is_well_clear_of_the_outline_stroke— the probe point is found viaPath::containsplus a margin check, not hand-picked coordinates, so the real test cannot pass by sampling the wrong placeoutline_tone_leaves_the_interior_transparent_and_paints_a_pointer_coloured_edge— interior must equal the background exactly(0,128,0,255), edge must be solid whitefilled_tones_still_paint_pointer_colour_in_both_interior_and_edge— pins Light and Dark byte-for-byte, the backward-compatibility guardoutline_tone_keeps_the_click_ring_visible_and_matched_to_the_white_outlineBoth proven to catch the absence:
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1529) all clean.SKILL.mdneeds no change: it referencespointeronly by component name and links topointer-walkthrough.md, which this PR extends. It never enumerated the tone values.Written comment-free, per the codebase-wide rule from #345.