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

Move 'transform-feedback' example out of WIP, fix errors #1883

Merged
merged 4 commits into from
Jan 4, 2024

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Dec 15, 2023

Fixes the transform-feedback example. Some cleanup still needed. Like #1879, I'm relying on an extra buffer instead of reading/writing the same buffer, and I'm not sure how to avoid that yet. The 'swap' implementation is more verbose than in v8, I'll wait for other ideas on the API to fix that.

transform-feedback.mov

Related:

@donmccurdy donmccurdy marked this pull request as ready for review December 19, 2023 16:37
@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Dec 19, 2023

Marking as 'ready for review' just in a general aim to move more code out of WIP into a stable folder, but clearly there are some non-ideal things still in this PR:

  • requires separate read/write buffers which could be consolidated
  • buffer-swapping implementation is verbose
  • final Transform API for v9 is still TBD

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

Thanks. This used to be part of the tutorials I think, but since TransformFeedback is webgl specific better to keep it in another section, maybe API, maybe webgl

This also affects the transform feedback tutorial page in the docs folder.

If you are interested in adding to the to the website, build the website (cd website; yarn; yarn start) and make sure it shows up in the right place and the tutorial pages work. That could be a separate PR of course.

@donmccurdy
Copy link
Collaborator Author

I've assumed we intend the Transform class to be platform-independent, and (aside from one bit of API leakage) the example doesn't touch TransformFeedback directly. Maybe it'd make sense to keep this in example/api, but rename it something that isn't WebGL-related, like transform/ or buffer-transitions/?

@donmccurdy donmccurdy changed the title Draft: Move 'transform-feedback' example out of WIP, fix errors Move 'transform-feedback' example out of WIP, fix errors Dec 19, 2023
@ibgreen
Copy link
Collaborator

ibgreen commented Dec 19, 2023

I've assumed we intend the Transform class to be platform-independent

That would be great, but I felt that first we have to create a computeShader variant for WebGPU and see how that looks. Until then it is hard to say how a portable API would look, or if making Transform portable truly makes sense.

Transform class accept code snippets I think, which might be very different.

Also computeShaders can e.g. do random access on memory through storage buffers which could lead to very different programming patterns from per-row attribute -> attribute processing.

@donmccurdy
Copy link
Collaborator Author

Hmmm good points. So for v9.0 I'm looking at splitting BufferTransform / TextureTransform as two (immediately deprecated) classes we'd replace in v9.1. Given that they're temporary, and only nominally WebGL-agnostic... maybe we'll put this in a webgl folder but keep it out of the website for now?

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 19, 2023

as two (immediately deprecated) classes we'd replace in v9.1.

Well, we can always "un-deprecate" them in 9.1, should they grow on us.

It is just one of our mechanisms for signaling that we are not sure. We can also export with leading underscores etc.

@ibgreen
Copy link
Collaborator

ibgreen commented Dec 19, 2023

maybe we'll put this in a webgl folder but keep it out of the website for now?

That is reasonable.

@donmccurdy donmccurdy changed the base branch from master to donmccurdy/texture-transform January 2, 2024 19:03
@donmccurdy donmccurdy force-pushed the donmccurdy/transform-feedback-example branch from fef6ebe to 391574a Compare January 2, 2024 19:03
@donmccurdy
Copy link
Collaborator Author

donmccurdy commented Jan 2, 2024

@donmccurdy donmccurdy force-pushed the donmccurdy/transform-feedback-example branch from 391574a to 0f70db4 Compare January 2, 2024 19:44
Base automatically changed from donmccurdy/texture-transform to master January 4, 2024 14:40
@donmccurdy donmccurdy force-pushed the donmccurdy/transform-feedback-example branch from 0f70db4 to 45b8bd0 Compare January 4, 2024 18:53
@donmccurdy donmccurdy merged commit 0b1bfca into master Jan 4, 2024
3 checks passed
@donmccurdy donmccurdy deleted the donmccurdy/transform-feedback-example branch January 4, 2024 19:30
@donmccurdy donmccurdy added this to the 9.0.0 milestone Feb 26, 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
Development

Successfully merging this pull request may close these issues.

2 participants