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

WebGPURenderer: Rename getArrayBuffer() to getArrayBufferAsync() #26393

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jul 8, 2023

Description

In order to standardize async functions name that need to return values must have the Async suffix.

@sunag sunag added this to the r155 milestone Jul 8, 2023
@LeviPesin
Copy link
Contributor

Why? loadAsync has such name only to differentiate with load. And no other async functions in Three.js AFAIR have this suffix, I think.

@sunag
Copy link
Collaborator Author

sunag commented Jul 8, 2023

Why? loadAsync has such name only to differentiate with load. And no other async functions in Three.js AFAIR have this suffix, I think.

I think it's more intuitive, in addition to the fact that in WebGPU we will have a lot more asynchronous calls than in WebGL, this will make it easier for us not to create possible conflicts in the future, and done to the same function but in sync by using another approach.

@LeviPesin
Copy link
Contributor

But maybe we can do this rename after -- if we will have this other approach?

@sunag
Copy link
Collaborator Author

sunag commented Jul 8, 2023

And no other async functions in Three.js AFAIR have this suffix, I think.

I forgot to quote this PR Renderer.readRenderTargetPixelsAsync() #26326, and what everything indicates we will have many others.

But maybe we can do this rename after -- if we will have this other approach?

I think we can save this additional effort in the future, both on our part and the users in changing their code because of three.js update.

@sunag sunag merged commit 7558e97 into mrdoob:dev Jul 9, 2023
@sunag sunag deleted the dev-async branch July 9, 2023 01:48
@LeviPesin
Copy link
Contributor

@mrdoob @Mugen87 What do you think?

@mrdoob
Copy link
Owner

mrdoob commented Jul 12, 2023

I agree with @sunag 👍

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.

3 participants