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

Screenshot plugin #170

Merged
merged 17 commits into from
Mar 9, 2021
Merged

Screenshot plugin #170

merged 17 commits into from
Mar 9, 2021

Conversation

jennuine
Copy link
Contributor

@jennuine jennuine commented Jan 28, 2021

Closes #95

This plugin takes screenshots similar to Gazebo-classic by clicking the camera icon. The plugin uses a screenshot service that saves an image of the current 3D scene from the user camera. The default directory of saved screenshots is ~/.ignition/gui/pictures but users can change the directory by selecting the folder icon.

This can be used along with ign-gazebo's screenshot service in gazebosim/gz-sim#588

EDIT: depends on gazebosim/gz-sim#598

Peek 2021-01-27 17-55

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jan 28, 2021
@jennuine jennuine marked this pull request as ready for review January 28, 2021 02:22
@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #170 (c33970b) into ign-gui3 (8fb1564) will decrease coverage by 1.32%.
The diff coverage is 26.41%.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gui3     #170      +/-   ##
============================================
- Coverage     60.55%   59.23%   -1.33%     
============================================
  Files            21       23       +2     
  Lines          2624     2730     +106     
============================================
+ Hits           1589     1617      +28     
- Misses         1035     1113      +78     
Impacted Files Coverage Δ
src/plugins/scene3d/Scene3D.cc 12.78% <ø> (ø)
src/plugins/screenshot/Screenshot.cc 25.71% <25.71%> (ø)
src/plugins/screenshot/Screenshot.hh 100.00% <100.00%> (ø)

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 8fb1564...c33970b. Read the comment docs.

chapulina added a commit that referenced this pull request Jan 29, 2021
Signed-off-by: Louise Poubel <louise@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.

Works for me! Some suggestions below.

Also, do you mind adding some basic tests? Maybe just instantiating the plugin, requesting the service, and checking that a file was created?

src/plugins/scene3d/Scene3D.hh Outdated Show resolved Hide resolved
src/plugins/screenshot/Screenshot.cc Outdated Show resolved Hide resolved
* Suggestions to #170

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

* Debug message with camera name

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

* fix path

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@jennuine jennuine marked this pull request as draft February 2, 2021 00:39
@jennuine
Copy link
Contributor Author

jennuine commented Feb 2, 2021

Currently looking into an issue where the colors in the the saved screenshot do not match what is shown in ignition. Here are some examples, the image on the left is ignition and on the right is the saved screenshot:

Screenshot from 2021-02-01 15-24-52

Screenshot from 2021-02-01 15-27-13

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@chapulina
Copy link
Contributor

Here's a fix for the swapped colors: gazebosim/gz-common#162

Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@jennuine jennuine marked this pull request as ready for review March 4, 2021 01:52
@jennuine jennuine requested a review from chapulina March 4, 2021 01:52
@jennuine
Copy link
Contributor Author

jennuine commented Mar 4, 2021

Also, do you mind adding some basic tests? Maybe just instantiating the plugin, requesting the service, and checking that a file was created?

Added a simple test that only instantiates plugin e8c3cd2. The code that loads the Scene3D plugin and requests the service is commented out because currently unable to load Scene3D from another plugin.

src/plugins/scene3d/Scene3D.hh Outdated Show resolved Hide resolved
src/plugins/screenshot/Screenshot.cc Outdated Show resolved Hide resolved
tutorials/08_screenshot.md Outdated Show resolved Hide resolved
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
@chapulina chapulina mentioned this pull request Mar 6, 2021
7 tasks
jennuine and others added 4 commits March 8, 2021 09:23
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Jenn Nguyen <jenn@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor

I pushed c33970b directly to this branch so we suppress the 4251 warnings on all plugin files. I have no idea where the ign-rendering warnings are leaking through, but this should handle it. This warning is already suppressed on all src files that are not in plugins.

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.

Thanks for iterating, works great!

@chapulina chapulina enabled auto-merge (squash) March 9, 2021 21:52
@chapulina chapulina merged commit 52a4e96 into ign-gui3 Mar 9, 2021
@chapulina chapulina deleted the jennuine/screenshot branch March 9, 2021 22:12
@chapulina chapulina mentioned this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants