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

Limits fallback #302

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Conversation

rajveermalviya
Copy link
Collaborator

@rajveermalviya rajveermalviya commented Oct 4, 2023

Fixes #297

Context

Support creating devices with lower than Limits::default() limits, by falling back to Limits::downlevel_default() if adapter's limits are lower than Limits::default(). (checked by using wgt::Limits::check_limits())

Testing

untested, but added a unit test

Copy link
Collaborator

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm not fully grasping how this works in practice.

IIUC, when you have an adapter that supports the default limits, it will create a device with these limits (unless the user specifies specific limits). If there is (at least) one default limit that the adapter cannot provide, the default limits (all of 'm) fall back to a lower standard (GLES3 compatible-ish).

@rajveermalviya
Copy link
Collaborator Author

IIUC, when you have an adapter that supports the default limits, it will create a device with these limits (unless the user specifies specific limits). If there is (at least) one default limit that the adapter cannot provide, the default limits (all of 'm) fall back to a lower standard (GLES3 compatible-ish).

Correct, also do note that these default limits are only used in following conditions:

  • got NULL for descriptor in wgpuAdapterRequestDevice
  • got NULL for WGPUDeviceDescriptor.requiredLimits
  • used as base for limits with value of WGPU_LIMIT_[U32/U64]_UNDEFINED for WGPUDeviceDescriptor.requiredLimits

If user is relying on these defaults, then with this behaviour user would have to only worry about two possible levels of device types, one with default webgpu limits another with downlevel limits. If we do something else like try to use the webgpu default limits and clamping to available adapter limits - then it would no longer be the case.

A more critical user should not rely on these defaults, they should do their own investigation of possible device limits for their app/game and pass them for device creation.

@rajveermalviya

This comment was marked as off-topic.

@rajveermalviya

This comment was marked as off-topic.

@almarklein
Copy link
Collaborator

So what happens when the user provides values for some limits, and WGPU_LIMIT_[U32/U64]_UNDEFINED for some others, and for one of these unspecified limits the adapter is not able to provide the webgpu defaults. Would it fall back to the downlevel limit (for that limit only) or fail to create the device?

This PR can be merged regardless of the answer, as far as I am concerned 👍

@rajveermalviya
Copy link
Collaborator Author

Currently, if user provides some limit values and provides undefined for some (WGPU_LIMIT_[U32/U64]_UNDEFINED), then wgpu-native will use webgpu default limits for the undefined ones, and in case even one of the adapter supported limit is lower than the corresponding webgpu default limit then wgpu-native will use downlevel limits for all those undefined ones instead of webgpu default limits. For the non-undefined ones the values provided will be used as-is.

@rajveermalviya
Copy link
Collaborator Author

And in case of limits that are exposed by wgpu-core are not present in webgpu.h we provide those extra limits in wgpu.h. If user doesn't provide the ext in nextInChain then wgpu-native will use whatever default/downlevel_default value for those limits.

@almarklein
Copy link
Collaborator

Thanks for the explanation! All clear now 😄

@rajveermalviya rajveermalviya merged commit 24e00f4 into gfx-rs:trunk Oct 9, 2023
16 checks passed
@rajveermalviya rajveermalviya deleted the limits-fallback branch October 9, 2023 08:09
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.

Adapters with lower-than-default limits
2 participants