Scroll without resetting the plot, and fix what scrolling exposed - #5
Merged
Conversation
Scrolling one channel threw away every buffer and started over, even
though 31 of 32 rows held exactly the data they had a moment earlier. The
cause was upstream of the reallocation: push_data sliced arriving data to
the visible window, so channels off screen were discarded on arrival and
a channel scrolled into view had no history to show. No amount of
copy-on-scroll fixes that -- the data was never kept.
So storage now spans every channel and the visible window is a view over
it. set_channel_offset stops reallocating: it moves the window, marks the
columns dirty, and leaves the version alone so the MultiLine is not
rebuilt. A channel scrolled into view is already populated, including one
that has never been displayed.
set_n_visible gets the same benefit. It still bumps the version, because
the graphic genuinely changes shape, but that is now a GPU-side rebuild
rather than a data reset -- /2 and x2 come back with their traces
already drawn.
Normalization follows the visible window rather than the whole buffer.
Without that, an off-screen channel ten times larger than the rest would
flatten everything the user is actually looking at.
Cost, measured at 256 channels over a 5 s window with 32 visible:
buffered push/10ms scroll+draw
full rate 30 kHz 158 MB 0.10 ms 0.36 ms
envelope 2 ms bins 9 MB 0.06 ms 0.33 ms
The reduction now runs over all 256 channels rather than the visible 32,
which sounded expensive and is not: 0.10 ms per 10 ms of data is about 1%
of a core. Memory is the real price, and it is why the envelope and this
go together -- 158 MB is defensible, 9 MB is nothing.
Also fixes a bug this work surfaced: _resize_display_dur rebuilt the ring
as (new_total, n_visible), which was both the wrong width and, since the
envelope landed, the wrong rank. Zooming time with the envelope on
mis-sized the buffer. The shape is now derived from the buffer being
replaced.
Nine tests. Four fail if scrolling is put back to reallocating.
Scrolling down now moves the channel window down the list, so the traces travel with the fingers the way a document does. A horizontal trackpad swipe arrives as a wheel event carrying dx with dy at 0, which the old branch read as a direction -- so a sideways swipe walked the channel window. Zero now means stay. Direction is split into a static helper so it can be tested without a canvas: it is obvious in use and invisible in review.
Two things that only show up when you actually drive the plot. Shift+scroll zoomed out whichever way the wheel turned, but only on a mouse. Holding Shift makes the OS report a wheel as horizontal scrolling -- the convention that scrolls a document sideways -- so the motion arrives in dx with dy pinned at 0. Reading dy alone made every notch negative. A trackpad sends both axes, which is why it looked fine there. Trace colour was keyed to the on-screen row, so scrolling repainted every trace in its neighbour's colour -- which defeats the point of a colour now that scrolling no longer resets the plot: the eye cannot follow a channel whose colour changes under it. Colour is now keyed to the absolute channel, via a _channel_color on the base widget shared by the sweep, the spectrum, and the hover tooltip. The tooltip swatch had its own palette and matched neither, so it now agrees with the trace it names. Scrolling rewrites the MultiLine's flat colour buffer in place rather than rebuilding the graphic, so the plot still does not blank.
The hover tooltip named the channel mirrored about the middle of the window: rows are laid out along world-y from the canvas bottom, and it read the channel off as channel_offset + row. That only holds bottom-up, and the sweep defaults to channel_order='top_down'. Event ticks had the same bug from the other direction, so an event on a channel drew its mark on the channel mirrored opposite -- silently, since a tick on a trace looks plausible wherever it lands. Both now go through _channel_at_row / _row_of_channel on the base widget, which is the only place the order is interpreted. The label overlay was already correct and now shares the same source of truth; that fixes the spectrum as a side effect, since it declares no order but offsets its rows bottom-up, and the old getattr default assumed top_down.
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
1 participant
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.
Scrolling the channel window by one used to reallocate every buffer and
blank the plot, so 31 of 32 rows were thrown away and redrawn from
nothing even though their data had not changed.
The cause was upstream of the reallocation:
push_datasliced arrivingdata to the visible window, so off-screen channels were discarded on
arrival. No copy-on-scroll scheme could have worked — a channel
scrolled into view had no history because it was never kept.
Storage now spans every channel and the visible window is a view over
it.
set_channel_offsetmoves the window, marks columns dirty, andleaves the version alone, so the graphic is never rebuilt.
/2andx2keep their history too.
Measured at 256 channels, 5 s, 32 visible:
Memory is the price, and it is why this and the envelope belong
together. Normalization now follows the visible window rather than the
whole buffer, so an off-screen channel ten times larger cannot flatten
what you are looking at.
Fixed on the way
_resize_display_durrebuilt the ring as(new_total, n_visible)—wrong width, and wrong rank since the envelope landed, so time-zoom
with the envelope on mis-sized the buffer.
direction — so a horizontal trackpad swipe walked the channel window.
mouse: holding Shift makes the OS report a wheel as horizontal
scrolling, so the motion arrives in
dxwithdypinned at 0.every trace in its neighbour's colour. Now keyed to the absolute
channel, shared by the sweep, the spectrum, and the hover tooltip —
whose swatch had its own palette and matched neither.
window: rows run up from the canvas bottom, and it read the channel as
channel_offset + row, which only holds bottom-up. The sweep defaultsto
channel_order="top_down". Event ticks had the same bug from theother direction, silently marking the mirrored channel. The spectrum's
labels were flipped for the opposite reason — it declares no order
but lays rows out bottom-up, and the missing attribute defaulted to
top-down.
Row/channel conversion now goes through
_channel_at_row/_row_of_channel, the only place the order is interpreted.Testing
46 tests pass, up from 25. The scroll tests were checked to fail when
set_channel_offsetis reverted to reallocating. The colour uploaditself is GPU-side and untested; its flat-buffer arithmetic — stride,
per-row runs, alpha, the single-visible-channel guard — is covered
against a stub.
🤖 Generated with Claude Code