-
Notifications
You must be signed in to change notification settings - Fork 969
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
Make it possible to filter labels out ahead of wgpu-hal #4246
Conversation
There are still a couple of internal labels that are missed (because the flag was not easily reachable). They are less important for hardening since they short and ascii-only, but we can look into filtering them as well for consistency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this PR is missing fixes for the following labels:
-
wgpu_hal::RenderPassDescriptor::label
, passed inwgpu_core::render::RenderPassInfo::start
-
wgpu_types::QuerySetDescriptor::label
, passed inwgpu_core::Device::create_query_set
- although this is not currently used by Firefox (GPUDevice.createQuerySet
is commented out in the WebIDL), we probably should address it now -
wgpu_types::RenderBundleDescriptor::label
, whichRenderBundle::execute
passes towgpu_hal::CommandEncoder::begin_debug_marker
- It seems like
wgpu_hal::CommandEncoder::begin_encoding
gets passedwgpu_core::command::CommandEncoder::label
, which it seems can be set fromwgpu_types::CommandEncoderDescriptor::label
. -
Global::command_encoder_run_render_pass_impl
takes aBasePassRef
as an argument whoselabel
field is passed towgpu_core::command::CommandEncoder::open_pass
, which passes it towgpu_hal::CommandEncoder::begin_encoding
. -
wgpu_hal::CommandEncoder::insert_debug_marker
seems to take labels from compute pass encoders, and is also called fromGloal::command_encoder_insert_debug_marker
. I don't know why we have two ways of doing this, but they both seem reachable from Firefox. - It seems like
wgpu_hal::CommandEncoder::begin_debug_marker
is also reachable.
I marked this as "changes requested", but the PR as it stands is a step in the right direction. If you'd like to address the cases I found in a follow-up, that's fine with me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came across a few missed labels, and some problems with string processing in render passes and compute passes. I've added a fixup commit; please look it over.
…8 strings unless we use them
Checklist
cargo clippy
.Description
This is intended as a hardening feature. If we don't trust the author of labels (for example web content) and don't trust drivers to handle long or non-ascii strings, an instance flags can be set to filter out hal labels. When the flag is set, wgpu-core can still hold on to labels but they are not passed down to GPU drivers.
Most people should not enable this flag. Labels are pretty useful for debugging with renderdoc and similar tools.