-
Notifications
You must be signed in to change notification settings - Fork 53
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
Update Metal depth camera and selection buffer shaders to match #446 #498
Update Metal depth camera and selection buffer shaders to match #446 #498
Conversation
…ebosim#446 - Add new param colorTexResolution to the selection buffer shader - Implement the Metal equivalent of texelFetch in the shaders Signed-off-by: Rhys Mainwaring <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-rendering6 #498 +/- ##
===============================================
Coverage 55.53% 55.53%
===============================================
Files 195 195
Lines 19697 19697
===============================================
Hits 10938 10938
Misses 8759 8759 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for catching this. Now that we have metal support in ign-rendering, we could consider setting ogre2
back as the default render engine in our homebrew CI builds and use GraphicsApi::METAL
in ign-rendering7
It would be great to have the macOS CI tests running using Metal and ogre2. It might help resolve a number of the open issues for that platform as well. I have a stash which prototypes adding another environment variable (to set the graphics api) to the list of arguments accepted in the gtests parameter list. Not sure if that's what you have in mind for configuring the CI tests but I can dig it out and submit as a draft PR if you think it's useful. |
yeah that sounds good. For Ignition Garden / ign-rendering7, now that Metal support is almost fully integrated, I was thinking that we can just set the default API to Metal if we find that the platform is |
That sounds a bit easier to implement. It's quite nice to be able to test some of examples using the |
🦟 Bug fix
Apply #446 to Metal shaders
Summary
This PR applies #446 to Metal shaders and updates them to use the Metal equivalent of OpenGL's
texelFetch
(which istexture.read()
). Previously the depth camera and selection buffer Metal shaders were using a standard texture sampler accepting normalised coordinates and ignored the texture resolution uniform parameter.Testing
Currently there are no automated tests for the Metal shaders. Tested manually using the
ogre2demo
andmouse_picking
examples and alsoign gazebo
(by cherry-picking this change intomain
to build the gui and gazebo libraries).Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge