Fix index out of bounds panic in keyboard rendering#121
Closed
Copilot wants to merge 4 commits into
Closed
Conversation
Replace direct array indexing `colors[i]` with bounds-checked `colors.get(i).copied().flatten()` to handle the case where the keyboard layout's visible range (0..257) exceeds the colors array length (256). Agent-Logs-Url: https://github.com/BlackMIDIDevs/wasabi/sessions/fd310b1e-3b5f-414b-b875-cdfeb59777b2 Co-authored-by: Kaydax <11893096+Kaydax@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
Kaydax
April 5, 2026 18:58
View session
All key/column/buffer arrays throughout the pipeline now match the keyboard layout's 257 keys (indices 0-256). This replaces the bounds-check workaround with properly sized data structures. Changes: - MIDI data: ram/parse, live/view, live/parse/notes, cake/tree_threader - GPU rendering: note_list_system buffers, cake_system BUFFER_ARRAY_LEN - GLSL shaders: key_positions[257], buffers[257] - Keyboard drawing: direct indexing, fallback colors vec![None; 257] Agent-Logs-Url: https://github.com/BlackMIDIDevs/wasabi/sessions/3254dffb-93f0-4223-8aaf-ba59ec352a99 Co-authored-by: Kaydax <11893096+Kaydax@users.noreply.github.com>
The keyboard layout arrays had an extra phantom key 256 that didn't correspond to any MIDI key. This was the root cause of the index-out-of-bounds issue when visible_range included index 256. Changes (all in keyboard_layout.rs): - Reduce keys/notes arrays from [; 257] to [; 256] - Reduce initialization loops from 0..257 to 0..256 - Clamp visible_range to max 256 with bounds checks - Keep load_key_numbers at 257 entries (Classic layout lookahead) Reverts all incorrect 256->257 expansions from previous commit. Agent-Logs-Url: https://github.com/BlackMIDIDevs/wasabi/sessions/1cf1b20a-e425-454a-95a4-5e8d0cd21326 Co-authored-by: Kaydax <11893096+Kaydax@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes an index-out-of-bounds panic in keyboard rendering by aligning KeyboardLayout’s internal key/note arrays to the actual MIDI key domain (0–255) and ensuring visible ranges never exceed that bound.
Changes:
- Shrinks
KeyboardLayoutstorage from 257 to 256 entries and updates iteration bounds accordingly. - Adds guards/clamping in view range computation to prevent
right_key - 1underflow and to keepvisible_rangewithin0..=256(exclusive end). - Updates
iter_all_keys/iter_all_notesto iterate0..256.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
42
to
46
| pub struct KeyboardLayout { | ||
| keys: [KeyPosition; 257], | ||
| notes: [KeyPosition; 257], | ||
| keys: [KeyPosition; 256], | ||
| notes: [KeyPosition; 256], | ||
| } | ||
|
|
There was a problem hiding this comment.
There are multiple hard-coded 256 values in this module (array sizes, loop bounds, clamp/guards). To reduce the chance of future off-by-one mismatches, consider introducing a single const KEY_COUNT: usize = 256 (and KEY_COUNT_F: f32 if needed) and using it consistently for bounds, unwrap_or, and min() calls.
Agent-Logs-Url: https://github.com/BlackMIDIDevs/wasabi/sessions/8d054582-0544-4164-a703-4e16e2741689 Co-authored-by: Kaydax <11893096+Kaydax@users.noreply.github.com>
Member
|
Yah idk this code is pretty shit. You have let me down claude |
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.
Uh oh!
There was an error while loading. Please reload this page.