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

clean up render window resize logic #716

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from
Open

Conversation

flynneva
Copy link

cleaned up some of the logic for when rviz render window gets resized.

this should make it easier to diagnose whats going on over at #1508

@@ -152,18 +154,18 @@ ToString(const EnumType & enumValue)
return QString("%1::%2").arg(enumName).arg(static_cast<int>(enumValue));
}

void RenderWindow::resizeEvent(QResizeEvent * resize_event)
Copy link
Contributor

Choose a reason for hiding this comment

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

unused parameter

Copy link
Author

Choose a reason for hiding this comment

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

so this is a virtual function from the parent QWindow. https://doc.qt.io/qt-5/qwindow.html#resizeEvent

we dont need to use the event param passed in

Copy link
Member

Choose a reason for hiding this comment

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

To avoid an unused parameter warning, we can use Q_UNUSED or just write (void)resize_event.

@@ -260,13 +260,8 @@ RenderWindowImpl::resize(size_t width, size_t height)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

width and height are unused

RenderWindowImpl::resize(size_t /*width*/, size_t /*height*/)

auto width = parent_->width();
// auto width = ogre_render_window_->getWidth() ? ogre_render_window_->getWidth() : 100;
auto height = parent_->height();
// auto height = ogre_render_window_->getHeight() ? ogre_render_window_->getHeight() : 100;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just remove these commented lines? I'm not sure why they were there to begin with.

…oud/point_cloud_common.cpp

Co-authored-by: Jacob Perron <jacob@openrobotics.org>
@flynneva
Copy link
Author

flynneva commented Jul 15, 2021

to give some context: we are running into an issue rendering pointclouds in rviz and rviz2 related to the render window height.

this PR is a result of me doing a deep dive over a few days to investigate that issue. To be clear this PR does not solve that open issue.

@jacobperron jacobperron self-assigned this Aug 5, 2021
@jacobperron
Copy link
Member

@flynneva I've tested the changes locally, and didn't notice any change in behavior so I'm happy to accept this change. Could you resolve the outstanding comments above and rebase on ros2 before I trigger CI?

@flynneva
Copy link
Author

flynneva commented Mar 1, 2022

@jacobperron sure! I'll try and get to this today

@audrow audrow changed the base branch from ros2 to rolling June 28, 2022 14:24
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.

None yet

3 participants