GUI option to change render rate #3127

WilliamLewww · 2021-10-25T22:32:55Z

The effect is not easily seen due to the choppiness of the GIF, but the render rate switches between 62 -> 15.

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

scpeters

merging with gazebo11 should fix the ABI checker

also, I think you could add the render rate to the GUI section of the ModelListWidget on the left-hand side of the screen by adding some code to ModelListWidget::FillUserCamera, since that method already has a pointer to the user camera. If you do this, then you could remove the new code for RenderOptionsWindow, so you can ignore any comments I made about those files if you end up removing the code

gazebo/gui/RenderOptionsWindowPrivate.hh

gazebo/gui/RenderOptionsWindow.hh

gazebo/gui/RenderOptionsWindowPrivate.hh

gazebo/rendering/UserCamera.hh

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

WilliamLewww · 2021-10-26T20:43:16Z

merging with gazebo11 should fix the ABI checker

also, I think you could add the render rate to the GUI section of the ModelListWidget on the left-hand side of the screen by adding some code to ModelListWidget::FillUserCamera, since that method already has a pointer to the user camera. If you do this, then you could remove the new code for RenderOptionsWindow, so you can ignore any comments I made about those files if you end up removing the code

Changed the position of the render rate tool.

* Fix the expectations in UNIT_ModelListWidtet_TEST to match the new layout. * Remove a console error message that is currently printed whenever clicking on a non-clip Camera field. * Clean up whitespace * Share changedProperty variable across logic blocks. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-10-27T00:31:01Z

nice work! I noticed a test failure and weird console error message while testing, so I've fixed those in a6a827d

scpeters

I've noticed a few more things; I will fix them myself in another commit

scpeters · 2021-10-27T05:11:58Z

gazebo/gui/ModelListWidget.hh

@@ -24,6 +24,7 @@
 #include <ignition/msgs/plugin_v.pb.h>

 #include "gazebo/gui/qt.h"
+#include "gazebo/gui/RenderWidget.hh"


I think we could use a forward declaration a few lines below as class RenderWidget; instead of including this header file

scpeters · 2021-10-27T05:12:55Z

gazebo/gui/ModelListWidget.hh

@@ -44,6 +45,8 @@ namespace gazebo
      public: explicit ModelListWidget(QWidget *_parent = 0);
      public: virtual ~ModelListWidget();

+      public: void ConnectRenderWidget(RenderWidget* renderWidget);


add doc-string for the new function, even though most others don't have them

scpeters · 2021-10-27T05:13:12Z

gazebo/gui/ModelListWidgetPrivate.hh

@@ -132,6 +133,8 @@ namespace gazebo

      /// \brief Node for ignition transport communication.
      public: ignition::transport::Node ignNode;
+
+      public: RenderWidget* renderWidget;


add doc-string for the new member variable, even though some others don't have them

scpeters · 2021-10-27T05:16:56Z

gazebo/gui/ModelListWidget.cc

