Allow overriding Instance::new#9606
Open
inner-daemons wants to merge 2 commits into
Open
Conversation
inner-daemons
force-pushed
the
custom-backend-override
branch
from
df0d2b3 to
2324dbb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in mechanism (gated on the custom cfg) to override Instance::new with a user-registered factory function, primarily so a custom backend (e.g. wgpu-native) can intercept instance creation in third-party code and the wgpu test suite. A new BackendOptions::skip_custom_backend_library flag lets a specific call site bypass the override, and wgpu-info gains a regression test that compares a custom-backend report against the wgpu-core report.
Changes:
- New global
set_instance_factory+INSTANCE_FACTORYatomic inwgpu/src/api/instance.rs, consulted fromInstance::newunlessskip_custom_backend_libraryis set. - New
BackendOptions::skip_custom_backend_libraryfield plusWGPU_NO_CUSTOM_BACKENDenv var handling inwgpu-types, with knock-on updates indeno_webgpu,tests/src/init.rs, andwgpu-info. - New
custom_backend_matches_wgpu_coretest inwgpu-info/src/tests.rsand small robustness changes intests/tests/wgpu-gpu/{device,mem_leaks}.rssogenerate_reportreturningNonedoesn't fail.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| wgpu/src/api/instance.rs | Adds set_instance_factory and dispatch through it from Instance::new. |
| wgpu-types/src/backend.rs | Adds skip_custom_backend_library field with env-var handling in from_env_or_default (but not with_env). |
| wgpu-info/src/report.rs | GpuReport::generate now takes a skip_custom flag. |
| wgpu-info/src/cli.rs | Updates CLI caller to pass false. |
| wgpu-info/src/tests.rs | New test comparing custom-backend vs wgpu-core reports. |
| tests/src/init.rs | Explicitly initializes the new field to false. |
| tests/tests/wgpu-gpu/device.rs | Tolerates generate_report returning None for custom backends. |
| tests/tests/wgpu-gpu/mem_leaks.rs | Same: early-return when no report is available. |
| deno_webgpu/lib.rs | Sets skip_custom_backend_library: true (deno does not want overrides). |
| CHANGELOG.md | Documents the new feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+222
| /// If false and the `custom` feature is enabled for wgpu, and if an override | ||
| /// instance factory was setup, wgpu will return a custom instance created from | ||
| /// that factory. | ||
| /// | ||
| /// Noop on wgpu-core. |
Comment on lines
+7
to
+26
| static INSTANCE_FACTORY: core::sync::atomic::AtomicUsize = core::sync::atomic::AtomicUsize::new(0); | ||
|
|
||
| /// Register a factory that can intercept [`Instance::new`] for custom backends. | ||
| /// | ||
| /// The factory receives the [`InstanceDescriptor`] and returns `Ok(Instance)` to | ||
| /// take ownership of the request, or `Err(desc)` to fall through to the built-in backend. | ||
| /// | ||
| /// Only the first call takes effect; subsequent calls are ignored. | ||
| /// | ||
| /// This can be safely called from a constructor. | ||
| #[cfg(custom)] | ||
| pub fn set_instance_factory(f: fn(InstanceDescriptor) -> Result<Instance, InstanceDescriptor>) { | ||
| // 0 is the sentinel for "not set"; fn pointers are never null. | ||
| let _ = INSTANCE_FACTORY.compare_exchange( | ||
| 0, | ||
| f as usize, | ||
| core::sync::atomic::Ordering::Release, | ||
| core::sync::atomic::Ordering::Relaxed, | ||
| ); | ||
| } |
Comment on lines
+6
to
+16
| let dir = std::env::temp_dir(); | ||
| let a_path = dir.join(format!("wgpu-info-{label}-custom.json")); | ||
| let b_path = dir.join(format!("wgpu-info-{label}-core.json")); | ||
| fs::write(&a_path, a).unwrap(); | ||
| fs::write(&b_path, b).unwrap(); | ||
| let out = Command::new("diff") | ||
| .args(["-u", "--label", "with-custom", "--label", "without-custom"]) | ||
| .args([&a_path, &b_path]) | ||
| .output() | ||
| .unwrap(); | ||
| String::from_utf8(out.stdout).unwrap() |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Member
|
Didn't we say we weren't going to do it this way? |
Collaborator
Author
|
@cwfitzgerald I realized that this would be more useful to me plus I'm still not convinced it's a bad idea. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Connections
Closes #9595
Split out from #9570.
Description
This is mainly useful for experimentation. It lets you change the behavior of
Instance::new, though users ofInstance::newcan still opt-out of this change.It lets you override the behavior of
Instance::newto return a custom instance (one that doesn't necessarily use webgpu or wgpu-core).My use case is for running wgpu's tests on an external backend (see gfx-rs/wgpu-native#594) but it can also be used to force other libraries or apps to use your own backend, if there isn't a more convenient way.
This shouldn't be a security issue because A. it only affects wgpu, so deno and Firefox are unaffected, and B. it requires a specific function to be called before the instance is created.
Testing
Tested in gfx-rs/wgpu-native#594. That PR has a test which ensures that the custom instance is actually used.
Squash or Rebase?
Squash
Checklist
wgpumay be affected behaviorally.CHANGELOG.mdentries for the user-facing effects of this change are present.