Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Oct 22, 2025

🦟 Bug fix

Summary

This PR fixes an issue where a static environment loaded by the EnvironmentPreload system would not display using the Environment Visualization Resolution and Point Cloud GUI widgets.

There is also a fix that prevents segmentation faults in the VisualizationTool caused by copying a DataFrame which may invalidate the session iterators.

Details

Before this change, if the <ignore_time> element is added to the EnvironmentPreload system xml it is ignored when visualising the point cloud data.

    <plugin
      filename="gz-sim-environment-preload-system"
      name="gz::sim::systems::EnvironmentPreload">
      <data>environmental_data.csv</data>
      <dimensions>
        <ignore_time>1</ignore_time>
        <time>timestamp</time>
        <space>
          <x>x</x>
          <y>y</y>
          <z>z</z>
        </space>
      </dimensions>
    </plugin>

This means that if only the time zero data is provided, for example:

# environmental_data.csv - modified to only contain data for timestamp = 0
timestamp,humidity,x,y,z
0,80,-1,-1,-1
0,80,-1,-1, 1
0,80,-1, 1,-1
0,80,-1, 1, 1
0,90, 1,-1,-1
0,90, 1,-1, 1
0,90, 1, 1,-1
0,90, 1, 1, 1

the VisualizationTool would never display the point cloud data.

A workaround for this is to set a very large timestamp for a second copy of data. This is unsatisfactory because it doubles the size of the environment data required (which may be significant for large vector fields), and also requires a temporal interpolation when none is required, which results in a performance penalty.

The other fix is to pay attention to unintended copies when using the auto keyword to obtain aliases to the data frame objects. The main change is

- auto frame = _data->frame[this->pubs.begin()->first];
+ const auto &frame = _data->frame[this->pubs.begin()->first];

The problem is that the session iterators will in general be invalid for the copied frame, and this may result in a segmentation fault when they are dereferenced later in the code. In this particular case the check that the iterator was in range in the interpolation code failed resulting in a out of range access violation a few lines later. This type of error can be difficult to trace.

With this change, and some other related fixes to the environmental data system, large static vector fields can be loaded and viewed in the GUI. Below is an example for a wind field containing 64000 rows of data.

05-1-gz-wind-pc-x-axis

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Remove this if GenAI was not used.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.


This is an automatic backport of pull request #3138 done by Mergify.

## Summary

This PR fixes an issue where a static environment loaded by the EnvironmentPreload system would not display using the Environment Visualization Resolution and Point Cloud GUI widgets.

There is also a fix that prevents segmentation faults in the VisualizationTool caused by copying a DataFrame which may invalidate the session iterators.

## Details

Before this change, if the `<ignore_time>` element is added to the EnvironmentPreload system xml it is ignored when visualising the point cloud data.

```xml
    <plugin
      filename="gz-sim-environment-preload-system"
      name="gz::sim::systems::EnvironmentPreload">
      <data>environmental_data.csv</data>
      <dimensions>
        <ignore_time>1</ignore_time>
        <time>timestamp</time>
        <space>
          <x>x</x>
          <y>y</y>
          <z>z</z>
        </space>
      </dimensions>
    </plugin>
```

This means that if only the time zero data is provided, for example:

```bash
# environmental_data.csv - modified to only contain data for timestamp = 0
timestamp,humidity,x,y,z
0,80,-1,-1,-1
0,80,-1,-1, 1
0,80,-1, 1,-1
0,80,-1, 1, 1
0,90, 1,-1,-1
0,90, 1,-1, 1
0,90, 1, 1,-1
0,90, 1, 1, 1
```

the VisualizationTool would never display the point cloud data.

A workaround for this is to set a very large timestamp for a second copy of data. This is unsatisfactory because it doubles the size of the environment data required (which may be significant for large vector fields), and also requires a temporal interpolation when none is required, which results in a performance penalty.

The other fix is to pay attention to unintended copies when using the auto keyword to obtain aliases to the data frame objects. The main change is

```diff
- auto frame = _data->frame[this->pubs.begin()->first];
+ const auto &frame = _data->frame[this->pubs.begin()->first];
```

The problem is that the session iterators will in general be invalid for the copied frame, and this may result in a segmentation fault when they are dereferenced later in the code. In this particular case the check that the iterator was in range in the interpolation code failed resulting in a out of range access violation a few lines later. This type of error can be difficult to trace.

With this change, and some other related fixes to the environmental data system, large static vector fields can be loaded and viewed in the GUI. Below is an example for a wind field containing 64000 rows of data.

<img width="1312" height="940" alt="05-1-gz-wind-pc-x-axis" src="https://github.com/user-attachments/assets/72d9bd99-3640-456d-975a-9fc2d6387884" />

---------

Signed-off-by: Rhys Mainwaring <[email protected]>
(cherry picked from commit bddeeea)
@mergify mergify bot requested a review from arjo129 as a code owner October 22, 2025 15:31
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Oct 22, 2025
@arjo129 arjo129 enabled auto-merge (squash) October 22, 2025 15:35
@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏛️ ionic Gazebo Ionic

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

3 participants