Skip to content

Visualizer Refactor - #139

Open
RoboDoig wants to merge 123 commits into
mainfrom
visualizer-refactor-staging
Open

Visualizer Refactor#139
RoboDoig wants to merge 123 commits into
mainfrom
visualizer-refactor-staging

Conversation

@RoboDoig

@RoboDoig RoboDoig commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This PR refactors the dynamic foraging visualizers to use a full Bonsai ImGui approach according the the requirements outlined in the visualizer refactor milestone

Addresses issues #99, #100, #101, #102, #103, #104, #105.

Individual visualizers take as input an observable source that describes their parent layout group (e.g. tab area, table window) such that visualizers can be reorganised within a parent layout structure. Generally, data is mapped to visualizers by externalized subject names corresponding to the appropriate data source or event.

RoboDoig added 30 commits June 23, 2026 18:14
Switched to time-based visualization so trial-based no longer needed
@RoboDoig

Copy link
Copy Markdown
Collaborator Author

@micahwoodard - I've reimplemented the visualizer with a shader pass rather than OpenCV transformations in PR #162. I was not able to test this on a camera with frame rate >200Hz but it seemed to eliminate latency problems on the hardware I have. Would you be able to test on your rigs?

@micahwoodard

Copy link
Copy Markdown
Collaborator

Yes will try and test this afternoon

@micahwoodard micahwoodard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tested and seems to be working. Once the ignored files are deleted, I believe this can be merged

@bruno-f-cruz

bruno-f-cruz commented Sep 2, 2026

Copy link
Copy Markdown
Member

@RoboDoig A few more notes while I was reviewing #164.

  1. The CameraPreview include workflow still creates a fair amount of copies (Flip + WarpAffine both allocate). Since you switched everything to a shader-based strategy, you may as well rotate the texture there as well. It comes at nearly 0 cost in terms of performance. And does not have to trigger per incoming frame, on the render frame hook.
  2. In the same CameraPreview It is not clear to me why you need that TakeUntil on the CameraStream
  3. I would avoid hard-coding the name of the cameras in the visualizer of the workflow. There is nothing in the rest of the workflow that guarantees them, since they come from an iterable (Dict in the rig schema). I would maybe consider using the order of the cameras in the dictionary to derive these visualizers (and leave it up to Micah to make sure the fist camera is FaceCamera for instance), but have a dropdown that allows people to pick which camera they want to watch alternatively. I am worried that, later on, people may want to add/modify cameras, and this will require changing code to accommodate.
  4. There are a fair amount of CombineLatest e.g:
image I have not played with bonsai-native imgui workflows that much, but out of curiosity why do you bind them with CombineLatest?

Thanks!

@RoboDoig

RoboDoig commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

@bruno-f-cruz thank you!

Re 4. I don't think you're missing anything - I think Zip would indeed be less overhead. As you say this is just to synchronize branches which is necessary for ordering of groups of elements in bonsai imgui.

@RoboDoig

RoboDoig commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

@bruno-f-cruz @micahwoodard - for rotating the image in shader, is free rotation necessary or is it always done in 90 degree increments? Both possible just much more straightforward for 90 degree increments!

@micahwoodard micahwoodard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I get crashing and memory errors without this take until which is why I added originally.

Unhandled Exception: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at OpenTK.Graphics.OpenGL4.GL.DeleteTextures(Int32 n, Int32& textures) in I:\work\github\opentk\src\OpenTK\Graphics\OpenGL4\GL4.cs:line 16159

@micahwoodard

Copy link
Copy Markdown
Collaborator

@RoboDoig I added back the takeuntil to circumvent the AccessViolationException to be able to continue running a mouse for testing today. We can remove and fix with something else though

@RoboDoig RoboDoig mentioned this pull request Sep 4, 2026
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.

4 participants