Skip to content

fix: preserve sprite aspect ratio in menu bar and popover - #194

Merged
chattymin merged 1 commit into
chattymin:mainfrom
Noahbanderson:fix/menu-bar-sprite-aspect-ratio
Aug 20, 2026
Merged

chattymin merged 1 commit into
chattymin:mainfrom
Noahbanderson:fix/menu-bar-sprite-aspect-ratio

Conversation

@Noahbanderson

@Noahbanderson Noahbanderson commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Gen-V animated sprites are stretched to fill a square frame, so any species whose
GIF canvas is not square renders distorted. Spoink (#325) has a 36x66 canvas and
comes out 1.83x too wide in the menu bar, the home card, and the evolution line.

Two render paths force the square:

  • SpriteView — .resizable().frame(width: size, height: size) with no aspect mode
  • AppDelegate.menuBarImage — sprite.draw(in: NSRect(width: h - 2, height: h - 2))
Cursor 2026-08-19 09 54 31 PokeTokenBar 2026-08-19 09 54 44

Worth calling out why this stayed hidden, since it shapes the fix: every static
asset is already square (species PNGs 96x96, item PNGs 30x30), so stretch-to-fill
was a no-op on the static path — the only path any test measured. The distortion
could only ever appear on the animated path. Other real canvases: Pikachu #25 is
50x46, Gengar #143 is 74x75.

This routes every render site through a single SpriteFit.size(for:box:):

  • SpriteView sizes the inner image to fit and keeps the outer size x size
    slot. The outer slot matters — EvoLineView.rowWidth and the dex grid assume
    square cells, so shrinking the outer frame would shift those layouts.
  • menuBarLayout keeps the canvas 22pt tall so the baseline cannot jitter
    between frames, and tracks the fitted width. A fixed 22x22 canvas would pad a
    tall sprite with ~5pt of dead space per side, pushing the usage number away from
    it. Square sources keep the exact previous geometry (22x22 canvas, 20x20 at x=1),
    and the bob offset is unchanged.
  • ItemIconView gets the same treatment. Today's item art is square, but it is
    externally sourced, so non-square input is reachable.

A zero-sized source (undecodable image) falls back to the square box rather than
dividing by zero.

Per the defect protocol in CLAUDE.md, the class is recorded in
docs/reference/defect-log.md under a new "render geometry" section, including the
"static assets are all square, so only the GIF path shows it" reasoning that made
this hard to catch.

image

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • Other:

UI changes

Menu bar status item, home card sprite, evolution line thumbnails, and dex grid
thumbnails. Only non-square (animated Gen-V) sprites change; square sources render
byte-identically to before.

Before After
Spoink's 36x66 GIF stretched to fill a square: the body is visibly wide and flat, the pearl smeared horizontally. Spoink renders at its source ratio — narrow and tall, pearl round.

Checklist

  • swift build and swift test pass locally
  • PR title and description are written in English
  • UI changes are described above (before/after — images optional)
  • No copyrighted assets, secrets, or private tooling references are committed
  • Tests were added or updated for this change

Test notes

SpriteAspectRatioTests (10 cases) uses the real PokeAPI canvas dimensions above and
states the trigger explicitly — a non-square source must not render square — so the
suite cannot pass by exercising square inputs alone. Guards were verified by injecting
the old square-stretch behavior into SpriteFit.size: 4 fail, including the trigger.

Full suite on this branch: 713 tests, 1 failure. The failure is
LocalAdditionalUsageTests.testHermesAcceptsMillisecondStartedAt, which reproduces
identically on a clean main checkout and is unrelated to this change — it looks
timezone-dependent (expects 2026-01-02, gets 2026-01-01). Untouched here; happy to
open a separate issue if useful.

🤖 Generated with Claude Code

Gen-V animated sprites have per-species cropped canvases that are not square
(Spoink chattymin#325 is 36x66, Pikachu chattymin#25 is 50x46, Gengar chattymin#143 is 74x75), but both
render paths forced them into a square box with no aspect mode: SpriteView used
.resizable().frame(width: size, height: size), and menuBarImage drew into
NSRect(width: h - 2, height: h - 2). Spoink came out 1.83x too wide.

The defect stayed hidden because every static asset is already square (species
PNGs 96x96, item PNGs 30x30), so stretch-to-fill was a no-op on the static path
-- the only path any test measured. It could only surface on the animated path.

Route every render site through one SpriteFit.size(for:box:):

- SpriteView sizes the inner image to fit and keeps the outer size x size slot,
  because EvoLineView.rowWidth and the dex grid assume square cells.
- menuBarLayout keeps the canvas 22pt tall so the baseline cannot jitter, and
  tracks the fitted width -- a fixed 22x22 canvas would pad a tall sprite with
  dead space that pushes the usage number away from it. Square sources keep the
  exact previous geometry.
- ItemIconView gets the same treatment; item art is externally sourced, so
  non-square input is reachable even though today's items are square.

SpriteAspectRatioTests uses real PokeAPI canvas dimensions and asserts the
trigger explicitly -- a non-square source must not render square -- so the suite
cannot pass by exercising square inputs alone. Verified by injecting the old
square-stretch behavior: 4 guards fail, including the trigger.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Noahbanderson
Noahbanderson force-pushed the fix/menu-bar-sprite-aspect-ratio branch from 0b347db to c5ce754 Compare August 19, 2026 15:21
@chattymin
chattymin merged commit bd0bba9 into chattymin:main Aug 20, 2026
2 checks passed
@chattymin

Copy link
Copy Markdown
Owner

Thank you for your contribution!

LuceteYang added a commit to LuceteYang/PikaTokenBar that referenced this pull request Aug 21, 2026
Brings in the representative-Pokémon pin (chattymin#158), Antigravity 2.0 multi-root
discovery (chattymin#182), Codex rollout memory bounding (chattymin#184), and fixes for Codex
usage after session archiving (chattymin#181), the Kiro rescan (chattymin#178), the AppLog
willTerminate flush (chattymin#174), the update-loop PID wait (chattymin#175), the popover
outside-click monitor (chattymin#168), and menu bar/popover sprite aspect ratio (chattymin#194).
All 11 are scope- and identity-agnostic, so nothing is rejected.

Conflict resolutions:
- PokeTokenBarApp / FloatingPetPanel: the menu bar and pet now read the
  representative subject AND the animation-quality frame floor. The floor stays
  in the frame-cache identity key and in the observation set, so pinning a
  species and changing FPS both take effect immediately.
- UpdateChecker: took upstream's PID-based exit wait and testable launcher
  signature, kept the fork's cask token and login-agent label via AppIdentity.
  The fork guard that asserted `pgrep -x <executable>` now asserts the PID wait,
  since upstream deliberately replaced that mechanism.
- UsageStore: kept the fork's async notification-authorization request, which
  avoids the Swift 6 executor trap the closure form hits.
- CLAUDE.md: kept the fork-maintenance row alongside upstream's expanded
  defect-log row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lukemonaghan pushed a commit to lukemonaghan/PokeTokenBar that referenced this pull request Aug 24, 2026
)

Gen-V animated sprites have per-species cropped canvases that are not square
(Spoink chattymin#325 is 36x66, Pikachu CallumAcorn#25 is 50x46, Gengar chattymin#143 is 74x75), but both
render paths forced them into a square box with no aspect mode: SpriteView used
.resizable().frame(width: size, height: size), and menuBarImage drew into
NSRect(width: h - 2, height: h - 2). Spoink came out 1.83x too wide.

The defect stayed hidden because every static asset is already square (species
PNGs 96x96, item PNGs 30x30), so stretch-to-fill was a no-op on the static path
-- the only path any test measured. It could only surface on the animated path.

Route every render site through one SpriteFit.size(for:box:):

- SpriteView sizes the inner image to fit and keeps the outer size x size slot,
  because EvoLineView.rowWidth and the dex grid assume square cells.
- menuBarLayout keeps the canvas 22pt tall so the baseline cannot jitter, and
  tracks the fitted width -- a fixed 22x22 canvas would pad a tall sprite with
  dead space that pushes the usage number away from it. Square sources keep the
  exact previous geometry.
- ItemIconView gets the same treatment; item art is externally sourced, so
  non-square input is reachable even though today's items are square.

SpriteAspectRatioTests uses real PokeAPI canvas dimensions and asserts the
trigger explicitly -- a non-square source must not render square -- so the suite
cannot pass by exercising square inputs alone. Verified by injecting the old
square-stretch behavior: 4 guards fail, including the trigger.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants