Skip to content

Conversation

@andyleiserson
Copy link
Contributor

On top of #8289. At time of writing, the last three commits are for this change.

This adds support for storing commands that resulted in an error (either during encoding, or on submission) in a trace. It also adds a simple test for tracing and for the new error tracing functionality.

Squash or Rebase? Rebase

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. 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.

@andyleiserson
Copy link
Contributor Author

Responding to #8289 (comment):

I just realized Fallible::Invalid also has type Arc - we could use that address to be able to record before validation.

ID resolution was already largely separated from validation. What I've implemented here will not write to the trace if ID resolution fails, but will write to the trace if validation fails. This seems like it should cover most use cases -- ID resolution generally shouldn't fail, and if it does fail, it seems like we want more detail about every ID/pointer association than a regular trace would contain.

Fallible::Invalid could work, but since we use the same data structures as the input to encoding, we'd either need a bunch more unwraps, error handling, or an additional transformation step to remove the Fallible layer before encoding.

/// Stop tracing and return the trace object.
///
/// This is mostly useful for in-memory traces.
pub fn take_trace(&self) -> Option<Box<dyn trace::Trace + Send + Sync + 'static>> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't work, because Trace isn't even defined when the feature isn't enabled.

There is a comment on wgt::Trace:

// This enum must be non-exhaustive so that enabling the "trace" feature is not a semver break.

I'm not sure I fully understand the concern here. Would it be a problem if take_trace is only available when the feature is enabled?

# `cargo clippy -- -Wunused-crate-dependencies` give fewer false positives.
[dependencies]
wgpu = { workspace = true, features = ["noop"] }
wgpu-core = { workspace = true, features = ["trace"] }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about this. Possibly it should be a separate build?

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.

1 participant