Prefer OpenGL over OpenGL ES

[valaphee]

Connections
resolves #5481

Description
While working with features which are only present on OpenGL, but not ES, I wondered why OpenGL ES was used on my desktop PC. This also seems to only be the case on Linux, as on Windows OpenGL is used.

Testing
Running any example with WGPU_BACKEND=gl; On cube for example the wireframe should be visibile if the device supports OpenGL, and not only OpenGL ES

Checklist

• Run cargo fmt.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[Wumpf]

as-is this certainly gonna be a problem for Android.

I believe it should be fine to do this conditionally for desktop platforms, but I lack the historical context here. Maybe @cwfitzgerald knows if there's more problems with binding the desktop OpenGL api.
The more obvious concern that comes to my mind is that by setting desktop opengl we more easily break webgl & android compatibility when testing locally. The prime usecase for OpenGL in wgpu is to be a fallback when other APIs are not available, so stepping this up to desktop OpenGL may actually not be desirable since any scenario where more features are required should instead use Vulkan/DX12/Metal. I'd also guess that on Windows it's only desktop because there's nothing else available in wgl.

[valaphee]

Yea I mean OpenGL and OpenGL ES share the same code path anyway, all features are gated accordingly, and allowing to support more modern features on Linux shouldn't be a problem as OpenGL is already used on Windows anyway, it's just for completeness and that I can test 64-bit vertex formats, as I don't have Windows.

Ah you're right, added a check if egl supports OpenGL, now it also works on Android (tested on Android 13).

[valaphee]

I noticed when running cargo xtask test that wgpu_test::clear_texture::clear_texture_uncompressed now fails.

[2024-04-06T12:34:24Z ERROR wgpu_test::expectations] Expected to fail due to [FailureReason { kind: Some(Panic), message: Some("texture with format Rg8Snorm was not fully cleared") }, FailureReason { kind: Some(Panic), message: Some("texture with format Rgb9e5Ufloat was not fully cleared") }, FailureReason { kind: Some(ValidationError), message: Some("GL_INVALID_FRAMEBUFFER_OPERATION") }, FailureReason { kind: Some(ValidationError), message: Some("GL_INVALID_OPERATION") }], but did not fail
thread '<unnamed>' panicked at 'explicit panic', tests/src/run.rs:119:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

the test is expected to fail, but does no longer fail, which I guess is a good thing. The fail should only be expected when the backend is GLES.

[Wumpf]

thanks for testing and fixing on Android. Thinking about this again, with this fix in place and the fact that we pass now even more tests there doesn't seem to be much reason not to enable desktop gl if possible :)

Can you change the test you mentioned from expected-to-fail to expected-to-succeed as part of this PR please?

[valaphee]

I noticed some odd behavior, with Bevy using OpenGL

normally it should look like (Win+Vulkan, WebGL, Linux+Vulkan):

but with OpenGL on Linux I get

on Windows with OpenGL it doesn't run at all

Caused by:
    In Device::create_render_pipeline
      note: label = `pbr_opaque_mesh_pipeline`
    Internal error in ShaderStages(FRAGMENT) shader: gsamplerCubeArrayShadow isn't supported in textureGrad, textureLod or texture with bias

[valaphee]

Ah correction, on Linux with OpenGL ES I get spammed with

2024-04-08T16:33:18.891346Z ERROR wgpu_hal::gles: wgpu-hal heuristics assumed that the view dimension will be equal to `Cube` rather than `CubeArray`.
`D2` textures with `depth_or_array_layers == 1` are assumed to have view dimension `D2`
`D2` textures with `depth_or_array_layers > 1` are assumed to have view dimension `D2Array`
`D2` textures with `depth_or_array_layers == 6` are assumed to have view dimension `Cube`
`D2` textures with `depth_or_array_layers > 6 && depth_or_array_layers % 6 == 0` are assumed to have view dimension `CubeArray`    

but seems to run as expected.

Fixed the test, added the comment for enabling sRGB conversion; GL now also fills out the AdapterInfo:
driver: "OpenGL ES", driver_info: "3.2 Mesa 24.0.4-arch1.2",
driver: "OpenGL", driver_info: "4.6 (Compatibility Profile) Mesa 24.0.4-arch1.2"
(looks the same as on Vulkan)

[Wumpf]

code lgtm to be me now, but those color differences are concerning (looks like a shader broke..?), we'll have to get to the bottom of this before landing otherwise anyone relying on gl on linux breaks.
Have you checked of the log spam on linux gl es happened before? Also, curious, how did you force gles with this patch now?

[valaphee]

code lgtm to be me now, but those color differences are concerning (looks like a shader broke..?), we'll have to get to the bottom of this before landing otherwise anyone relying on gl on linux breaks. Have you checked of the log spam on linux gl es happened before? Also, curious, how did you force gles with this patch now?

On Windows it doesn't run at all, because of missing shader features, and on Linux without this PR I get the errors in the log (wgpu-hal heuristics, also probably the shadow shader, even though it seems to work just fine)

With this pr, the error in the log is gone, but the result is just wrong. I'll try to isolate this issue.

Did it manually (by changing bind_api to OPENGL_ES), don't know if its worth exposing this, or always just prefer OpenGL.

[Wumpf]

Did it manually (by changing bind_api to OPENGL_ES), don't know if its worth exposing this, or always just prefer OpenGL.

don't think it's worth exposing either if we get this stuff fixed. Was just curious if there's some magic trick :)

[valaphee]

It was an oversight, also got this issue on Windows. The problem was that draw indexed has a path for when the base_vertex is 0 and uses draw_elements_instanced, and first_instance gets always passed though as an uniform, regardless of if the shader is compiled with the draw_param feature or not.

The path also only seems to be relevant to ANGLE, and ANGLE is OpenGL ES anyway, therefore the feature will not be available there.

[Wumpf]

great stuff, thank you!