Skip to content

Commit

Permalink
Relax render pass color_attachments validation (gfx-rs#2778)
Browse files Browse the repository at this point in the history
* Make the color attachments `Option`-al in render pipelines, render passes, and render bundles

* vk:  `Option`-al color attachments support

* dx12: sparse color_attachments support

* Only non-hole attachments is supported on wasm target and gl backend

* deno_webgpu: `Option`-al color attachments support

* Follow all suggestions
  • Loading branch information
jinleili authored Jun 27, 2022
1 parent 892c272 commit 61796b1
Show file tree
Hide file tree
Showing 46 changed files with 484 additions and 374 deletions.
2 changes: 1 addition & 1 deletion deno_webgpu/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl Resource for WebGpuRenderBundle {
pub struct CreateRenderBundleEncoderArgs {
device_rid: ResourceId,
label: Option<String>,
color_formats: Vec<wgpu_types::TextureFormat>,
color_formats: Vec<Option<wgpu_types::TextureFormat>>,
depth_stencil_format: Option<wgpu_types::TextureFormat>,
sample_count: u32,
depth_read_only: bool,
Expand Down
53 changes: 29 additions & 24 deletions deno_webgpu/src/command_encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
state: &mut OpState,
command_encoder_rid: ResourceId,
label: Option<String>,
color_attachments: Vec<GpuRenderPassColorAttachment>,
color_attachments: Vec<Option<GpuRenderPassColorAttachment>>,
depth_stencil_attachment: Option<GpuRenderPassDepthStencilAttachment>,
_occlusion_query_set: Option<u32>, // not yet implemented
) -> Result<WebGpuResult, AnyError> {
Expand All @@ -89,30 +89,35 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
let color_attachments = color_attachments
.into_iter()
.map(|color_attachment| {
let texture_view_resource = state
.resource_table
.get::<super::texture::WebGpuTextureView>(color_attachment.view)?;

let resolve_target = color_attachment
.resolve_target
.map(|rid| {
state
.resource_table
.get::<super::texture::WebGpuTextureView>(rid)
let rp_at = if let Some(at) = color_attachment.as_ref() {
let texture_view_resource = state
.resource_table
.get::<super::texture::WebGpuTextureView>(at.view)?;

let resolve_target = at
.resolve_target
.map(|rid| {
state
.resource_table
.get::<super::texture::WebGpuTextureView>(rid)
})
.transpose()?
.map(|texture| texture.0);

Some(wgpu_core::command::RenderPassColorAttachment {
view: texture_view_resource.0,
resolve_target,
channel: wgpu_core::command::PassChannel {
load_op: at.load_op,
store_op: at.store_op,
clear_value: at.clear_value.unwrap_or_default(),
read_only: false,
},
})
.transpose()?
.map(|texture| texture.0);

Ok(wgpu_core::command::RenderPassColorAttachment {
view: texture_view_resource.0,
resolve_target,
channel: wgpu_core::command::PassChannel {
load_op: color_attachment.load_op,
store_op: color_attachment.store_op,
clear_value: color_attachment.clear_value.unwrap_or_default(),
read_only: false,
},
})
} else {
None
};
Ok(rp_at)
})
.collect::<Result<Vec<_>, AnyError>>()?;

Expand Down
2 changes: 1 addition & 1 deletion deno_webgpu/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl From<GpuMultisampleState> for wgpu_types::MultisampleState {
#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct GpuFragmentState {
targets: Vec<wgpu_types::ColorTargetState>,
targets: Vec<Option<wgpu_types::ColorTargetState>>,
module: u32,
entry_point: String,
// TODO(lucacasonato): constants
Expand Down
6 changes: 3 additions & 3 deletions deno_webgpu/webgpu.idl
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ dictionary GPUMultisampleState {
};

dictionary GPUFragmentState : GPUProgrammableStage {
required sequence<GPUColorTargetState> targets;
required sequence<GPUColorTargetState?> targets;
};

dictionary GPUColorTargetState {
Expand Down Expand Up @@ -908,7 +908,7 @@ GPURenderPassEncoder includes GPUBindingCommandsMixin;
GPURenderPassEncoder includes GPURenderCommandsMixin;

dictionary GPURenderPassDescriptor : GPUObjectDescriptorBase {
required sequence<GPURenderPassColorAttachment> colorAttachments;
required sequence<GPURenderPassColorAttachment?> colorAttachments;
GPURenderPassDepthStencilAttachment depthStencilAttachment;
GPUQuerySet occlusionQuerySet;
};
Expand Down Expand Up @@ -947,7 +947,7 @@ enum GPUStoreOp {
};

dictionary GPURenderPassLayout: GPUObjectDescriptorBase {
required sequence<GPUTextureFormat> colorFormats;
required sequence<GPUTextureFormat?> colorFormats;
GPUTextureFormat depthStencilFormat;
GPUSize32 sampleCount = 1;
};
Expand Down
8 changes: 4 additions & 4 deletions player/tests/data/quad.ron
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@
entry_point: "fs_main",
),
targets: [
(
Some((
format: rgba8unorm,
),
)),
],
)),
),
Expand All @@ -90,7 +90,7 @@
push_constant_data: [],
),
target_colors: [
(
Some((
view: Id(0, 1, Empty),
resolve_target: None,
channel: (
Expand All @@ -104,7 +104,7 @@
),
read_only: false,
),
),
)),
],
target_depth_stencil: None,
),
Expand Down
4 changes: 2 additions & 2 deletions player/tests/data/zero-init-texture-rendertarget.ron
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
push_constant_data: [],
),
target_colors: [
(
Some((
view: Id(0, 1, Empty),
resolve_target: None,
channel: (
Expand All @@ -57,7 +57,7 @@
),
read_only: false,
),
),
)),
],
target_depth_stencil: None,
),
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub struct RenderBundleEncoderDescriptor<'a> {
pub label: Label<'a>,
/// The formats of the color attachments that this render bundle is capable to rendering to. This
/// must match the formats of the color attachments in the renderpass this render bundle is executed in.
pub color_formats: Cow<'a, [wgt::TextureFormat]>,
pub color_formats: Cow<'a, [Option<wgt::TextureFormat>]>,
/// Information about the depth attachment that this render bundle is capable to rendering to. The format
/// must match the format of the depth attachments in the renderpass this render bundle is executed in.
pub depth_stencil: Option<wgt::RenderBundleDepthStencil>,
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,15 +408,15 @@ fn clear_texture_via_render_passes<A: hal::Api>(
for depth_or_layer in layer_or_depth_range {
let color_attachments_tmp;
let (color_attachments, depth_stencil_attachment) = if is_color {
color_attachments_tmp = [hal::ColorAttachment {
color_attachments_tmp = [Some(hal::ColorAttachment {
target: hal::Attachment {
view: dst_texture.get_clear_view(mip_level, depth_or_layer),
usage: hal::TextureUses::COLOR_TARGET,
},
resolve_target: None,
ops: hal::AttachmentOps::STORE,
clear_value: wgt::Color::TRANSPARENT,
}];
})];
(&color_attachments_tmp[..], None)
} else {
(
Expand Down
44 changes: 28 additions & 16 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl RenderPassDepthStencilAttachment {
pub struct RenderPassDescriptor<'a> {
pub label: Label<'a>,
/// The color attachments of the render pass.
pub color_attachments: Cow<'a, [RenderPassColorAttachment]>,
pub color_attachments: Cow<'a, [Option<RenderPassColorAttachment>]>,
/// The depth and stencil attachment of the render pass, if any.
pub depth_stencil_attachment: Option<&'a RenderPassDepthStencilAttachment>,
}
Expand All @@ -182,7 +182,7 @@ pub struct RenderPassDescriptor<'a> {
pub struct RenderPass {
base: BasePass<RenderCommand>,
parent_id: id::CommandEncoderId,
color_targets: ArrayVec<RenderPassColorAttachment, { hal::MAX_COLOR_ATTACHMENTS }>,
color_targets: ArrayVec<Option<RenderPassColorAttachment>, { hal::MAX_COLOR_ATTACHMENTS }>,
depth_stencil_target: Option<RenderPassDepthStencilAttachment>,

// Resource binding dedupe state.
Expand Down Expand Up @@ -441,7 +441,7 @@ pub enum RenderPassErrorInner {
InvalidDepthStencilAttachmentFormat(wgt::TextureFormat),
#[error("attachment format {0:?} can not be resolved")]
UnsupportedResolveTargetFormat(wgt::TextureFormat),
#[error("necessary attachments are missing")]
#[error("missing color or depth_stencil attachments, at least one is required.")]
MissingAttachments,
#[error("attachments have differing sizes: {previous:?} is followed by {mismatch:?}")]
AttachmentsDimensionMismatch {
Expand Down Expand Up @@ -646,7 +646,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
fn start(
device: &Device<A>,
label: Option<&str>,
color_attachments: &[RenderPassColorAttachment],
color_attachments: &[Option<RenderPassColorAttachment>],
depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>,
cmd_buf: &mut CommandBuffer<A>,
view_guard: &'a Storage<TextureView<A>, id::TextureViewId>,
Expand Down Expand Up @@ -722,11 +722,15 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
expected: sample_count,
});
}
if sample_count != 1 && sample_count != 4 {
return Err(RenderPassErrorInner::InvalidSampleCount(sample_count));
}
attachment_type_name = type_name;
Ok(())
};

let mut colors = ArrayVec::<hal::ColorAttachment<A>, { hal::MAX_COLOR_ATTACHMENTS }>::new();
let mut colors =
ArrayVec::<Option<hal::ColorAttachment<A>>, { hal::MAX_COLOR_ATTACHMENTS }>::new();
let mut depth_stencil = None;

if let Some(at) = depth_stencil_attachment {
Expand Down Expand Up @@ -840,6 +844,12 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
}

for at in color_attachments {
let at = if let Some(attachment) = at.as_ref() {
attachment
} else {
colors.push(None);
continue;
};
let color_view: &TextureView<A> = cmd_buf
.trackers
.views
Expand Down Expand Up @@ -919,36 +929,38 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
});
}

colors.push(hal::ColorAttachment {
colors.push(Some(hal::ColorAttachment {
target: hal::Attachment {
view: &color_view.raw,
usage: hal::TextureUses::COLOR_TARGET,
},
resolve_target: hal_resolve_target,
ops: at.channel.hal_ops(),
clear_value: at.channel.clear_value,
});
}));
}

if sample_count != 1 && sample_count != 4 {
return Err(RenderPassErrorInner::InvalidSampleCount(sample_count));
}
let extent = extent.ok_or(RenderPassErrorInner::MissingAttachments)?;
let multiview = detected_multiview.expect("Multiview was not detected, no attachments");

let view_data = AttachmentData {
colors: color_attachments
.iter()
.map(|at| view_guard.get(at.view).unwrap())
.map(|at| at.as_ref().map(|at| view_guard.get(at.view).unwrap()))
.collect(),
resolves: color_attachments
.iter()
.filter_map(|at| at.resolve_target)
.map(|attachment| view_guard.get(attachment).unwrap())
.filter_map(|at| match *at {
Some(RenderPassColorAttachment {
resolve_target: Some(resolve),
..
}) => Some(view_guard.get(resolve).unwrap()),
_ => None,
})
.collect(),
depth_stencil: depth_stencil_attachment.map(|at| view_guard.get(at.view).unwrap()),
};
let extent = extent.ok_or(RenderPassErrorInner::MissingAttachments)?;

let multiview = detected_multiview.expect("Multiview was not detected, no attachments");
let context = RenderPassContext {
attachments: view_data.map(|view| view.desc.format),
sample_count,
Expand Down Expand Up @@ -1076,7 +1088,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
&self,
encoder_id: id::CommandEncoderId,
base: BasePassRef<RenderCommand>,
color_attachments: &[RenderPassColorAttachment],
color_attachments: &[Option<RenderPassColorAttachment>],
depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>,
) -> Result<(), RenderPassError> {
profiling::scope!("run_render_pass", "CommandEncoder");
Expand Down
Loading

0 comments on commit 61796b1

Please sign in to comment.