+    if (cam)
+    {
+      cam->SetRenderRate(this->dataPtr->variantManager->value(
+          this->ChildItem(cameraRenderRateProperty, "render rate")).toDouble());


nit: this value is duplicated; instead use a variable to store this value and reference it on this line and in the statement below

double renderRate = this->dataPtr->variantManager->value( this->ChildItem(cameraRenderRateProperty, "render rate")).toDouble();

Shouldn't the property name be set to render_rate as everything else in these menus is using this type of notation?
@scpeters @WilliamLewww

* doc-strings for new function and member variable * de-duplicate code accessing render rate * use forward declaration instead of header include Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-10-27T18:08:03Z

Mark just asked how feasible it would be to specify the GUI render rate from the <gui> element in a world file, and as I started thinking about how to implement it, it occurs to me that perhaps we should create a GUI event for changing the render rate instead of adding the ModelListWidget::ConnectRenderWidget method.

what do you think @iche033 ?

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-10-27T18:50:30Z

Mark just asked how feasible it would be to specify the GUI render rate from the <gui> element in a world file, and as I started thinking about how to implement it, it occurs to me that perhaps we should create a GUI event for changing the render rate instead of adding the ModelListWidget::ConnectRenderWidget method.

what do you think @iche033 ?

I've implemented the approach using a GUI event in 595f235

iche033 · 2021-10-27T19:00:29Z

I've implemented the approach using a GUI event in 595f235

yep, looks good to me.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-10-27T21:35:14Z

I used invokeMethod in e9e162e at @iche033's suggestion to address some threading issues I found while testing an approach to loading the render rate from a world file

scpeters

I approve, though I also made a few changes to it

scpeters · 2021-10-27T23:14:02Z

I'm not going to try reading the render rate from a world file in this PR

iche033

works for me.

HighPriest · 2021-12-07T01:15:05Z

Is usage of this option documented somewhere? How can it be updated, other than manually in the GUI?
Can this property be set in a world file?
e.g

<gui>
  <camera>
    <render_rate>30</render_rate>
  </camera>
</gui>

It would be very useful, when running on a target machine, to reduce gzclient impact on performance.

scpeters · 2021-12-14T01:34:57Z

Is usage of this option documented somewhere? How can it be updated, other than manually in the GUI? Can this property be set in a world file? e.g
<gui>
  <camera>
    <render_rate>30</render_rate>
  </camera>
</gui>
It would be very useful, when running on a target machine, to reduce gzclient impact on performance.

there is very simple code to set render rate from SDFormat, but it requires a change to the gui_camera protobuf message, which breaks ABI, so it can't be released in gazebo11 (see scpeters@07ceabf).

an alternative approach could be made that creates an ignition-transport service to allow gzclient to query gzserver for the render_rate value supplied in the SDFormat world file, similar to the approach in 42f7e48. This was more complex, and I did not have time to implement this when working on this pull request

WilliamLewww added 5 commits October 7, 2021 13:54

Added render options to main menu

eed4d8e

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

Added gui controls for render rate

0f12072

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

Added render rate update in GLWidget

429f998

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

Fixed render rate change in GUI

45907ea

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

Added comments to render rate methods

142aa95

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

scpeters reviewed Oct 26, 2021

View reviewed changes

gazebo/gui/RenderOptionsWindowPrivate.hh Outdated Show resolved Hide resolved

gazebo/gui/RenderOptionsWindow.hh Outdated Show resolved Hide resolved

gazebo/gui/RenderOptionsWindowPrivate.hh Outdated Show resolved Hide resolved

gazebo/rendering/UserCamera.hh Outdated Show resolved Hide resolved

Merge branch 'osrf:gazebo11' into wlew/render_options_2

228f982

WilliamLewww force-pushed the wlew/render_options_2 branch from d2700ec to fd7f550 Compare October 26, 2021 20:21

Moved render rate option to model list widget

5745d2e

Signed-off-by: William Lew <WilliamMilesLew@gmail.com>

WilliamLewww force-pushed the wlew/render_options_2 branch from fd7f550 to 5745d2e Compare October 26, 2021 20:22

scpeters reviewed Oct 27, 2021

View reviewed changes

Add doc-strings and clean up some code

ef876fd

* doc-strings for new function and member variable * de-duplicate code accessing render rate * use forward declaration instead of header include Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Add GUI event instead of connecting widgets

595f235

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Use invokeMethod to fix threading issues

e9e162e

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters approved these changes Oct 27, 2021

View reviewed changes

scpeters changed the title ~~Added new GUI option to change render rate.~~ GUI option to change render rate Oct 28, 2021

iche033 approved these changes Oct 28, 2021

View reviewed changes

scpeters merged commit 6941a39 into gazebosim:gazebo11 Oct 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GUI option to change render rate #3127

GUI option to change render rate #3127

WilliamLewww commented Oct 25, 2021

scpeters left a comment

WilliamLewww commented Oct 26, 2021

scpeters commented Oct 27, 2021

scpeters left a comment

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

scpeters Oct 27, 2021

HighPriest Dec 7, 2021

scpeters commented Oct 27, 2021

scpeters commented Oct 27, 2021

iche033 commented Oct 27, 2021

scpeters commented Oct 27, 2021

scpeters left a comment

scpeters commented Oct 27, 2021

iche033 left a comment

HighPriest commented Dec 7, 2021

scpeters commented Dec 14, 2021

GUI option to change render rate #3127

GUI option to change render rate #3127

Conversation

WilliamLewww commented Oct 25, 2021

scpeters left a comment

Choose a reason for hiding this comment

WilliamLewww commented Oct 26, 2021

scpeters commented Oct 27, 2021

scpeters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scpeters commented Oct 27, 2021

scpeters commented Oct 27, 2021

iche033 commented Oct 27, 2021

scpeters commented Oct 27, 2021

scpeters left a comment

Choose a reason for hiding this comment

scpeters commented Oct 27, 2021

iche033 left a comment

Choose a reason for hiding this comment

HighPriest commented Dec 7, 2021

scpeters commented Dec 14, 2021