Skip to content

Fix ZED wrapper correctness and optimization issues - #301

Open
danielryckman wants to merge 6 commits into
mainfrom
dr/zed-fixes
Open

Fix ZED wrapper correctness and optimization issues#301
danielryckman wants to merge 6 commits into
mainfrom
dr/zed-fixes

Conversation

@danielryckman

Copy link
Copy Markdown
Contributor

Summary

Closes #(Your issue number here)
Not tested on rover since it hasn't been rebuilt, will test once done

What features did you add, bugs did you fix, etc?
Fixed bug where ZED wouldn't stream raw frames if depth was disabled. Optimized parts of the critical section to retrieve ZED frame data at a faster rate. Replaced calls to std::swap() with performant class method swap(), saving redundant calls to move assignment operator and move constructor. Added comments documenting what was changed.

Did you add documentation to the wiki?

No, overall structure did not change. Modifications are commented in the source code.

How was this code tested?

Ran individual binaries ros2 run mrover zed and ros2 topic echo and ros2 topic hz to verify the topics were publishing properly. Ran camera streamer locally to verify everything worked properly. Ran stereo object detector as a test to confirm it still works.

Did you test this in sim?

No, does not apply

Did you test this on the rover?

No, rover has not been rebuilt yet. Will test once it has.

Did you add unit tests?

No, ran different parts as detailed in testing section to ensure it worked.

…lish raw image data if depth_mode=NONE. Replaced std::swap() and move/copy operators with custom performant swap(). Decreased size of critical section for faster ZED frame retrieval. Added comments for better documentation.
Comment thread perception/zed_wrapper/zed_wrapper.bridge.cu
Comment thread perception/zed_wrapper/zed_wrapper.cpp Outdated
@qhdwight

Copy link
Copy Markdown
Contributor

saving redundant calls to move assignment operator and move constructor

Is this actually measurably slower? It should only be touching pointers which are cheap to edit. Having to move around the entire image would be actually slow.

@danielryckman

danielryckman commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Is this actually measurably slower? It should only be touching pointers which are cheap to edit. Having to move around the entire image would be actually slow.

I haven't measured it (and it probably isn't much faster). Regardless, each call in the current wrapper to std::swap() results in 1 call to move constructor and 2+1 calls to the move assignment operator resulting in three times the amount of calls to sl::Mat::swap() than necessary. I feel like a custom swap() function that only calls sl::Mat::swap() on each matrix once seems both syntactically and functionally cleaner.

@MyCabbages4 MyCabbages4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

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