Sitelet https://github.com/gpuweb/gpuweb/pull/102/files
Skip to content

Pass attachments state by value #102

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.

Sign up for GitHub

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

Merged
kvark merged 1 commit into from
Oct 31, 2018
Merged

Pass attachments state by value #102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 13 additions & 7 deletions design/sketch.webidl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,13 +402,20 @@ dictionary WebGPUShaderModuleDescriptor {
interface WebGPUShaderModule {
};

// Description of the framebuffer attachments
dictionary WebGPUAttachmentsStateDescriptor {
sequence<WebGPUTextureFormatEnum> formats;
// TODO other stuff like sample count etc.
// Description of a single attachment
dictionary WebGPUAttachment {

@litherum litherum Oct 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this eventually going to be the place where we add things like blending operations?

@kvark kvark Oct 29, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We might want to have separate types for color attachments and depth/stencil attachments before we add blending.

// Attachment data format
WebGPUTextureFormatEnum format;
// Number of MSAA samples
u32 samples;

@Kangz Kangz Oct 25, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is pending discussion of multisampling, but I think we also need to add "boolean isResolvedAtEndOfRenderPass" because VkRenderPass compatibility includes "VkSubpassDescription:: pResolveAttachments". I'll open an issue on Vulkan-Docs to ask if this can be relaxed for single-subpass render passes.

@kvark kvark Oct 25, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the point of this PR is that we can derive the need of those being resolved simply by looking at the sample counts

};

interface WebGPUAttachmentsState {
// Description of the framebuffer attachments
dictionary WebGPUAttachmentsState {

@litherum litherum Oct 29, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why make it a dictionary?

@kvark kvark Oct 29, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The point of this PR is to remove the attachment state as a handle, because there is no backend-specific data to associate with it. The render pass details is derived from the actual data passed to render pipeline creation and the beginning of a render pass.

// Array of color attachments
sequence<WebGPUAttachment> colorAttachments;
// Optional depth/stencil attachment
WebGPUAttachment? depthStencilAttachment;

@Kangz Kangz Oct 25, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not important for WebGPU, but this will translate to a nullable pointer to struct in Dawn, which our remoting layer doesn't have support for atm.

@kvark kvark Oct 25, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How else do you express a proper Option? :)

};

// Common stuff for ComputePipeline and RenderPipeline
Expand All @@ -431,7 +438,7 @@ dictionary WebGPUPipelineDescriptorBase {
sequence<WebGPUPipelineStageDescriptor> stages;
};

// ComputePipeline
// WebGPUComputePipeline
dictionary WebGPUComputePipelineDescriptor : WebGPUPipelineDescriptorBase {
};

Expand Down Expand Up @@ -642,7 +649,6 @@ interface WebGPUDevice {
WebGPUDepthStencilState createDepthStencilState(WebGPUDepthStencilStateDescriptor descriptor);
WebGPUInputState createInputState(WebGPUInputStateDescriptor descriptor);
WebGPUShaderModule createShaderModule(WebGPUShaderModuleDescriptor descriptor);
WebGPUAttachmentsState createAttachmentsState(WebGPUAttachmentsStateDescriptor descriptor);
WebGPUComputePipeline createComputePipeline(WebGPUComputePipelineDescriptor descriptor);
WebGPURenderPipeline createRenderPipeline(WebGPURenderPipelineDescriptor descriptor);

Expand Down