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

fix(engine) Workaround for deck.gl attribute access #1778

Merged
merged 6 commits into from
Aug 12, 2023

Conversation

ibgreen
Copy link
Collaborator

@ibgreen ibgreen commented Aug 11, 2023

For #

Background

Change List

Copy link
Collaborator

@Pessimistress Pessimistress left a comment

Choose a reason for hiding this comment

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

Should we just update how deck passes attributes to luma?

@ibgreen
Copy link
Collaborator Author

ibgreen commented Aug 11, 2023

Should we just update how deck passes attributes to luma?

Yes.

We should also decide how to handle constant attributes. The new Model class doesn't yet expose that API since WebGPU doesn't. I prefer the simplest possible API, meaning each attribute is either a Buffer or a typed array constant.

For WebGPU I see two choices, either we have to allocate and fill big buffers under the hood, or we could dynamically rewrite the shaders to use a uniform in those cases.

Replacing attributes with uniforms is a bit of a feature, and also complicated by the fact that uniforms need be provided through uniform buffers, so I suppose that could come later and we start with under the hood buffer generation...

@ibgreen
Copy link
Collaborator Author

ibgreen commented Aug 12, 2023

x

@ibgreen ibgreen closed this Aug 12, 2023
@ibgreen ibgreen reopened this Aug 12, 2023
@ibgreen ibgreen merged commit 3af6827 into master Aug 12, 2023
2 checks passed
@ibgreen ibgreen deleted the ib/model-attributes branch August 12, 2023 01:42
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