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

Support using custom shader materials and updating uniform variables (ogre2) #520

Merged
merged 5 commits into from
Jan 4, 2022

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Dec 22, 2021

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

Implemented code to supported loading custom shader material and updating their uniform variables.

Extended the custom_shaders_uniforms to run with ogre2 render engine. Note I made a slight tweak to the shader code in the demo to make the ogre 1.x and ogre 2.x behavior more consistent. When running with ogre2, the clip space z pos returned by gl_Position.z is different from ogre so it was rendering slightly different visual effect.

To test:

build and run the custom_shaders_uniforms demo:

./custom_shaders_uniforms ogre2

custom_shader_uniforms_ogre2

Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 marked this pull request as draft December 22, 2021 22:42
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Dec 22, 2021
@codecov
Copy link

codecov bot commented Dec 22, 2021

Codecov Report

Merging #520 (9eb3421) into ign-rendering6 (5e19061) will decrease coverage by 0.21%.
The diff coverage is 21.21%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering6     #520      +/-   ##
==================================================
- Coverage           54.70%   54.48%   -0.22%     
==================================================
  Files                 198      198              
  Lines               20029    20125      +96     
==================================================
+ Hits                10956    10966      +10     
- Misses               9073     9159      +86     
Impacted Files Coverage Δ
src/ShaderParam.cc 100.00% <ø> (ø)
ogre2/src/Ogre2Material.cc 71.60% <18.94%> (-12.61%) ⬇️
ogre2/src/Ogre2Mesh.cc 83.09% <75.00%> (-0.48%) ⬇️
include/ignition/rendering/base/BaseMaterial.hh 61.37% <0.00%> (-2.89%) ⬇️

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 5e19061...9eb3421. Read the comment docs.

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 changed the title Test custom shader ABI compatibility Support using custom shader materials and updating uniform variables (ogre2) Dec 24, 2021
@iche033 iche033 marked this pull request as ready for review December 24, 2021 00:22
@iche033 iche033 requested a review from WilliamLewww December 24, 2021 00:22
Copy link
Contributor

@WilliamLewww WilliamLewww left a comment

Choose a reason for hiding this comment

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

Just 1 small syntax error, but everything looks great!

@@ -64,6 +68,19 @@ void buildScene(ScenePtr _scene)
light0->SetSpecularColor(0.5, 0.5, 0.5);
root->AddChild(light0);

t std::string vertexShaderFile;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
t std::string vertexShaderFile;
std::string vertexShaderFile;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, fixed. 00d56a1

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 merged commit d444ff5 into ign-rendering6 Jan 4, 2022
@iche033 iche033 deleted the custom_shaders branch January 4, 2022 19:56
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

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.

3 participants