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

Fix overriding ogre2 transparency blend mode #266

Merged
merged 3 commits into from
Mar 13, 2021
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Mar 6, 2021

Signed-off-by: Ian Chen ichen@osrfoundation.org

🦟 Bug fix

Fixes gazebosim/gz-sim#496

Summary

Fixes issue with ign-gazebo's TransformControl tool being occluded by models.

The ogre2 transparency blend mode setting was being override when copying a material due to the order of the calls.

-> SetTransparency -> sets blend mode
-> SetAlphaFromTexture- > overrides the blend mode set in the previous call if arg is false

The PR reverses the order to make sure we keep the blend mode if visual is semi-transparent

Also made sure that if SetAlphaFromTexture(false) is called any time afterwards, we don't change the existing blend mode.

transform_control_blendmode

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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: Ian Chen <ichen@osrfoundation.org>
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Mar 6, 2021
@osrf-triage osrf-triage added this to Inbox in Core development Mar 6, 2021
Core development automation moved this from Inbox to In review Mar 8, 2021
@chapulina chapulina added the bug Something isn't working label Mar 8, 2021
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's fixed for me, thanks!

@codecov
Copy link

codecov bot commented Mar 13, 2021

Codecov Report

Merging #266 (78a5faf) into ign-rendering4 (20d6471) will increase coverage by 0.90%.
The diff coverage is 77.34%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering4     #266      +/-   ##
==================================================
+ Coverage           52.59%   53.50%   +0.90%     
==================================================
  Files                 143      145       +2     
  Lines               13329    13753     +424     
==================================================
+ Hits                 7010     7358     +348     
- Misses               6319     6395      +76     
Impacted Files Coverage Δ
include/ignition/rendering/ArrowVisual.hh 100.00% <ø> (ø)
include/ignition/rendering/AxisVisual.hh 100.00% <ø> (ø)
include/ignition/rendering/Camera.hh 100.00% <ø> (ø)
include/ignition/rendering/CompositeVisual.hh 100.00% <ø> (ø)
include/ignition/rendering/Geometry.hh 100.00% <ø> (ø)
include/ignition/rendering/Image.hh 100.00% <ø> (ø)
include/ignition/rendering/Light.hh 100.00% <ø> (ø)
include/ignition/rendering/Material.hh 100.00% <ø> (ø)
include/ignition/rendering/Mesh.hh 100.00% <ø> (ø)
include/ignition/rendering/Node.hh 100.00% <ø> (ø)
... and 52 more

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 108e28b...78a5faf. Read the comment docs.

@iche033 iche033 merged commit 7d55d9b into ign-rendering4 Mar 13, 2021
Core development automation moved this from In review to Done Mar 13, 2021
@iche033 iche033 deleted the fix_blendmode branch March 13, 2021 08:21
@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
bug Something isn't working 🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transform Controller Markers Behind Environment Models
3 participants