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 selection buffer material script #456

Merged
merged 2 commits into from
Oct 13, 2021
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Oct 7, 2021

🦟 Bug fix

Summary

on some machines, ogre would not find the definition of the vertex shader program referred to by the selection_buffer.material script. The vertex program definition was actually specified in a different file. This causes a compiler error to be printed in the ogre2.log:

18:54:23: Compiler error: reference to a non existing object in selection_buffer.material(37)

The fix is to just define a new one in the same file.

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

@github-actions github-actions bot added the 🔮 dome Ignition Dome label Oct 7, 2021
@iche033 iche033 requested a review from nkoenig October 7, 2021 18:18
Signed-off-by: Ian Chen <[email protected]>
@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

Merging #456 (bd25c12) into ign-rendering4 (8ce12a2) will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering4     #456      +/-   ##
==================================================
+ Coverage           55.83%   55.95%   +0.12%     
==================================================
  Files                 147      147              
  Lines               14120    14120              
==================================================
+ Hits                 7884     7901      +17     
+ Misses               6236     6219      -17     
Impacted Files Coverage Δ
src/Utils.cc 85.26% <0.00%> (+1.05%) ⬆️
ogre2/src/Ogre2MaterialSwitcher.cc 90.00% <0.00%> (+1.42%) ⬆️
ogre2/src/Ogre2SelectionBuffer.cc 87.39% <0.00%> (+2.60%) ⬆️
ogre2/src/Ogre2RayQuery.cc 60.60% <0.00%> (+9.09%) ⬆️

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 8ce12a2...bd25c12. Read the comment docs.

@chapulina chapulina added the bug Something isn't working label Oct 7, 2021
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 12, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 12, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
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.

Works for me on Bionic 👍

@chapulina chapulina merged commit 5e183b5 into ign-rendering4 Oct 13, 2021
@chapulina chapulina deleted the selection_buffer_mat branch October 13, 2021 00:32
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 17, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
chapulina added a commit that referenced this pull request Oct 18, 2021
* Fix ray query distance calculation (#438)

* fix ray query dist calc

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

* disable test

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

* ogre: Add missing required Paging component (#452)

Signed-off-by: Silvio <[email protected]>

* Fix selection buffer material script (#456)

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

Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Silvio Traversaro <[email protected]>
srmainwaring pushed a commit to srmainwaring/gz-rendering that referenced this pull request Oct 21, 2021
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 23, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 1, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 3, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 7, 2021
…material

- This is the same issue as fixed in gazebosim#456

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 9, 2021
- Add parameter "metal" and a member variable useMetalRenderSystem to Ogre2RenderEngine
- Modify LoadPlugins to load the Metal render system if enabled
- Modify CreateRenderSystem to select the Metal render system
- Modify RegisterHlms to load the Metal resources
- Add missing Metal shaders for the Terra (from Ogre2)
- Add Metal shader for skybox
- Update skybox material to support universal shaders
- Create subdirectories for GLSL and Metal in media/materials/programs and update Ogre2RenderEngine with new resource locations
- Add Metal and unified shaders to each material
- Add placeholders for a number of shaders to prevent compile errors when running examples
- Minor formatting changes to plain_color, point and skybox Metal shaders.
- Port depth_camera_vs and selection_buffer_fs to Metal
- The mouse_picker, ogre2_demo and transform_control demos now work correctly.
- Shader formatting - replace tabs with whitespace
- Port gaussian_noise vertex and pixel shaders from the GLSL counterparts
- The ogre2_demo and render_pass examples now work correctly.
- Port thermal_camera pixel shader
  - Correct error in thermal material script
  - Port thermal_camera pixel shaders from the GLSL counterparts
  - The thermal_camera examples now runs but further requires testing
- Add stubs for the remaining vertex and pixel shaders to port from GLSL
- Port heat_signature pixel shader
- Initial ports of depth_camera and gpu_rays shaders
  - On Metal the compositor in Ogre2GpuRay has a pixel format exception
  - Texture sampling for depth_camera_final_fs.metal is not using the equivalent of OpenGL's texelFetch
- Fix fragment program specifier in gaussian noise material
  - Unified shader had incorrect specifier
- Fix depth camera vertex shader not found in selection buffer material
  - This is the same issue as fixed in gazebosim#456
- Switch to user RGBA format for internal textures in gpu ray sensor
- Complete port of gpu rays 1st pass shader
  - Complete port of the GPU Rays shader
  - This depends upon the pixel format fix in gazebosim#468
- Add a method to render targets and cameras to retrieve the Metal texture id
  - These functions provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.
  - The argument is the address of a pointer to void* as the object required for Metal is an objective-c type id<MTLTexture> which we do not want exposed in the interface
  - There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873
  - cherry-pick 8fc230c
- Ensure line length limits are adhered to.
- Fix inconsistent parameter name in RenderTextureMetalId
  - Fix an inconsistent parameter name causing a CI failure.
- Fix unused parameter warning in RenderTextureMetalId
  - Fix an unused parameter warning causing a CI failure.

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 9, 2021
- Add parameter "metal" and a member variable useMetalRenderSystem to Ogre2RenderEngine
- Modify LoadPlugins to load the Metal render system if enabled
- Modify CreateRenderSystem to select the Metal render system
- Modify RegisterHlms to load the Metal resources
- Add missing Metal shaders for the Terra (from Ogre2)
- Add Metal shader for skybox
- Update skybox material to support universal shaders
- Create subdirectories for GLSL and Metal in media/materials/programs and update Ogre2RenderEngine with new resource locations
- Add Metal and unified shaders to each material
- Add placeholders for a number of shaders to prevent compile errors when running examples
- Minor formatting changes to plain_color, point and skybox Metal shaders.
- Port depth_camera_vs and selection_buffer_fs to Metal
- The mouse_picker, ogre2_demo and transform_control demos now work correctly.
- Shader formatting - replace tabs with whitespace
- Port gaussian_noise vertex and pixel shaders from the GLSL counterparts
- The ogre2_demo and render_pass examples now work correctly.
- Port thermal_camera pixel shader
  - Correct error in thermal material script
  - Port thermal_camera pixel shaders from the GLSL counterparts
  - The thermal_camera examples now runs but further requires testing
- Add stubs for the remaining vertex and pixel shaders to port from GLSL
- Port heat_signature pixel shader
- Initial ports of depth_camera and gpu_rays shaders
  - On Metal the compositor in Ogre2GpuRay has a pixel format exception
  - Texture sampling for depth_camera_final_fs.metal is not using the equivalent of OpenGL's texelFetch
- Fix fragment program specifier in gaussian noise material
  - Unified shader had incorrect specifier
- Fix depth camera vertex shader not found in selection buffer material
  - This is the same issue as fixed in gazebosim#456
- Switch to user RGBA format for internal textures in gpu ray sensor
- Complete port of gpu rays 1st pass shader
  - Complete port of the GPU Rays shader
  - This depends upon the pixel format fix in gazebosim#468
- Add a method to render targets and cameras to retrieve the Metal texture id
  - These functions provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.
  - The argument is the address of a pointer to void* as the object required for Metal is an objective-c type id<MTLTexture> which we do not want exposed in the interface
  - There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873
  - cherry-pick 8fc230c
- Ensure line length limits are adhered to.
- Fix inconsistent parameter name in RenderTextureMetalId
  - Fix an inconsistent parameter name causing a CI failure.
- Fix unused parameter warning in RenderTextureMetalId
  - Fix an unused parameter warning causing a CI failure.

Signed-off-by: Rhys Mainwaring <[email protected]>
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 9, 2021
- Add parameter "metal" and a member variable useMetalRenderSystem to Ogre2RenderEngine
- Modify LoadPlugins to load the Metal render system if enabled
- Modify CreateRenderSystem to select the Metal render system
- Modify RegisterHlms to load the Metal resources
- Add missing Metal shaders for the Terra (from Ogre2)
- Add Metal shader for skybox
- Update skybox material to support universal shaders
- Create subdirectories for GLSL and Metal in media/materials/programs and update Ogre2RenderEngine with new resource locations
- Add Metal and unified shaders to each material
- Add placeholders for a number of shaders to prevent compile errors when running examples
- Minor formatting changes to plain_color, point and skybox Metal shaders.
- Port depth_camera_vs and selection_buffer_fs to Metal
- The mouse_picker, ogre2_demo and transform_control demos now work correctly.
- Shader formatting - replace tabs with whitespace
- Port gaussian_noise vertex and pixel shaders from the GLSL counterparts
- The ogre2_demo and render_pass examples now work correctly.
- Port thermal_camera pixel shader
  - Correct error in thermal material script
  - Port thermal_camera pixel shaders from the GLSL counterparts
  - The thermal_camera examples now runs but further requires testing
- Add stubs for the remaining vertex and pixel shaders to port from GLSL
- Port heat_signature pixel shader
- Initial ports of depth_camera and gpu_rays shaders
  - On Metal the compositor in Ogre2GpuRay has a pixel format exception
  - Texture sampling for depth_camera_final_fs.metal is not using the equivalent of OpenGL's texelFetch
- Fix fragment program specifier in gaussian noise material
  - Unified shader had incorrect specifier
- Fix depth camera vertex shader not found in selection buffer material
  - This is the same issue as fixed in gazebosim#456
- Switch to user RGBA format for internal textures in gpu ray sensor
- Complete port of gpu rays 1st pass shader
  - Complete port of the GPU Rays shader
  - This depends upon the pixel format fix in gazebosim#468
- Add a method to render targets and cameras to retrieve the Metal texture id
  - These functions provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.
  - The argument is the address of a pointer to void* as the object required for Metal is an objective-c type id<MTLTexture> which we do not want exposed in the interface
  - There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873
  - cherry-pick 8fc230c
- Ensure line length limits are adhered to.
- Fix inconsistent parameter name in RenderTextureMetalId
  - Fix an inconsistent parameter name causing a CI failure.
- Fix unused parameter warning in RenderTextureMetalId
  - Fix an unused parameter warning causing a CI failure.

Signed-off-by: Rhys Mainwaring <[email protected]>
ahcorde pushed a commit that referenced this pull request Nov 25, 2021
* [Metal] enable loading of the Metal render system for ogre2

- Add parameter "metal" and a member variable useMetalRenderSystem to Ogre2RenderEngine
- Modify LoadPlugins to load the Metal render system if enabled
- Modify CreateRenderSystem to select the Metal render system
- Modify RegisterHlms to load the Metal resources
- Add missing Metal shaders for the Terra (from Ogre2)

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] enable metal render system for some examples

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] add Metal shader for skybox and update material

- Add Metal shader for skybox
- Update skybox material to support universal shaders
- Create subdirectories for GLSL and Metal in media/materials/programs and update Ogre2RenderEngine with new resource locations

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] add Metal and unified shaders to each material (wip)

- Add Metal and unified shaders to each material
- Add placeholders for a number of shaders to prevent compile errors when running examples

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] enable metal render system for remaining examples [do not merge]

- Note: these changes will need to be modified / reverted before merging

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] update examples to accept optional command line option "metal"

- Examples accept an additional command line option metal when specifying the ogre2 render engine.
- Pass parameters to the createCamera function.

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix formatting on example ogre2_demo

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] revert changes to example text_geom

- This example is for ogre only (not ogre2)

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] update formatting of shaders

- Minor formatting changes to plain_color, point and skybox Metal shaders.

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] port depth_camera and selection_buffer shaders

- Port depth_camera_vs and selection_buffer_fs to Metal
- The mouse_picker, ogre2_demo and transform_control demos now work correctly.

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] shader formatting - replace tabs with whitespace

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] port gaussian_noise shaders

- Port gaussian_noise vertex and pixel shaders from the GLSL counterparts
- The ogre2_demo and render_pass examples now work correctly.

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] port thermal_camera pixel shader

- Correct error in thermal material script
- Port thermal_camera pixel shaders from the GLSL counterparts
- The thermal_camera examples now runs but further requires testing

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] add placeholders for remaining shaders

- Add stubs for the remaining vertex and pixel shaders to port from GLSL

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] port heat_signature pixel shader

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] port depth_camera and gpu_rays shaders [wip]

- Initial ports of depth_camera and gpu_rays shaders
- This is work in progress:
	- On Metal the compositor in Ogre2GpuRay has a pixel format exception
	- Texture sampling for depth_camera_final_fs.metal is not using the equivalent of OpenGL's texelFetch

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] update custom_scene_viewer example to accept optional command line option "metal"

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix fragment program specifier in gaussian noise material

- Unified shader had incorrect specifier

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix depth camera vertex shader not found in selection buffer material

- This is the same issue as fixed in #456

Signed-off-by: Rhys Mainwaring <[email protected]>

* switch to user RGBA format for internal textures in gpu ray sensor

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

* [Metal] complete port of gpu rays 1st pass shader

