diff --git a/CHANGELOG.md b/CHANGELOG.md index e28391366c..b8627c0ada 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,7 +77,7 @@ the same every time it is rendered, we now warn if it is missing. - Added downlevel restriction error message for `InvalidFormatUsages` error by @Seamooo in [#2886](https://github.com/gfx-rs/wgpu/pull/2886) - Add warning when using CompareFunction::*Equal with vertex shader that is missing @invariant tag by @cwfitzgerald in [#2887](https://github.com/gfx-rs/wgpu/pull/2887) - Update Winit to version 0.27 and raw-window-handle to 0.5 by @wyatt-herkamp in [#2918](https://github.com/gfx-rs/wgpu/pull/2918) - +- Address Clippy 0.1.63 complaints. By @jimb in [#2977](https://github.com/gfx-rs/wgpu/pull/2977) #### Metal - Extract the generic code into `get_metal_layer` by @jinleili in [#2826](https://github.com/gfx-rs/wgpu/pull/2826) @@ -85,6 +85,7 @@ the same every time it is rendered, we now warn if it is missing. #### General - Add WGSL examples to complement existing examples written in GLSL by @norepimorphism in [#2888](https://github.com/gfx-rs/wgpu/pull/2888) +- Document `wgpu_core` resource allocation. @jimb in [#2973](https://github.com/gfx-rs/wgpu/pull/2973) ### Performance diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index ff09999ad5..71f95a723d 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -648,7 +648,7 @@ impl Resource for PipelineLayout { } #[repr(C)] -#[derive(Clone, Debug, Hash, PartialEq)] +#[derive(Clone, Debug, Hash, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct BufferBinding { @@ -784,7 +784,7 @@ pub enum GetBindGroupLayoutError { InvalidGroupIndex(u32), } -#[derive(Clone, Debug, Error, PartialEq)] +#[derive(Clone, Debug, Error, Eq, PartialEq)] #[error("Buffer is bound with size {bound_size} where the shader expects {shader_size} in group[{group_index}] compact index {compact_index}")] pub struct LateMinBufferBindingSizeMismatch { pub group_index: u32, diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index b4510629c7..cbbb2e054e 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -136,7 +136,7 @@ pub struct ComputePassDescriptor<'a> { pub label: Label<'a>, } -#[derive(Clone, Debug, Error, PartialEq)] +#[derive(Clone, Debug, Error, Eq, PartialEq)] pub enum DispatchError { #[error("compute pipeline must be set")] MissingPipeline, diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index 33ee685548..24b0be9d53 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -14,7 +14,7 @@ use std::num::NonZeroU32; use thiserror::Error; /// Error validating a draw call. -#[derive(Clone, Debug, Error, PartialEq)] +#[derive(Clone, Debug, Error, Eq, PartialEq)] pub enum DrawError { #[error("blend constant needs to be set")] MissingBlendConstant, diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index b8246b054f..257bb2edaf 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -149,7 +149,7 @@ impl CommandBuffer { base.buffers.set_from_tracker(&head.buffers); base.textures - .set_from_tracker(&*texture_guard, &head.textures); + .set_from_tracker(texture_guard, &head.textures); Self::drain_barriers(raw, base, buffer_guard, texture_guard); } @@ -165,7 +165,7 @@ impl CommandBuffer { base.buffers.set_from_usage_scope(&head.buffers); base.textures - .set_from_usage_scope(&*texture_guard, &head.textures); + .set_from_usage_scope(texture_guard, &head.textures); Self::drain_barriers(raw, base, buffer_guard, texture_guard); } diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index df235565f1..4e221b3c95 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -71,7 +71,7 @@ pub enum StoreOp { /// Describes an individual channel within a render pass, such as color, depth, or stencil. #[repr(C)] -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] #[cfg_attr(any(feature = "serial-pass", feature = "trace"), derive(Serialize))] #[cfg_attr(any(feature = "serial-pass", feature = "replay"), derive(Deserialize))] pub struct PassChannel { @@ -737,7 +737,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { let view: &TextureView = cmd_buf .trackers .views - .add_single(&*view_guard, at.view) + .add_single(view_guard, at.view) .ok_or(RenderPassErrorInner::InvalidAttachment(at.view))?; check_multiview(view)?; add_view(view, "depth")?; @@ -853,7 +853,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { let color_view: &TextureView = cmd_buf .trackers .views - .add_single(&*view_guard, at.view) + .add_single(view_guard, at.view) .ok_or(RenderPassErrorInner::InvalidAttachment(at.view))?; check_multiview(color_view)?; add_view(color_view, "color")?; @@ -883,7 +883,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { let resolve_view: &TextureView = cmd_buf .trackers .views - .add_single(&*view_guard, resolve_target) + .add_single(view_guard, resolve_target) .ok_or(RenderPassErrorInner::InvalidAttachment(resolve_target))?; check_multiview(resolve_view)?; @@ -1015,7 +1015,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> { self.usage_scope .textures .merge_single( - &*texture_guard, + texture_guard, ra.texture_id.value, Some(ra.selector.clone()), &ra.texture_id.ref_count, diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index afe32801d7..a5c5cbe51c 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -46,7 +46,7 @@ const EP_FAILURE: &str = "EP is invalid"; pub type DeviceDescriptor<'a> = wgt::DeviceDescriptor>; #[repr(C)] -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub enum HostMap { diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 091e785665..fbd0800efe 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -1,3 +1,154 @@ +/*! Allocating resource ids, and tracking the resources they refer to. + +The `wgpu_core` API uses identifiers of type [`Id`] to refer to +resources of type `R`. For example, [`id::DeviceId`] is an alias for +`Id>`, and [`id::BufferId`] is an alias for +`Id>`. `Id` implements `Copy`, `Hash`, `Eq`, `Ord`, and +of course `Debug`. + +Each `Id` contains not only an index for the resource it denotes but +also a [`Backend`] indicating which `wgpu` backend it belongs to. You +can use the [`gfx_select`] macro to dynamically dispatch on an id's +backend to a function specialized at compile time for a specific +backend. See that macro's documentation for details. + +`Id`s also incorporate a generation number, for additional validation. + +The resources to which identifiers refer are freed explicitly. +Attempting to use an identifier for a resource that has been freed +elicits an error result. + +## Assigning ids to resources + +The users of `wgpu_core` generally want resource ids to be assigned +in one of two ways: + +- Users like `wgpu` want `wgpu_core` to assign ids to resources itself. + For example, `wgpu` expects to call `Global::device_create_buffer` + and have the return value indicate the newly created buffer's id. + +- Users like `player` and Firefox want to allocate ids themselves, and + pass `Global::device_create_buffer` and friends the id to assign the + new resource. + +To accommodate either pattern, `wgpu_core` methods that create +resources all expect an `id_in` argument that the caller can use to +specify the id, and they all return the id used. For example, the +declaration of `Global::device_create_buffer` looks like this: + +```ignore +impl Global { + /* ... */ + pub fn device_create_buffer( + &self, + device_id: id::DeviceId, + desc: &resource::BufferDescriptor, + id_in: Input, + ) -> (id::BufferId, Option) { + /* ... */ + } + /* ... */ +} +``` + +Users that want to assign resource ids themselves pass in the id they +want as the `id_in` argument, whereas users that want `wgpu_core` +itself to choose ids always pass `()`. In either case, the id +ultimately assigned is returned as the first element of the tuple. + +Producing true identifiers from `id_in` values is the job of an +[`IdentityHandler`] implementation, which has an associated type +[`Input`] saying what type of `id_in` values it accepts, and a +[`process`] method that turns such values into true identifiers of +type `I`. There are two kinds of `IdentityHandler`s: + +- Users that want `wgpu_core` to assign ids generally use + [`IdentityManager`] ([wrapped in a mutex]). Its `Input` type is + `()`, and it tracks assigned ids and generation numbers as + necessary. (This is what `wgpu` does.) + +- Users that want to assign ids themselves use an `IdentityHandler` + whose `Input` type is `I` itself, and whose `process` method simply + passes the `id_in` argument through unchanged. For example, the + `player` crate uses an `IdentityPassThrough` type whose `process` + method simply adjusts the id's backend (since recordings can be + replayed on a different backend than the one they were created on) + but passes the rest of the id's content through unchanged. + +Because an `IdentityHandler` can only create ids for a single +resource type `I`, constructing a [`Global`] entails constructing a +separate `IdentityHandler` for each resource type `I` that the +`Global` will manage: an `IdentityHandler`, an +`IdentityHandler`, and so on. + +The [`Global::new`] function could simply take a large collection of +`IdentityHandler` implementations as arguments, but that would be +ungainly. Instead, `Global::new` expects a `factory` argument that +implements the [`GlobalIdentityHandlerFactory`] trait, which extends +[`IdentityHandlerFactory`] for each resource id type `I`. This +trait, in turn, has a `spawn` method that constructs an +`IdentityHandler` for the `Global` to use. + +What this means is that the types of resource creation functions' +`id_in` arguments depend on the `Global`'s `G` type parameter. A +`Global`'s `IdentityHandler` implementation is: + +```ignore +>::Filter +``` + +where `Filter` is an associated type of the `IdentityHandlerFactory` trait. +Thus, its `id_in` type is: + +```ignore +<>::Filter as IdentityHandler>::Input +``` + +The [`Input`] type is an alias for this construction. + +## Id allocation and streaming + +Perhaps surprisingly, allowing users to assign resource ids themselves +enables major performance improvements in some applications. + +The `wgpu_core` API is designed for use by Firefox's [WebGPU] +implementation. For security, web content and GPU use must be kept +segregated in separate processes, with all interaction between them +mediated by an inter-process communication protocol. As web content uses +the WebGPU API, the content process sends messages to the GPU process, +which interacts with the platform's GPU APIs on content's behalf, +occasionally sending results back. + +In a classic Rust API, a resource allocation function takes parameters +describing the resource to create, and if creation succeeds, it returns +the resource id in a `Result::Ok` value. However, this design is a poor +fit for the split-process design described above: content must wait for +the reply to its buffer-creation message (say) before it can know which +id it can use in the next message that uses that buffer. On a common +usage pattern, the classic Rust design imposes the latency of a full +cross-process round trip. + +We can avoid incurring these round-trip latencies simply by letting the +content process assign resource ids itself. With this approach, content +can choose an id for the new buffer, send a message to create the +buffer, and then immediately send the next message operating on that +buffer, since it already knows its id. Allowing content and GPU process +activity to be pipelined greatly improves throughput. + +To help propagate errors correctly in this style of usage, when resource +creation fails, the id supplied for that resource is marked to indicate +as much, allowing subsequent operations using that id to be properly +flagged as errors as well. + +[`gfx_select`]: crate::gfx_select +[`Input`]: IdentityHandler::Input +[`process`]: IdentityHandler::process +[`Id`]: crate::id::Id +[wrapped in a mutex]: trait.IdentityHandler.html#impl-IdentityHandler%3CI%3E-for-Mutex%3CIdentityManager%3E +[WebGPU]: https://www.w3.org/TR/webgpu/ + +*/ + use crate::{ binding_model::{BindGroup, BindGroupLayout, PipelineLayout}, command::{CommandBuffer, RenderBundle}, @@ -36,6 +187,9 @@ use std::{fmt::Debug, marker::PhantomData, mem, ops}; /// - `IdentityManager` reuses the index values of freed ids before returning /// ids with new index values. Freed vector entries get reused. /// +/// See the module-level documentation for an overview of how this +/// fits together. +/// /// [`Id`]: crate::id::Id /// [`Backend`]: wgt::Backend; /// [`alloc`]: IdentityManager::alloc @@ -431,9 +585,24 @@ impl<'a, T> Drop for Token<'a, T> { } } +/// A type that can build true ids from proto-ids, and free true ids. +/// +/// For some implementations, the true id is based on the proto-id. +/// The caller is responsible for providing well-allocated proto-ids. +/// +/// For other implementations, the proto-id carries no information +/// (it's `()`, say), and this `IdentityHandler` type takes care of +/// allocating a fresh true id. +/// +/// See the module-level documentation for details. pub trait IdentityHandler: Debug { + /// The type of proto-id consumed by this filter, to produce a true id. type Input: Clone + Debug; + + /// Given a proto-id value `id`, return a true id for `backend`. fn process(&self, id: Self::Input, backend: Backend) -> I; + + /// Free the true id `id`. fn free(&self, id: I); } @@ -447,11 +616,28 @@ impl IdentityHandler for Mutex { } } +/// A type that can produce [`IdentityHandler`] filters for ids of type `I`. +/// +/// See the module-level documentation for details. pub trait IdentityHandlerFactory { + /// The type of filter this factory constructs. + /// + /// "Filter" and "handler" seem to both mean the same thing here: + /// something that can produce true ids from proto-ids. type Filter: IdentityHandler; + + /// Create an [`IdentityHandler`] implementation that can + /// transform proto-ids into ids of type `I`. + /// + /// [`IdentityHandler`]: IdentityHandler fn spawn(&self) -> Self::Filter; } +/// A global identity handler factory based on [`IdentityManager`]. +/// +/// Each of this type's `IdentityHandlerFactory::spawn` methods +/// returns a `Mutex>`, which allocates fresh `I` +/// ids itself, and takes `()` as its proto-id type. #[derive(Debug)] pub struct IdentityManagerFactory; @@ -462,6 +648,8 @@ impl IdentityHandlerFactory for IdentityManagerFactor } } +/// A factory that can build [`IdentityHandler`]s for all resource +/// types. pub trait GlobalIdentityHandlerFactory: IdentityHandlerFactory + IdentityHandlerFactory diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 1721d41de2..dd66b48d14 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -479,7 +479,7 @@ impl Borrow for Texture { } /// Describes a [`TextureView`]. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize), serde(default))] pub struct TextureViewDescriptor<'a> { diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index c906ceb04d..7cf822e4cf 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -238,7 +238,7 @@ fn iterate_bitvec_indices(ownership: &BitVec) -> impl Iterator { current_state: T, new_state: T, diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index fcaf03946f..02d3c13af1 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -106,7 +106,7 @@ pub type Label<'a> = Option<&'a str>; pub type MemoryRange = Range; pub type FenceValue = u64; -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Eq, PartialEq, Error)] pub enum DeviceError { #[error("out of memory")] OutOfMemory, @@ -114,7 +114,7 @@ pub enum DeviceError { Lost, } -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Eq, PartialEq, Error)] pub enum ShaderError { #[error("compilation failed: {0:?}")] Compilation(String), @@ -122,7 +122,7 @@ pub enum ShaderError { Device(#[from] DeviceError), } -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Eq, PartialEq, Error)] pub enum PipelineError { #[error("linkage failed for stage {0:?}: {1}")] Linkage(wgt::ShaderStages, String), @@ -132,7 +132,7 @@ pub enum PipelineError { Device(#[from] DeviceError), } -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Eq, PartialEq, Error)] pub enum SurfaceError { #[error("surface is lost")] Lost, @@ -144,7 +144,7 @@ pub enum SurfaceError { Other(&'static str), } -#[derive(Clone, Debug, PartialEq, Error)] +#[derive(Clone, Debug, Eq, PartialEq, Error)] #[error("Not supported")] pub struct InstanceError; @@ -1032,7 +1032,7 @@ pub struct RenderPipelineDescriptor<'a, A: Api> { /// Specifies how the alpha channel of the textures should be handled during (martin mouv i step) /// compositing. -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum CompositeAlphaMode { /// The alpha channel, if it exists, of the textures is ignored in the /// compositing process. Instead, the textures is treated as if it has a diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 5072a2445d..cf53098a83 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -1113,7 +1113,7 @@ pub enum ShaderModel { /// Supported physical device types. #[repr(u8)] -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub enum DeviceType { @@ -1132,7 +1132,7 @@ pub enum DeviceType { //TODO: convert `vendor` and `device` to `u32` /// Information about an adapter. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub struct AdapterInfo { @@ -4115,7 +4115,7 @@ pub struct ImageCopyTexture { /// Subresource range within an image #[repr(C)] -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] #[cfg_attr(feature = "replay", derive(serde::Deserialize))] pub struct ImageSubresourceRange { diff --git a/wgpu/examples/water/point_gen.rs b/wgpu/examples/water/point_gen.rs index 21465e6ea8..bb657c8e0d 100644 --- a/wgpu/examples/water/point_gen.rs +++ b/wgpu/examples/water/point_gen.rs @@ -40,7 +40,7 @@ pub struct TerrainVertexAttributes { } #[repr(C)] -#[derive(Copy, Clone, Debug, PartialEq, Pod, Zeroable)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Pod, Zeroable)] pub struct WaterVertexAttributes { position: [i16; 2], offsets: [i8; 4], diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 14a93731e2..ad94bd5b1c 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -46,7 +46,7 @@ pub use wgt::{ use backend::{BufferMappedRange, Context as C, QueueWriteBuffer}; /// Filter for error scopes. -#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)] pub enum ErrorFilter { /// Catch only out-of-memory errors. OutOfMemory, @@ -1295,7 +1295,7 @@ pub type Maintain = wgt::Maintain; /// /// Corresponds to [WebGPU `GPUTextureViewDescriptor`]( /// https://gpuweb.github.io/gpuweb/#dictdef-gputextureviewdescriptor). -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct TextureViewDescriptor<'a> { /// Debug label of the texture view. This will show up in graphics debuggers for easy identification. pub label: Label<'a>, @@ -2262,7 +2262,7 @@ impl Display for BufferAsyncError { impl error::Error for BufferAsyncError {} /// Type of buffer mapping. -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum MapMode { /// Map only for reading Read, diff --git a/wgpu/tests/common/image.rs b/wgpu/tests/common/image.rs index 4af7419415..1c0e3396bb 100644 --- a/wgpu/tests/common/image.rs +++ b/wgpu/tests/common/image.rs @@ -62,7 +62,7 @@ fn write_png( } fn calc_difference(lhs: u8, rhs: u8) -> u8 { - (lhs as i16 - rhs as i16).abs() as u8 + (lhs as i16 - rhs as i16).unsigned_abs() as u8 } pub fn compare_image_output( diff --git a/wgpu/tests/example_wgsl.rs b/wgpu/tests/example_wgsl.rs index db51d8fc59..d645dd4ed3 100644 --- a/wgpu/tests/example_wgsl.rs +++ b/wgpu/tests/example_wgsl.rs @@ -28,7 +28,7 @@ fn parse_example_wgsl() { for file_entry in read_files { let shader = match file_entry { Ok(entry) => match entry.path().extension() { - Some(ostr) if &*ostr == "wgsl" => { + Some(ostr) if ostr == "wgsl" => { println!("Validating {:?}", entry.path()); fs::read_to_string(entry.path()).unwrap_or_default() } diff --git a/wgpu/tests/zero_init_texture_after_discard.rs b/wgpu/tests/zero_init_texture_after_discard.rs index cce48c0fe8..6043d0388f 100644 --- a/wgpu/tests/zero_init_texture_after_discard.rs +++ b/wgpu/tests/zero_init_texture_after_discard.rs @@ -67,6 +67,7 @@ fn discarding_color_target_resets_texture_init_state_check_visible_on_copy_in_sa } #[test] +#[allow(clippy::single_element_loop)] fn discarding_depth_target_resets_texture_init_state_check_visible_on_copy_in_same_encoder() { initialize_test( TestParameters::default()