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

World exporter #474

Merged
merged 24 commits into from
Jan 29, 2021
Merged

World exporter #474

merged 24 commits into from
Jan 29, 2021

Conversation

gonzodepedro
Copy link
Collaborator

Adds a plugin to export worlds to meshes

Requires: gazebosim/gz-common#133

Signed-off-by: Gonzalo de Pedro [email protected]

Signed-off-by: Gonzalo de Pedro <[email protected]>
@gonzodepedro gonzodepedro self-assigned this Dec 4, 2020
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Dec 4, 2020
@gonzodepedro gonzodepedro changed the title Added world exporter World exporter Dec 4, 2020
Signed-off-by: Gonzalo de Pedro <[email protected]>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I haven't tried it yet. Just left some high-level comments. I also believe we need an ign-common3 release.

src/systems/world_exporter/WorldExporter.hh Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Dec 4, 2020
@nkoenig
Copy link
Contributor

nkoenig commented Dec 10, 2020

Would it be possible to add a short tutorial that describes what the world exporter is and how to use it?

@gonzodepedro
Copy link
Collaborator Author

gonzodepedro commented Dec 10, 2020

Would it be possible to add a short tutorial that describes what the world exporter is and how to use it?

That is a good idea. Should I add it as a Readme file in the system plugin directory or is there a better place to add it?

Signed-off-by: Gonzalo de Pedro <[email protected]>
@chapulina
Copy link
Contributor

Should I add it as a Readme file in the system plugin directory or is there a better place to add it?

Could you create a new one in the tutorials folder?

https:/ignitionrobotics/ign-gazebo/tree/ign-gazebo4/tutorials

Be sure to link to it from tutorials.md.in. Those tutorials are generated when we do make doc and are rendered here.

Thanks!

@nkoenig
Copy link
Contributor

nkoenig commented Dec 22, 2020

@gonzodepedro , I added a sample world that you could use in the tutorial. See: 63c9eb7

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Nice, it works for me!

@gonzodepedro , besides the tutorial, would it be possible to add a test under test/integration? At a minimum, I think it could load a world that has one of each geometry type, and at the end we check that a collada file was generated (and delete it).

src/systems/world_exporter/WorldExporter.hh Outdated Show resolved Hide resolved
examples/worlds/world_exporter.sdf Outdated Show resolved Hide resolved
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

This is awesome! I was able to generate a cave world and import it into blender. There is an issue related to transparency that I mention below that caused everything to be transparent, but otherwise, the world, with 524 submeshes loaded without a problem 🎉.

src/systems/world_exporter/CMakeLists.txt Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
src/systems/world_exporter/CMakeLists.txt Outdated Show resolved Hide resolved
src/systems/world_exporter/WorldExporter.cc Outdated Show resolved Hide resolved
@chapulina
Copy link
Contributor

ign-common 3.8.0 is out. Bumped the requirement in 9ff6f09.

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Dec 23, 2020
@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #474 (7ea379d) into ign-gazebo4 (59ea1c6) will increase coverage by 0.06%.
The diff coverage is 79.04%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo4     #474      +/-   ##
===============================================
+ Coverage        77.24%   77.31%   +0.06%     
===============================================
  Files              213      215       +2     
  Lines            11925    12029     +104     
