From c044253fc1481f192fd3d42f452ac0456527c818 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 26 Oct 2021 17:49:24 -0700 Subject: [PATCH 1/2] Support setting the background color for sensors Signed-off-by: Nate Koenig --- .../ignition/gazebo/rendering/RenderUtil.hh | 6 +- src/rendering/RenderUtil.cc | 37 +++++-- src/systems/sensors/Sensors.cc | 10 ++ src/systems/sensors/Sensors.hh | 11 +- test/integration/CMakeLists.txt | 1 + test/integration/camera_sensor_background.cc | 104 ++++++++++++++++++ test/worlds/camera_sensor_empty_scene.sdf | 55 +++++++++ 7 files changed, 213 insertions(+), 11 deletions(-) create mode 100644 test/integration/camera_sensor_background.cc create mode 100644 test/worlds/camera_sensor_empty_scene.sdf diff --git a/include/ignition/gazebo/rendering/RenderUtil.hh b/include/ignition/gazebo/rendering/RenderUtil.hh index 90912f4431..7cc824a632 100644 --- a/include/ignition/gazebo/rendering/RenderUtil.hh +++ b/include/ignition/gazebo/rendering/RenderUtil.hh @@ -86,11 +86,13 @@ inline namespace IGNITION_GAZEBO_VERSION_NAMESPACE { /// \return Name of the rendering scene. public: std::string SceneName() const; - /// \brief Set background color of render window + /// \brief Set background color of render window. This will override + /// other sources, such as from SDF. /// \param[in] _color Color of render window background public: void SetBackgroundColor(const math::Color &_color); - /// \brief Set ambient light of render window + /// \brief Set ambient light of render window. This will override + /// other sources, such as from SDF. /// \param[in] _ambient Color of ambient light public: void SetAmbientLight(const math::Color &_ambient); diff --git a/src/rendering/RenderUtil.cc b/src/rendering/RenderUtil.cc index 43c2ca676c..6e347d01bc 100644 --- a/src/rendering/RenderUtil.cc +++ b/src/rendering/RenderUtil.cc @@ -104,11 +104,15 @@ class ignition::gazebo::RenderUtilPrivate /// \brief Name of scene public: std::string sceneName = "scene"; - /// \brief Scene background color - public: math::Color backgroundColor = math::Color::Black; + /// \brief Scene background color. This is optional because a is + /// always present, which has a default background color value. This + /// backgroundColor variable is used to override the value. + public: std::optional backgroundColor; - /// \brief Ambient color - public: math::Color ambientLight = math::Color(1.0, 1.0, 1.0, 1.0); + /// \brief Ambient color. This is optional because an is always + /// present, which has a default ambient light value. This ambientLight + /// variable is used to override the value. + public: std::optional ambientLight; /// \brief Scene manager public: SceneManager sceneManager; @@ -355,8 +359,16 @@ void RenderUtil::Update() // extend the sensor system to support mutliple scenes in the future for (auto &scene : newScenes) { - this->dataPtr->scene->SetAmbientLight(scene.Ambient()); - this->dataPtr->scene->SetBackgroundColor(scene.Background()); + // Only set the ambient color if the RenderUtil::SetBackgroundColor + // was not called. + if (!this->dataPtr->ambientLight) + this->dataPtr->scene->SetAmbientLight(scene.Ambient()); + + // Only set the background color if the RenderUtil::SetBackgroundColor + // was not called. + if (!this->dataPtr->backgroundColor) + this->dataPtr->scene->SetBackgroundColor(scene.Background()); + if (scene.Grid() && !this->dataPtr->enableSensors) this->ShowGrid(); // only one scene so break @@ -1203,8 +1215,17 @@ void RenderUtil::Init() this->dataPtr->engine->CreateScene(this->dataPtr->sceneName); if (this->dataPtr->scene) { - this->dataPtr->scene->SetAmbientLight(this->dataPtr->ambientLight); - this->dataPtr->scene->SetBackgroundColor(this->dataPtr->backgroundColor); + if (this->dataPtr->ambientLight) + { + this->dataPtr->scene->SetAmbientLight( + *this->dataPtr->ambientLight); + } + + if (this->dataPtr->backgroundColor) + { + this->dataPtr->scene->SetBackgroundColor( + *this->dataPtr->backgroundColor); + } } } this->dataPtr->sceneManager.SetScene(this->dataPtr->scene); diff --git a/src/systems/sensors/Sensors.cc b/src/systems/sensors/Sensors.cc index 1568820551..157220098e 100644 --- a/src/systems/sensors/Sensors.cc +++ b/src/systems/sensors/Sensors.cc @@ -164,6 +164,9 @@ class ignition::gazebo::systems::SensorsPrivate /// \brief Stop the rendering thread public: void Stop(); + + /// \brief Use to optionally set the background color. + public: std::optional backgroundColor; }; ////////////////////////////////////////////////// @@ -184,6 +187,8 @@ void SensorsPrivate::WaitForInit() { // Only initialize if there are rendering sensors igndbg << "Initializing render context" << std::endl; + if (this->backgroundColor) + this->renderUtil.SetBackgroundColor(*this->backgroundColor); this->renderUtil.Init(); this->scene = this->renderUtil.Scene(); this->initialized = true; @@ -331,10 +336,15 @@ void Sensors::Configure(const Entity &/*_id*/, EventManager &_eventMgr) { igndbg << "Configuring Sensors system" << std::endl; + // Setup rendering std::string engineName = _sdf->Get("render_engine", "ogre2").first; + // Get the background color, if specified. + if (_sdf->HasElement("background_color")) + this->dataPtr->backgroundColor = _sdf->Get("background_color"); + this->dataPtr->renderUtil.SetEngineName(engineName); this->dataPtr->renderUtil.SetEnableSensors(true, std::bind(&Sensors::CreateSensor, this, diff --git a/src/systems/sensors/Sensors.hh b/src/systems/sensors/Sensors.hh index 1e0602644e..b2d236e8ed 100644 --- a/src/systems/sensors/Sensors.hh +++ b/src/systems/sensors/Sensors.hh @@ -36,7 +36,16 @@ namespace systems class SensorsPrivate; /// \class Sensors Sensors.hh ignition/gazebo/systems/Sensors.hh - /// \brief TODO(louise) Have one system for all sensors, or one per + /// \brief A system that manages sensors. + /// + /// ## System Parameters + /// + /// - `` Name of the render engine, such as 'ogre' or 'ogre2'. + /// - `` Color used for the scene background. This + /// will override the background color specified in a world's SDF + /// element. This background color is used by sensors, not the GUI. + /// + /// \TODO(louise) Have one system for all sensors, or one per /// sensor / sensor type? class Sensors: public System, diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index afcd29cfb4..1aebf4bd2d 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -52,6 +52,7 @@ set(tests # Tests that require a valid display set(tests_needing_display + camera_sensor_background.cc camera_video_record_system.cc depth_camera.cc gpu_lidar.cc diff --git a/test/integration/camera_sensor_background.cc b/test/integration/camera_sensor_background.cc new file mode 100644 index 0000000000..48a872b6e4 --- /dev/null +++ b/test/integration/camera_sensor_background.cc @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2021 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include + +#include + +#include +#include + +#include "ignition/gazebo/Server.hh" +#include "ignition/gazebo/SystemLoader.hh" +#include "ignition/gazebo/test_config.hh" + +#include "../helpers/EnvTestFixture.hh" + +using namespace ignition; +using namespace std::chrono_literals; + +std::mutex mutex; +int cbCount = 0; + +////////////////////////////////////////////////// +class CameraSensorBackgroundFixture : + public InternalFixture> +{ +}; + +///////////////////////////////////////////////// +void cameraCb(const msgs::Image & _msg) +{ + ASSERT_EQ(msgs::PixelFormatType::RGB_INT8, + _msg.pixel_format_type()); + + for (unsigned int y = 0; y < _msg.height(); ++y) + { + for (unsigned int x = 0; x < _msg.width(); ++x) + { + // The "/test/worlds/camera_sensor_empty_scene.sdf" world has set a + // background color of 1,0,0,1. So, all the pixels returned by the + // camera should be red. + unsigned char r = _msg.data()[y * _msg.step() + x*3]; + EXPECT_EQ(255, static_cast(r)); + + unsigned char g = _msg.data()[y * _msg.step() + x*3+1]; + EXPECT_EQ(0, static_cast(g)); + + unsigned char b = _msg.data()[y * _msg.step() + x*3+2]; + EXPECT_EQ(0, static_cast(b)); + } + } + std::lock_guard lock(mutex); + cbCount++; +} + +///////////////////////////////////////////////// +// Test the ability to set the background color using the sensor system +// plugin. +TEST_F(CameraSensorBackgroundFixture, + IGN_UTILS_TEST_DISABLED_ON_MAC(RedBackground)) +{ + // Start server + gazebo::ServerConfig serverConfig; + const auto sdfFile = std::string(PROJECT_SOURCE_PATH) + + "/test/worlds/camera_sensor_empty_scene.sdf"; + serverConfig.SetSdfFile(sdfFile); + + gazebo::Server server(serverConfig); + EXPECT_FALSE(server.Running()); + EXPECT_FALSE(*server.Running(0)); + + // subscribe to the camera topic + transport::Node node; + cbCount = 0; + node.Subscribe("/camera", &cameraCb); + + // Run server and verify that we are receiving a message + // from the depth camera + server.Run(true, 100, false); + + int i = 0; + while (i < 100 && cbCount <= 0) + { + common::Time::Sleep(common::Time(0.1)); + i++; + } + + std::lock_guard lock(mutex); + EXPECT_GE(cbCount, 1); +} diff --git a/test/worlds/camera_sensor_empty_scene.sdf b/test/worlds/camera_sensor_empty_scene.sdf new file mode 100644 index 0000000000..1f5db3260a --- /dev/null +++ b/test/worlds/camera_sensor_empty_scene.sdf @@ -0,0 +1,55 @@ + + + + + .001 + 1.0 + + + + + ogre2 + 1 0 0 1 + + + + true + 0 0 1.0 0 0 0 + + + + + 0.1 0.1 0.1 + + + + + + + 0.1 0.1 0.1 + + + + + + 1.047 + + 320 + 240 + + + 0.1 + 100 + + + 30 + camera + + + + + From 5ca8a7aba179b97d1fc2c453ec2eb6c9bc124544 Mon Sep 17 00:00:00 2001 From: Nate Koenig Date: Tue, 26 Oct 2021 18:05:13 -0700 Subject: [PATCH 2/2] Added ambient light Signed-off-by: Nate Koenig --- src/systems/sensors/Sensors.cc | 9 +++++++++ src/systems/sensors/Sensors.hh | 5 ++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/systems/sensors/Sensors.cc b/src/systems/sensors/Sensors.cc index 157220098e..217f57829c 100644 --- a/src/systems/sensors/Sensors.cc +++ b/src/systems/sensors/Sensors.cc @@ -167,6 +167,9 @@ class ignition::gazebo::systems::SensorsPrivate /// \brief Use to optionally set the background color. public: std::optional backgroundColor; + + /// \brief Use to optionally set the ambient light. + public: std::optional ambientLight; }; ////////////////////////////////////////////////// @@ -189,6 +192,8 @@ void SensorsPrivate::WaitForInit() igndbg << "Initializing render context" << std::endl; if (this->backgroundColor) this->renderUtil.SetBackgroundColor(*this->backgroundColor); + if (this->ambientLight) + this->renderUtil.SetAmbientLight(*this->ambientLight); this->renderUtil.Init(); this->scene = this->renderUtil.Scene(); this->initialized = true; @@ -345,6 +350,10 @@ void Sensors::Configure(const Entity &/*_id*/, if (_sdf->HasElement("background_color")) this->dataPtr->backgroundColor = _sdf->Get("background_color"); + // Get the ambient light, if specified. + if (_sdf->HasElement("ambient_light")) + this->dataPtr->ambientLight = _sdf->Get("ambient_light"); + this->dataPtr->renderUtil.SetEngineName(engineName); this->dataPtr->renderUtil.SetEnableSensors(true, std::bind(&Sensors::CreateSensor, this, diff --git a/src/systems/sensors/Sensors.hh b/src/systems/sensors/Sensors.hh index b2d236e8ed..a4d3c17cfe 100644 --- a/src/systems/sensors/Sensors.hh +++ b/src/systems/sensors/Sensors.hh @@ -41,9 +41,12 @@ namespace systems /// ## System Parameters /// /// - `` Name of the render engine, such as 'ogre' or 'ogre2'. - /// - `` Color used for the scene background. This + /// - `` Color used for the scene's background. This /// will override the background color specified in a world's SDF /// element. This background color is used by sensors, not the GUI. + /// - `` Color used for the scene's ambient light. This + /// will override the ambient value specified in a world's SDF + /// element. This ambient light is used by sensors, not the GUI. /// /// \TODO(louise) Have one system for all sensors, or one per /// sensor / sensor type?