- Complete port of the GPU Rays shader
- This depends upon the pixel format fix in #468

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] update shaders to fix lidar near plane clipping

- Update Metal shaders to match changes made in PR #356
- Update CrossPlatformSettings_piece to #define outVs_gl_ClipDistance
- Update Ign_piece_vs_any to use cross platform version of gl_ClipDistance

Signed-off-by: Rhys Mainwaring <[email protected]>

* Fix style for code check

- Remove line end whitespace causing CI to fail.

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] incorporate review feedback

- Revert debug output in Ogre2RenderEngine
- Fix codecheck issue with variable declaration shadows previous local
- Revert changes to simple_demo_qml
- Move useMetalRenderSystem to Ogre2RenderEnginePrivate

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] introduce enum for graphics interface

- Add enum class GraphicsAPI to RenderEngine.hh
- Add utils class GraphicsAPIUtils for setting enum from string
- Update Ogre2RenderEngine to use GraphicsAPI enum
- Update examples to use GraphicsAPI enum

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] refactor enum for graphics interface

- Move GraphicsAPI to own file

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix examples

- Fix custom_scene_viewer

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix class description

- Fix code check
- Change GraphicsAPIUtils class documentation
- Improve graphics API check in Ogre2RenderEngine

Signed-off-by: Rhys Mainwaring <[email protected]>

* [Metal] fix codecheck

- Fix code check - uninitialised variable warning

Signed-off-by: Rhys Mainwaring <[email protected]>

Co-authored-by: Ian Chen <[email protected]>
@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-01-10/1228/1

srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 28, 2022
- Add parameter "metal" and a member variable useMetalRenderSystem to Ogre2RenderEngine
- Modify LoadPlugins to load the Metal render system if enabled
- Modify CreateRenderSystem to select the Metal render system
- Modify RegisterHlms to load the Metal resources
- Add missing Metal shaders for the Terra (from Ogre2)
- Add Metal shader for skybox
- Update skybox material to support universal shaders
- Create subdirectories for GLSL and Metal in media/materials/programs and update Ogre2RenderEngine with new resource locations
- Add Metal and unified shaders to each material
- Add placeholders for a number of shaders to prevent compile errors when running examples
- Minor formatting changes to plain_color, point and skybox Metal shaders.
- Port depth_camera_vs and selection_buffer_fs to Metal
- The mouse_picker, ogre2_demo and transform_control demos now work correctly.
- Shader formatting - replace tabs with whitespace
- Port gaussian_noise vertex and pixel shaders from the GLSL counterparts
- The ogre2_demo and render_pass examples now work correctly.
- Port thermal_camera pixel shader
  - Correct error in thermal material script
  - Port thermal_camera pixel shaders from the GLSL counterparts
  - The thermal_camera examples now runs but further requires testing
- Add stubs for the remaining vertex and pixel shaders to port from GLSL
- Port heat_signature pixel shader
- Initial ports of depth_camera and gpu_rays shaders
  - On Metal the compositor in Ogre2GpuRay has a pixel format exception
  - Texture sampling for depth_camera_final_fs.metal is not using the equivalent of OpenGL's texelFetch
- Fix fragment program specifier in gaussian noise material
  - Unified shader had incorrect specifier
- Fix depth camera vertex shader not found in selection buffer material
  - This is the same issue as fixed in gazebosim#456
- Switch to user RGBA format for internal textures in gpu ray sensor
- Complete port of gpu rays 1st pass shader
  - Complete port of the GPU Rays shader
  - This depends upon the pixel format fix in gazebosim#468
- Add a method to render targets and cameras to retrieve the Metal texture id
  - These functions provide the Metal equivalent of GLId() and RenderTextureGLId() for OpenGL.
  - The argument is the address of a pointer to void* as the object required for Metal is an objective-c type id<MTLTexture> which we do not want exposed in the interface
  - There is a runtime dependency on an upstream change to ogre-next: OGRECave/ogre-next@3b11873
  - cherry-pick 8fc230c
- Ensure line length limits are adhered to.
- Fix inconsistent parameter name in RenderTextureMetalId
  - Fix an inconsistent parameter name causing a CI failure.
- Fix unused parameter warning in RenderTextureMetalId
  - Fix an unused parameter warning causing a CI failure.

Signed-off-by: Rhys Mainwaring <[email protected]>
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.

3 participants