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

[wip] Added transparent mode #336

Closed
wants to merge 4 commits into from

Conversation

atharva-18
Copy link
Contributor

🎉 New feature

Summary

Adds transparent mode to visuals
transparent

Test it

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Atharva Pusalkar <[email protected]>
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jun 9, 2021
@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #336 (605d7ee) into main (8c34f9d) will decrease coverage by 0.22%.
The diff coverage is 16.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #336      +/-   ##
==========================================
- Coverage   57.92%   57.70%   -0.23%     
==========================================
  Files         166      166              
  Lines       16433    16522      +89     
==========================================
+ Hits         9519     9534      +15     
- Misses       6914     6988      +74     
Impacted Files Coverage Δ
include/ignition/rendering/Visual.hh 100.00% <ø> (ø)
include/ignition/rendering/base/BaseVisual.hh 68.82% <0.00%> (-2.96%) ⬇️
ogre/src/OgreVisual.cc 16.57% <0.00%> (-6.63%) ⬇️
ogre2/src/Ogre2Visual.cc 70.12% <46.87%> (-6.10%) ⬇️

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 8c34f9d...605d7ee. Read the comment docs.

@ahcorde
Copy link
Contributor

ahcorde commented Jun 9, 2021

TODO

@iche033
Copy link
Contributor

iche033 commented Jun 15, 2021

transparency settings can be set through the Material class. Is it not working for your needs?

@atharva-18
Copy link
Contributor Author

transparency settings can be set through the Material class. Is it not working for your needs?

I wanted to set the transparency for a particular visual rather than a material being used by multiple visuals, similar to the View as transparent mode in Gazebo classic.

@iche033
Copy link
Contributor

iche033 commented Jun 15, 2021

I see. To do that, we need to make sure the material that each visual has is unique. That's done when the material is set, i.e. visual->SetMaterial(mat, true), with the second arg telling the visual to clone the material first so it now has a copy instead of an instance of the material. Afterwards, you can do:

auto mat = visual->Material();
mat->SetTransparency(...);

Then it should only affect that particular visual.

It's the same idea that's implemented in gazebo classic, except the transparency logic is being handled in the Material class instead of the Visual class.

@atharva-18
Copy link
Contributor Author

I see. To do that, we need to make sure the material that each visual has is unique. That's done when the material is set, i.e. visual->SetMaterial(mat, true), with the second arg telling the visual to clone the material first so it now has a copy instead of an instance of the material. Afterwards, you can do:

auto mat = visual->Material();
mat->SetTransparency(...);

Then it should only affect that particular visual.

It's the same idea that's implemented in gazebo classic, except the transparency logic is being handled in the Material class instead of the Visual class.

This would be a better approach. I think we can move the transparency logic to ign-gazebo (since the material is cloned for each visual) -

auto mat = visual->Material();

if (showTransp)
  mat->SetTransparency(0.5);
else
  mat->SetTransparency(0.0);

@ahcorde ahcorde requested a review from iche033 June 22, 2021 10:22
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@atharva-18
Copy link
Contributor Author

atharva-18 commented Jun 23, 2021

@ahcorde I have moved the transparency logic to ign-gazebo. PR - gazebosim/gz-sim#878. The transparency can be set for each material (as @iche033 suggested) since the material is cloned for each visual. This is similar to the Align Tool plugin.

@chapulina
Copy link
Contributor

I have moved the transparency logic to ign-gazebo

Thanks, @atharva-18 . Does this mean this PR can be closed?

@atharva-18
Copy link
Contributor Author

I have moved the transparency logic to ign-gazebo

Thanks, @atharva-18 . Does this mean this PR can be closed?

Yes, we can close this PR.

@atharva-18 atharva-18 closed this Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants