Skip to content

Conversation

@MeFisto94
Copy link
Member

Background: https://github.com/MeFisto94/test-spotbugs/runs/560327969?check_suite_focus=true#step:5:1490

On MESA with Debug Mode or Assertions on, we get a GL_INVALID_ENUM at Line 510 (getInteger(GL_MAX_GEOMETRY_SHADER_...)).

I assume this is because this device supports SSBOs as an extension while still being on OpenGL 3.1. For most non-strict-real-gpus, SSBOs are only available with a much higher GL Version.

Anyway, this is my blind poke at this, I will re-run the above test, but I'd appreciate if someone finds some official source about when the Integers can be getted.

Also note the GL_ARB_geometry_shader_4, I hope I did it right, it's not a core extension but an extension and I've read some stuff that those _4 usually had a completely different API.

@MeFisto94 MeFisto94 requested a review from riccardobl April 4, 2020 13:45
@MeFisto94
Copy link
Member Author

MeFisto94 commented Apr 4, 2020

I just realized that Geometry and Compute Shaders are checked just below my code, so I can probably reorder that.

I only wonder if the extensions are always present, even when the shader type should already be in core?

i.e. we miss a bunch of if version >=s there, don't we?
Also I wonder if this is a MESA only thing, because the UBO Extension should expose all those limits, even when the specific shaders aren't available. The good thing is this patch doesn't hurt other implementations.

Edit: The second concern is not true, provided loadCapabilitiesGL2 is run before that code, see

if (oglVer >= 330) {
caps.add(Caps.OpenGL33);
caps.add(Caps.GeometryShader);
}

When I checked that, I will reorder the other stuff.

@MeFisto94
Copy link
Member Author

Green light: https://github.com/MeFisto94/test-spotbugs/runs/560492126?check_suite_focus=true#step:5:1485

So @pspeed42 and @riccardobl let's talk about the specific implementation, especially given that if you expand the diff down you see that geometry shader detection is already there. When I move that up I could use Caps.contains(GeometryShader), but I think the Geometry Shader detection misses out implementations where the extension isn't present but the GL Version supports Geom Shaders already, or is this done somewhere else already?

@MeFisto94
Copy link
Member Author

@MeFisto94
Copy link
Member Author

@MeFisto94 MeFisto94 merged commit 989b2a6 into jMonkeyEngine:master Apr 4, 2020
@stephengold stephengold added this to the v3.3.1 milestone Apr 16, 2020
@stephengold
Copy link
Member

Included in v3.3 branch at 828ca92.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants