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

Issue SetDrawColorBuffers before clearing buffers in GLES, use clear_buffer_f32_slice instead of clear #5666

Conversation

nickguletskii
Copy link
Contributor

@nickguletskii nickguletskii commented May 5, 2024

Description

Currently, CommandEncoder issues SetDrawColorBuffers after issuing ClearColorF, ClearColorU and ClearColorI. On AMD Renoir graphics on Linux, this results in glClearBuffer calls sometimes being ignored. I can see the glClearBuffer calls in RenderDoc, but the output texture is not cleared. I suspect that this may also be the cause of some of the crashes on other machines (e.g. Sandy Bridge on Windows).

It seems that the correct way to use glClearBuffer is to call glDrawBuffers before calling glClearBuffer.

Additionally, I've replaced the glClear call with glClearBufferF (via glow::clear_buffer_f32_slice), because glDrawBuffers does not like it when you pass COLOR_ATTACHMENTi as the j-th buffer unless i==j. When rendering using a WebGL backend, this results in warnings:

WebGL warning: drawBuffers: `buffers[i]` must be NONE or COLOR_ATTACHMENTi."

On native OpenGL, this doesn't seem to be an issue, but the spec for OpenGL ES 3.2 also demands this:

If the GL is bound to a draw framebuffer object, the ith buffer listed in bufs
must be COLOR_ATTACHMENTi or NONE.

(See page 398, OpenGL ES 3.2)

Testing
Theoretically, issuing SetDrawColorBuffers(0) before beginning a render pass (maybe with a non-default frame-buffer?) and attempting to clear the color buffers will result in the clear commands being ignored on some machines. Admittedly, I've only tested this in a larger application, so I don't have a self-contained test for this.

The changes regarding clearing float buffers can be verified by attempting to clear buffers as part of a render pass involving multiple color attachments in WebGL (both Firefox and Chrome produce similar warnings).

It would be great if someone could verify that this fixes their issues on Sandy Bridge on Windows, because I've removed (a part of) the workaround in hopes that the crashes were caused by this bug(or quirk).

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.

This is necessary for glClearBuffer calls to work correctly on some machines (e.g. AMD Renoir graphics running on Linux). Without this, glClearBuffer calls are ignored.
…ings

This fixes the following WebGL warning: "WebGL warning: drawBuffers: `buffers[i]` must be NONE or COLOR_ATTACHMENTi."

When using native OpenGL, it is acceptable to call glDrawBuffers with an array of buffers where i != COLOR_ATTACHMENTi. In WebGL, this is not allowed.
@nickguletskii nickguletskii marked this pull request as ready for review May 5, 2024 13:35
@nickguletskii nickguletskii requested a review from a team as a code owner May 5, 2024 13:35
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Code looks good! @Zoxc if you can test on sandy bridge, but I lets not block on that

@cwfitzgerald cwfitzgerald merged commit 65d8c94 into gfx-rs:trunk May 14, 2024
25 checks passed
EriKWDev pushed a commit to bitwise-git/wgpu that referenced this pull request May 14, 2024
…buffer_f32_slice instead of clear (gfx-rs#5666)

* Issue SetDrawColorBuffers commands before issuing ClearColor

This is necessary for glClearBuffer calls to work correctly on some machines (e.g. AMD Renoir graphics running on Linux). Without this, glClearBuffer calls are ignored.

* Use clear_buffer_f32_slice instead of gl.clear to suppress WebGL warnings

This fixes the following WebGL warning: "WebGL warning: drawBuffers: `buffers[i]` must be NONE or COLOR_ATTACHMENTi."

When using native OpenGL, it is acceptable to call glDrawBuffers with an array of buffers where i != COLOR_ATTACHMENTi. In WebGL, this is not allowed.

* Run cargo fmt

* Add changes for PR gfx-rsGH-5666 to the CHANGELOG
jimblandy pushed a commit to jimblandy/wgpu that referenced this pull request May 23, 2024
…buffer_f32_slice instead of clear (gfx-rs#5666)

* Issue SetDrawColorBuffers commands before issuing ClearColor

This is necessary for glClearBuffer calls to work correctly on some machines (e.g. AMD Renoir graphics running on Linux). Without this, glClearBuffer calls are ignored.

* Use clear_buffer_f32_slice instead of gl.clear to suppress WebGL warnings

This fixes the following WebGL warning: "WebGL warning: drawBuffers: `buffers[i]` must be NONE or COLOR_ATTACHMENTi."

When using native OpenGL, it is acceptable to call glDrawBuffers with an array of buffers where i != COLOR_ATTACHMENTi. In WebGL, this is not allowed.

* Run cargo fmt

* Add changes for PR gfx-rsGH-5666 to the CHANGELOG
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.

None yet

2 participants