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

WGPUProgrammableStageDescriptor is actually for compute only #409

Closed
Tracked by #299
kainino0x opened this issue Nov 12, 2024 · 6 comments · Fixed by #413 or #437
Closed
Tracked by #299

WGPUProgrammableStageDescriptor is actually for compute only #409

kainino0x opened this issue Nov 12, 2024 · 6 comments · Fixed by #413 or #437

Comments

@kainino0x
Copy link
Collaborator

The thing called WGPUProgrammableStageDescriptor is only used in WGPUComputePipelineDescriptor because we have independent definitions for the vertex/fragment equivalents, that have their own module/entryPoint/constants (WGPUVertexState and WGPUFragmentState).

  • Should it be flattened into WGPUComputePipelineDescriptor? (Future extensions would go there)
  • If not:
    • Should WGPUProgrammableStageDescriptor be renamed to be compute-specific?
    • Or should vertex/fragment also use WGPUProgrammableStageDescriptor? (This would make it slightly more uniform to add extensions that apply to all three; but we can already do that by allowing that extension to have multiple base structs.)
    • Should both the parent and child struct still be extensible?
@kainino0x kainino0x added the !discuss Needs discussion (at meeting or online) label Nov 12, 2024
@Kangz
Copy link
Collaborator

Kangz commented Nov 12, 2024

+1 for renaming to WGPUComputePipelineDescriptor. IMHO it's overkill to make the C struct hierarchy match the JS dictionary hierarchy because it in C you'd need to have the parent as members, which makes the API more annoying to use. But also in JS we can easily add intermediate dictionaries any time, which would make webgpu.h no longer match the WebGPU API.

@kainino0x
Copy link
Collaborator Author

+1 for renaming to WGPUComputePipelineDescriptor.

Do you mean flatten into WGPUComputePipelineDescriptor, or rename to something else like WGPUComputeState or WGPUComputeStage? (I think I'd prefer to flatten)

@Kangz
Copy link
Collaborator

Kangz commented Nov 12, 2024

Ah yeah flattening works as well, I'm not looking forward to the breaking change though.

@kainino0x
Copy link
Collaborator Author

I forgot it's not flattened in the JS API, so probably just renaming makes more sense to match (it's easier anyway)

@kainino0x
Copy link
Collaborator Author

Fixed tentatively, leaving !discuss to confirm.

@kainino0x
Copy link
Collaborator Author

Nov 25 meeting:

  • Renamed already to WGPUComputeStage. OK? (Note "Stage" vs "State" difference from WGPUVertexState/WGPUFragmentState)
  • CF: Think inconsistency is confusing. If adding more stages or whatever they would also be State.
  • Rename to WGPUComputeState

@kainino0x kainino0x reopened this Nov 26, 2024
@kainino0x kainino0x removed the !discuss Needs discussion (at meeting or online) label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants