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 ColladaLoader loading <transparent> tag when <transparency> does not exist #68

Merged
merged 2 commits into from
Jun 5, 2020

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented May 27, 2020

When loading <transparent> tag in our current implementation, we use a transparency value of 0.0 if <transparency> does not tag exist. But according to collada 1.5 spec chapter 7-6, page 250:

If <transparency> does not exist then it has no effect on the equation’s result. This is equivalent
to a factor that is 1.0: 

This causes some meshes to be transparent, see gazebosim/gz-rendering#89. This PR fixes the default value used in calculation.

Signed-off-by: Ian Chen [email protected]

@iche033 iche033 requested a review from mjcarroll as a code owner May 27, 2020 18:18
@github-actions github-actions bot added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels May 27, 2020
Signed-off-by: Ian Chen <[email protected]>
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #68 into ign-common3 will increase coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-common3      #68      +/-   ##
===============================================
+ Coverage        73.79%   73.80%   +0.01%     
===============================================
  Files               68       68              
  Lines             9257     9261       +4     
===============================================
+ Hits              6831     6835       +4     
  Misses            2426     2426              
Impacted Files Coverage Δ
graphics/src/ColladaLoader.cc 84.01% <66.66%> (+0.04%) ⬆️

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 381e65c...e95312b. Read the comment docs.

@chapulina chapulina merged commit a3ac618 into ign-common3 Jun 5, 2020
@chapulina chapulina deleted the collada_transparent branch June 5, 2020 22:19
mjcarroll pushed a commit that referenced this pull request Oct 14, 2020
…not exist (#68)

* fix loading <transparent> when <transparency> does not exist

Signed-off-by: Ian Chen <[email protected]>
mjcarroll pushed a commit that referenced this pull request Oct 19, 2020
…not exist (#68)

* fix loading <transparent> when <transparency> does not exist

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants