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

MeshSurfaceSampler: Add uv support. #26207

Merged
merged 2 commits into from
Jun 8, 2023
Merged

MeshSurfaceSampler: Add uv support. #26207

merged 2 commits into from
Jun 8, 2023

Conversation

makc
Copy link
Contributor

@makc makc commented Jun 7, 2023

this is one of two changes I had to do to MeshSurfaceSampler in order to use it for the grass effect - let's see if it makes sense to you, and then I will maybe come back with the other one

@Mugen87 Mugen87 added this to the r154 milestone Jun 7, 2023
@Mugen87 Mugen87 changed the title add uv support to MeshSurfaceSampler.js MeshSurfaceSampler: Add uv support. Jun 7, 2023
@gkjohnson
Copy link
Collaborator

At some point this function should really just return a barycoord value and face index so users can interpolate between whatever buffer attributes they want to. Adding a new argument for every possible buffer attribute doesn't seem sustainable 😅

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 7, 2023

I'm afraid not all users know how to do the interpolation correctly. Maybe better when MeshSurfaceSampler takes care about this.

@LeviPesin
Copy link
Contributor

Maybe we can add another method that does just that (i.e. returns barycoords and index)? So we can have both.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Jun 7, 2023

Adding that method (sampleBarycentric?) would be ideal I think, though I'm also fine with this PR as a starting point.

EDIT: Related –

@makc
Copy link
Contributor Author

makc commented Jun 7, 2023

Oh I did not see 24629. Maybe I could borrow the docs thing from there 🤔

@makc
Copy link
Contributor Author

makc commented Jun 7, 2023

that's weird, I did add the commit by @IanSweeneyAC but it does not appear in this PR... edit: so I was hit by this

@makc
Copy link
Contributor Author

makc commented Jun 7, 2023

and now there is failed webxr_vr_sandbox screenshot. not my fault, lol

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 7, 2023

Don't worry about this example. It frequently fails in other PRs, too. Maybe regenerating the screenshots would help.

@mrdoob mrdoob merged commit 63710e7 into mrdoob:dev Jun 8, 2023
@LeviPesin
Copy link
Contributor

Maybe we should add that example to the exceptions?

(The number of exceptions starts to be really large... I should look into it and see what can be fixed (e.g. setIntervals), also need to remove BrowserFetcher for Puppeteer 20)

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.

7 participants