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

[macOS] modify definition of bufferFetch1 to work when GL_ARB_texture_buffer_range is not available #462

Conversation

srmainwaring
Copy link
Contributor

This PR modifies the shader piece CrossPlatformSettings_piece_all.glsl to work when the OpenGL extension GL_ARB_texture_buffer_range is not available.

The definition of bufferFetch1 was failing for OpenGL < 4.2 resulting in compile errors for lighting shaders

🦟 Bug fix

Partial fix of #422

Summary

This is a single line change to the cross platform settings shader piece. It changes the definition of bufferFetch1 to use the bufferFetch definition which has compatibility settings rather than use texelFetch directly.

Before this change the shaders would fail to compile using the ogre2 render engine on macOS. After this change the shaders will compile.

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

Not all tests pass on macOS. The failures are listed below, but are not connected to the changes in this PR.

The following tests FAILED:
	  7 - UNIT_COMVisual_TEST (Subprocess aborted)
	 19 - UNIT_Heightmap_TEST (SEGFAULT)
	 59 - UNIT_RenderingIface_TEST (SEGFAULT)
	 83 - INTEGRATION_depth_camera (Failed)

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

…_buffer_range is not available

- The definition of bufferFetch1 was failing for OpenGL < 4.2 resulting in compile errors for lighting shaders

Signed-off-by: Rhys Mainwaring <[email protected]>
@srmainwaring srmainwaring requested a review from iche033 as a code owner October 9, 2021 19:05
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress labels Oct 9, 2021
@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #462 (5aceaf2) into ign-rendering6 (67b5420) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering6     #462      +/-   ##
==================================================
- Coverage           53.49%   53.49%   -0.01%     
==================================================
  Files                 192      192              
  Lines               19560    19560              
==================================================
- Hits                10464    10463       -1     
- Misses               9096     9097       +1     
Impacted Files Coverage Δ
...e/ignition/rendering/base/BaseGaussianNoisePass.hh 96.66% <0.00%> (-3.34%) ⬇️

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 67b5420...5aceaf2. Read the comment docs.

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me.

@iche033 iche033 merged commit cde3e08 into gazebosim:ign-rendering6 Oct 12, 2021
@srmainwaring srmainwaring deleted the feature/ign-rendering6-shaders-macOS branch October 13, 2021 09:39
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 21, 2021
…_buffer_range is not available (gazebosim#462)

- The definition of bufferFetch1 was failing for OpenGL < 4.2 resulting in compile errors for lighting shaders

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
🏯 fortress Ignition Fortress 🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants