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

Add shapes plugin #107

Merged
merged 21 commits into from
May 14, 2020
Merged

Add shapes plugin #107

merged 21 commits into from
May 14, 2020

Conversation

JShep1
Copy link

@JShep1 JShep1 commented Apr 28, 2020

This PR makes use of ign-gazebo#95 so that PR will need to be merged before this one (it doesn't have to be, but I thought a separate PR for that functionality would be better for visibility and documentation purposes.

There are a few things I made "prototypes" of that I would like to have looked and discussed for improvement:

  • The unused entity id generation in UniqueId()
  • How should the hovered mouse position be stored for best scalability and access for future users of the value
  • Should the code for creating a model from the sdf string be moved to a separate function?
  • Is the current location of HandleModelPlacement() good/could there be a better way to handle the mouse events here?

There's currently a bug with this addition messing with the button pressing functionalities - I'll take a closer look at this soon

Resolves #81

jshep1 and others added 10 commits April 8, 2020 11:16
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1
Copy link
Author

JShep1 commented Apr 28, 2020

Demo:
shapesplugin

Signed-off-by: John Shepherd <john@openrobotics.org>
@chapulina chapulina added the 📜 blueprint Ignition Blueprint label Apr 28, 2020
@chapulina chapulina added this to Inbox in Core development via automation Apr 28, 2020
@chapulina chapulina moved this from Inbox to In review in Core development Apr 28, 2020
Signed-off-by: John Shepherd <john@openrobotics.org>
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.

It's working for me!

Should the code for creating a model from the sdf string be moved to a separate function?

+1


I noticed this issue when spawning entities from below the ground. The spawned pose is slightly offset from the preview pose at click time:

place_change_pos

src/gui/plugins/scene3d/Scene3D.cc Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.hh Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
This was referenced Apr 29, 2020
John Shepherd added 4 commits May 5, 2020 11:35
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1
Copy link
Author

JShep1 commented May 5, 2020

I noticed this issue when spawning entities from below the ground. The spawned pose is slightly offset from the preview pose at click time:

Fixed in dd864dd

@JShep1 JShep1 requested a review from chapulina May 6, 2020 06:54
@JShep1
Copy link
Author

JShep1 commented May 6, 2020

Ready for another iteration

include/ignition/gazebo/rendering/SceneManager.hh Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
Signed-off-by: John Shepherd <john@openrobotics.org>
@codecov
Copy link

codecov bot commented May 10, 2020

Codecov Report

❗ No coverage uploaded for pull request base (ign-gazebo2@dd5c72b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             ign-gazebo2     #107   +/-   ##
==============================================
  Coverage               ?   62.38%           
==============================================
  Files                  ?      123           
  Lines                  ?     6091           
  Branches               ?        0           
==============================================
  Hits                   ?     3800           
  Misses                 ?     2291           
  Partials               ?        0           

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 dd5c72b...5d7f4ac. Read the comment docs.

John Shepherd added 2 commits May 12, 2020 13:32
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 requested a review from iche033 May 12, 2020 20:39
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

the new mouse hover events works great

I only have a couple comments left. Otherwise looks good to me.

src/gui/plugins/scene3d/Scene3D.hh Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
John Shepherd added 2 commits May 12, 2020 15:30
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: John Shepherd <john@openrobotics.org>
@JShep1 JShep1 requested a review from iche033 May 13, 2020 17:12
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.

Works great!

@JShep1 JShep1 merged commit 24573da into ign-gazebo2 May 14, 2020
Core development automation moved this from In review to Done May 14, 2020
@JShep1 JShep1 deleted the add_shapes_plugin branch May 14, 2020 17:19
azeey pushed a commit to azeey/gz-sim that referenced this pull request May 20, 2020
Signed-off-by: John Shepherd <john@openrobotics.org>
nkoenig added a commit that referenced this pull request May 21, 2020
* Add shapes plugin (#107)

Signed-off-by: John Shepherd <john@openrobotics.org>

* Add option to suppress warning about missing child model (#132)

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

* Support multi entity spawn (#146)

* Support multi entity spawn

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* Added more documentation

Signed-off-by: Nate Koenig <nate@openrobotics.org>

Co-authored-by: Nate Koenig <nate@openrobotics.org>

* Bump ign-msgs version to 4.9 (#148)

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* Revert silly mistake

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* ign-msgs 5.3 dependency

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* Bump minor version

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* Changelog

Signed-off-by: Nate Koenig <nate@openrobotics.org>

* RawPose

Signed-off-by: Nate Koenig <nate@openrobotics.org>

Co-authored-by: John <john@openrobotics.org>
Co-authored-by: Addisu Taddese <addisu@openrobotics.org>
Co-authored-by: Nate Koenig <nate@openrobotics.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
azeey pushed a commit to azeey/gz-sim that referenced this pull request May 28, 2020
Signed-off-by: John Shepherd <john@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants