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

Relax render pass color_attachments validation #2778

Merged
merged 6 commits into from
Jun 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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