Skip to content

Fix LFP Viewer assertion when channels hang off the channel bitmap - #707

Merged
anjaldoshi merged 1 commit into
open-ephys:developmentfrom
adityasingh2400:fix/705-lfp-viewer-negative-overlay-span
Aug 5, 2026
Merged

Fix LFP Viewer assertion when channels hang off the channel bitmap#707
anjaldoshi merged 1 commit into
open-ephys:developmentfrom
adityasingh2400:fix/705-lfp-viewer-negative-overlay-span

Conversation

@adityasingh2400

Copy link
Copy Markdown

Fixes #705.

Since commit afe1d71 made the channel bitmap a viewport-sized window, pxPaint and pxPaintHistory measure each channel from channelBitmapYOrigin rather than from the top of the whole channel stack. The two clamps that bound the channel span were left as they were, and each of them only moves one end of that span: jfrom is raised to 0 and jto is lowered to the last row of the bitmap. A channel sitting above or below the bitmap window therefore ends up with jto < jfrom.

LfpDisplay::refresh paints every channel that overlaps the bitmap window at all, so the channels straddling its top and bottom edges are painted in exactly that state on every refresh. The yellow playhead fillRect and drawEventOverlay then receive a negative height, which trips the jassertquiet in juce_GraphicsContext.cpp:94 and floods the console of a Debug build. That matches the reporter's observation that it prints a few times when the chain is instantiated and then continuously during streaming.

Component height is channelHeight + channelOverlap * overlapFactor with channelOverlap = channelHeight * 2, so the affected band at the top edge is roughly a full channel wide, which is why it is easy to hit.

A negative-height rectangle is already a no-op in JUCE, since Rectangle::isEmpty() covers it, so skipping the draw when the clipped span is empty leaves rendering unchanged and only removes the assertion. The fix is three guards totalling 18 lines, skipping the playhead fillRect in both paint paths and early-returning from drawEventOverlay.

I also corrected the LfpDisplay doc comment, which still described the bitmap as spanning the sum of all channel heights. That stopped being true with afe1d71 and now contradicts the code.

Added a regression test that scrolls a 16 channel stack at 120 px per channel past a 400 px viewport and captures stderr, where Logger::outputDebugString sends the assertion. On development it fails with eight assertions reading JUCE Assertion failure in juce_GraphicsContext.cpp:94, which is character for character the message in the report. With this change the full LfpViewer suite is green at 9 of 9 on a Debug macOS build, with zero JUCE assertions anywhere in the run. git clang-format --diff reports no modifications.

Disclosure: this change was prepared with AI assistance. I have reviewed and tested it.

Since the channel bitmap became a viewport-sized window, pxPaint() and
pxPaintHistory() measure each channel from channelBitmapYOrigin rather
than from the top of the whole channel stack. The two clamps that bound
the channel span were left as they were, and each of them only moves one
end of that span: jfrom is raised to 0 and jto is lowered to the last row
of the bitmap. A channel sitting above or below the bitmap window
therefore ends up with jto < jfrom.

LfpDisplay::refresh paints every channel that overlaps the bitmap window
at all, so the channels straddling its top and bottom edges are painted
in exactly that state on every refresh. The yellow playhead fillRect and
drawEventOverlay then receive a negative height, which trips the
jassertquiet in juce_GraphicsContext.cpp and floods the console of a
Debug build.

A negative-height rectangle is already a no-op in JUCE, so skipping the
draw when the clipped span is empty leaves rendering unchanged and only
removes the assertion. Also corrects the LfpDisplay doc comment, which
still described the bitmap as spanning the sum of all channel heights.

Adds a regression test that scrolls a tall channel stack past the
viewport and asserts that no JUCE assertion reaches stderr.

Fixes open-ephys#705
@anjaldoshi

Copy link
Copy Markdown
Member

Hi @adityasingh2400,

Thanks for putting together this PR! I managed to replicate the #705 issue reported by @ckemere after trying to adjust the channel height while the LFP Viewer was busy plotting. Initially, I was only testing with 16 channel data, which perfectly fit within the entire bitmap in my setup, so I couldn’t reproduce it. However, after increasing the channel height, it hit the edge case you mentioned when plotting the event overlay rectangle with a negative height. The fix and test seem reasonable (and thanks for disclosing the use of AI!). Once all the checks are passed, I’ll go ahead and merge it.

Since juce::drawRect doesn’t actually do anything with negative width and height values, and the bug doesn’t impact production code, we won’t create a patch release right away. We’ll definitely include this fix in the next patch release once we’re all set. Until then, I recommend using the development branch (once this is merged) for running debug builds.

@adityasingh2400

Copy link
Copy Markdown
Author

Thanks for taking the time to actually reproduce it, and for the release reasoning. Increasing the channel height is exactly the shortcut I ended up using too, since it is the quickest way to push a channel span past the bitmap edge without needing a large probe.

Your read on the impact matches mine. fillRect and drawRect treat a negative extent as nothing to draw, so in a release build this is invisible. It only bites in debug, where the jassert in the Rectangle path fires. That is why the fix is guards at the call sites rather than anything that changes what gets painted, no visible pixel differs before and after.

Checks are at 4 of 5 green right now, with Integration Tests still running and nothing failing. I will keep an eye on it and sort out anything that comes back red.

@adityasingh2400

Copy link
Copy Markdown
Author

Following up on my note above, all five checks came back green: build-ubuntu, build-windows, build-osx, Unit Tests, and Integration Tests. The branch is mergeable with no conflicts, so nothing is outstanding on my side whenever you are ready.

@anjaldoshi
anjaldoshi self-requested a review August 5, 2026 22:00
@anjaldoshi

Copy link
Copy Markdown
Member

Just gave it a final local test, and it's looking good. Thanks again for the PR!

@anjaldoshi
anjaldoshi merged commit b5bfc87 into open-ephys:development Aug 5, 2026
5 checks passed
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