Skip to content

perf(atlas_viewer): cache sorted sprite list (#156)#185

Open
apotema wants to merge 1 commit into
mainfrom
feat/156-atlas-viewer-cache
Open

perf(atlas_viewer): cache sorted sprite list (#156)#185
apotema wants to merge 1 commit into
mainfrom
feat/156-atlas-viewer-cache

Conversation

@apotema

@apotema apotema commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes one of the #156 nits — atlas_viewer.zig sprite-list per-frame sort.

renderSpriteList was rebuilding + sorting an ArrayList of frame keys every frame the Atlas Viewer was open — N allocs + O(N log N) per frame for atlases with hundreds of entries (flying-platform's characters.json has 100+).

Atlas.sorted_frame_keys (added via #180) already carries the alpha-ordered view, built once at load. Iterate that directly. Net change: +3 / −16, zero per-frame allocations on this render path.

Drive-by:

  • renderSpriteList's app parameter was only there for the allocator; with the per-frame ArrayList gone it's unused, so it's dropped from the signature + call site.
  • The orphaned lessName comparator is removed.

Test plan

  • `zig build` — clean
  • `zig build test` — 411/411 pass
  • CI: Build, ImGui Test Engine, Launch Smoke, Bugbot
  • Manual: open Atlas Viewer (View → Atlas Viewer), pick an atlas, sprite list still renders alpha-sorted, scroll + click still selects, sprite detail panel still populates.

… per frame (#156)

`renderSpriteList` was rebuilding + sorting an ArrayList of frame
keys every frame the Atlas Viewer was open — N allocs + an O(N log N)
sort per frame for atlases with hundreds of entries (flying-platform's
`characters.json` has 100+).

`Atlas.sorted_frame_keys` (added via #180) already holds the alpha-
ordered key view, built once at load. Iterate that directly.

Drive-by: `renderSpriteList`'s `app` parameter was only there for the
allocator; with the per-frame ArrayList gone the parameter is unused,
so it's dropped from both the signature and the call site. The
now-orphaned `lessName` comparator is removed.
@cursor

cursor Bot commented Jun 2, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
UI-only render-path optimization with no change to atlas loading, selection, or data ownership; behavior should stay alpha-sorted as before.

Overview
The Atlas Viewer sprite list no longer rebuilds and alphabetically sorts frame names on every UI frame. It now walks Atlas.sorted_frame_keys, which is built once when the atlas loads, so large atlases avoid repeated allocations and O(N log N) work while the panel is open.

renderSpriteList drops the unused app argument (it was only needed for the temporary list’s allocator), and the local lessName comparator is removed because sorting happens at load time.

Reviewed by Cursor Bugbot for commit 5d4aaab. Bugbot is set up for automated code reviews on this repo. Configure here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the renderSpriteList function in src/modules/atlas_viewer.zig by utilizing a pre-sorted list of frame keys (cur.sorted_frame_keys) instead of dynamically allocating, collecting, and sorting the frame names on every render call. This removes the dependency on the App allocator and the helper function lessName. There are no review comments, so no additional feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the Atlas Viewer’s per-frame sprite list rendering by eliminating a rebuild+sort of frame names each frame and instead iterating the atlas’s precomputed sorted_frame_keys, removing per-frame allocations and redundant O(N log N) work.

Changes:

  • Switch sprite list rendering to iterate Atlas.sorted_frame_keys directly (pre-sorted at atlas load time).
  • Remove the now-unneeded per-frame ArrayList construction/sort and drop the unused app parameter from renderSpriteList.
  • Delete the orphaned lessName comparator.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants