Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory access in case of 3-byte pixel formats #1519

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

rhaschke
Copy link
Contributor

This fixes an invalid memory access found in #1508 (comment):

While the memory array allocated for Ogre::PixelBox is considering the pixel format size here:

delete[](uint8_t*) dst_box.data;
dst_box = Ogre::PixelBox(render_w, render_h, 1, format, data);

the unpacking routine always assumes that the pixel format is 4 bytes here:

uint32_t pos = (x + y * w) * 4;
uint32_t pix_val = *(uint32_t*)((uint8_t*)box.data + pos);
uint32_t handle = colorToHandle(box.format, pix_val);

This is failing if the pixel format is 3 bytes only...
Unfortunately. PixelBox::getColourAt() transforms the bytes into float values, which we transform back subsequently to bytes... That's a lot more overhead 😢

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. I don't know enough about this part of the code or this part of Ogre's API to know if there's a more efficient way to do it. It seems like it is just necessary to do this, regardless of the performance issues.

src/rviz/selection/selection_manager.cpp Outdated Show resolved Hide resolved
@rhaschke rhaschke merged commit e5ef587 into ros-visualization:melodic-devel Jun 23, 2020
@rhaschke rhaschke deleted the fix-memory-access branch June 23, 2020 14:55
seanyen pushed a commit to seanyen/rviz that referenced this pull request Oct 16, 2020
seanyen pushed a commit to seanyen/rviz that referenced this pull request Oct 16, 2020
seanyen referenced this pull request in ms-iot/rviz Oct 16, 2020
)

Co-authored-by: Wolf Vollprecht <w.vollprecht@gmail.com>
seanyen referenced this pull request in ms-iot/rviz Oct 16, 2020
)

Co-authored-by: Wolf Vollprecht <w.vollprecht@gmail.com>
UniBwTAS pushed a commit to UniBwTAS/rviz that referenced this pull request Jun 7, 2021
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