===============================================
+ Hits              9212     9300      +88     
- Misses            2713     2729      +16     
Impacted Files Coverage Δ
...ems/collada_world_exporter/ColladaWorldExporter.cc 78.64% <78.64%> (ø)
src/ServerPrivate.cc 83.79% <100.00%> (ø)
...ems/collada_world_exporter/ColladaWorldExporter.hh 100.00% <100.00%> (ø)
src/SimulationRunner.cc 92.92% <0.00%> (+1.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59ea1c6...7ea379d. Read the comment docs.

Signed-off-by: Nate Koenig <[email protected]>
@nkoenig
Copy link
Contributor

nkoenig commented Jan 21, 2021

@gonzodepedro , I believe all that is left is a test.

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

I left one more minor comment. Otherwise, with the addition of the test, I think this is ready to go.

@nkoenig nkoenig self-requested a review January 28, 2021 14:12
@nkoenig
Copy link
Contributor

nkoenig commented Jan 28, 2021

Test has been added in db69c56. I believe this completes this PR, pending CI.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

LGTM, I just have some minor comments that would be nice to address, but don't need to block the PR.

test/worlds/collada_world_exporter.sdf Show resolved Hide resolved
tutorials/collada_world_exporter.md Outdated Show resolved Hide resolved
test/integration/collada_world_exporter.cc Outdated Show resolved Hide resolved
test/integration/collada_world_exporter.cc Show resolved Hide resolved
Nate Koenig added 3 commits January 28, 2021 09:46
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM!

@nkoenig nkoenig merged commit f6a7329 into ign-gazebo4 Jan 29, 2021
@nkoenig nkoenig deleted the gonzodepedro/world_exporter branch January 29, 2021 18:15
chapulina added a commit that referenced this pull request Jul 23, 2021
* Added world exporter

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Fixed CMake

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Changes based on review

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Added example world

Signed-off-by: Nate Koenig <[email protected]>

* bump required ign-common version

Signed-off-by: Louise Poubel <[email protected]>

* PR updates

Signed-off-by: Nate Koenig <[email protected]>

* Update documentation to have simulation self terminate

Signed-off-by: Nate Koenig <[email protected]>

* Rename world_export to collada_world_exporter

Signed-off-by: Nate Koenig <[email protected]>

* Finish world exporter renaming

Signed-off-by: Nate Koenig <[email protected]>

* Revert change

Signed-off-by: Nate Koenig <[email protected]>

* Added a tutorial

Signed-off-by: Nate Koenig <[email protected]>

* Update transform

Signed-off-by: Nate Koenig <[email protected]>

* Added message

Signed-off-by: Nate Koenig <[email protected]>

* Added a test

Signed-off-by: Nate Koenig <[email protected]>

* Added more shapes

Signed-off-by: Nate Koenig <[email protected]>

* Cleanup in two locations

Signed-off-by: Nate Koenig <[email protected]>

* Fix build

Signed-off-by: Nate Koenig <[email protected]>

* Apply scale, and fix codecheck

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
chapulina added a commit that referenced this pull request Jul 23, 2021
* Added world exporter

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Fixed CMake

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Changes based on review

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Added example world

Signed-off-by: Nate Koenig <[email protected]>

* bump required ign-common version

Signed-off-by: Louise Poubel <[email protected]>

* PR updates

Signed-off-by: Nate Koenig <[email protected]>

* Update documentation to have simulation self terminate

Signed-off-by: Nate Koenig <[email protected]>

* Rename world_export to collada_world_exporter

Signed-off-by: Nate Koenig <[email protected]>

* Finish world exporter renaming

Signed-off-by: Nate Koenig <[email protected]>

* Revert change

Signed-off-by: Nate Koenig <[email protected]>

* Added a tutorial

Signed-off-by: Nate Koenig <[email protected]>

* Update transform

Signed-off-by: Nate Koenig <[email protected]>

* Added message

Signed-off-by: Nate Koenig <[email protected]>

* Added a test

Signed-off-by: Nate Koenig <[email protected]>

* Added more shapes

Signed-off-by: Nate Koenig <[email protected]>

* Cleanup in two locations

Signed-off-by: Nate Koenig <[email protected]>

* Fix build

Signed-off-by: Nate Koenig <[email protected]>

* Apply scale, and fix codecheck

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
chapulina added a commit that referenced this pull request Jul 23, 2021
* Added world exporter

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Fixed CMake

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Changes based on review

Signed-off-by: Gonzalo de Pedro <[email protected]>

* Added example world

Signed-off-by: Nate Koenig <[email protected]>

* bump required ign-common version

Signed-off-by: Louise Poubel <[email protected]>

* PR updates

Signed-off-by: Nate Koenig <[email protected]>

* Update documentation to have simulation self terminate

Signed-off-by: Nate Koenig <[email protected]>

* Rename world_export to collada_world_exporter

Signed-off-by: Nate Koenig <[email protected]>

* Finish world exporter renaming

Signed-off-by: Nate Koenig <[email protected]>

* Revert change

Signed-off-by: Nate Koenig <[email protected]>

* Added a tutorial

Signed-off-by: Nate Koenig <[email protected]>

* Update transform

Signed-off-by: Nate Koenig <[email protected]>

* Added message

Signed-off-by: Nate Koenig <[email protected]>

* Added a test

Signed-off-by: Nate Koenig <[email protected]>

* Added more shapes

Signed-off-by: Nate Koenig <[email protected]>

* Cleanup in two locations

Signed-off-by: Nate Koenig <[email protected]>

* Fix build

Signed-off-by: Nate Koenig <[email protected]>

* Apply scale, and fix codecheck

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
@iche033 iche033 mentioned this pull request Oct 22, 2021
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants