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

InputNode: Added .setPrecision() #25561

Merged
merged 2 commits into from
Feb 24, 2023
Merged

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Feb 24, 2023

Related issue: #20297

Description

const nodePI = uniform( Math.PI );

// set high precision
nodePI.setPresicion( 'high' );

@sunag sunag added this to the r151 milestone Feb 24, 2023
@LeviPesin
Copy link
Contributor

Maybe we should move it to UniformNode? Because I don't think precisions are useful for ConstNodes.

@sunag sunag added the Nodes label Feb 24, 2023
@sunag sunag merged commit 6a9e961 into mrdoob:dev Feb 24, 2023
@sunag sunag deleted the nodes-input-precision branch February 24, 2023 20:11
@LeviPesin
Copy link
Contributor

By the way, shouldn't we also add some kind of precision-to-type mapping to WebGPU builder?

@sunag
Copy link
Collaborator Author

sunag commented Mar 12, 2023

@LeviPesin Certainly, not sure if WebGPU accepts precision who are not 16 or 32 bits. Maybe high and medium to 32 bits and low to 16bit.
I think we can improve this precision integration using nodes for this, something like:

const nodePI = uniform( Math.PI ).highPrecision();
// or
const nodePI = lowPrecision( uniform( Math.PI ) );

@sunag
Copy link
Collaborator Author

sunag commented Mar 12, 2023

I think that precision can be added contextually.

@LeviPesin
Copy link
Contributor

Maybe high and medium to 32 bits and low to 16bit.

Sounds good to me!

I think we can improve this precision integration using nodes for this, something like:

I think .setPrecision is already fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants