Skip to content

Allow the "gles" backend to (theoretically) compile on no_std targets. #7267

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

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

kpreid
Copy link
Collaborator

@kpreid kpreid commented Mar 5, 2025

Connections
Part of #6826.

Description
Removes std usage from the gles backend in wgpu-hal, allowing wgpu-hal to build on no_std targets like wasm32v1-none (once its dependencies, notably naga, are all no_std compatible too).

This entirely consists of conditionally replacing Mutex with RefCell, then making sure that this doesn’t accidentally happen if we are also exposing Send + Sync.

I’m not very confident in the design of this change, but I tried to do it in a way which at least doesn’t add any unsound configurations.

Testing
Confirmed the backend still builds on native and wasm, and ran the test suite.

Squash or Rebase?
Rebase

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@kpreid kpreid requested a review from a team as a code owner March 5, 2025 05:02
@@ -104,6 +104,7 @@ gles = [
"naga/glsl-out",
"dep:arrayvec",
"dep:bytemuck",
"dep:cfg-if",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I notice that before this change, cfg-if is only a dev-dependency, never a library dependency. I assume there is no strong reason for that; I can rewrite the code to use only #[cfg] if desired.

Copy link
Member

Choose a reason for hiding this comment

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

it's an absolutely tiny dependency and makes code look nicer here, don't mind it :)

@kpreid
Copy link
Collaborator Author

kpreid commented Mar 5, 2025

Ah, I see I broke Emscripten. It’s late, so I will think about that another day.

@kpreid kpreid marked this pull request as draft March 5, 2025 05:07
…gets.

This entirely consists of conditionally replacing `Mutex` with `RefCell`,
then making sure that this doesn’t accidentally happen if we are also
exposing `Send + Sync`.
@kpreid kpreid marked this pull request as ready for review March 5, 2025 06:07
@kpreid
Copy link
Collaborator Author

kpreid commented Mar 5, 2025

There we go. I just forgot that cfg(target = "...") isn’t a thing.

Copy link
Member

@Wumpf Wumpf 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. Changes are non-invasive enough imho
We should follow-up though with a way to check this configuration on ci, lest someone adds e.g. a new mutex or similar again

@Wumpf Wumpf merged commit 289d0e5 into gfx-rs:trunk Mar 5, 2025
34 checks passed
@kpreid kpreid deleted the gles-no-std branch March 5, 2025 14:39
@kpreid
Copy link
Collaborator Author

kpreid commented Mar 5, 2025

A sufficient CI test would be

cargo (check|build|clippy) -p wgpu-hal --features gles

but once #6826 is closer to done, we can and should run builds on actual doesn’t-have-std targets, which will be even stronger.

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