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

Procedural geometry refactoring, createMesh and similar deprecation #6261

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Apr 18, 2024

Refactoring goals:

  • separate geometry functionality, from Mesh functionality. Current function would simply create a mesh, and if CPU processing / access to data was needed, those would have to be extracted from the mesh VBs.
  • simplify a way to add new geometry shapes while allowing three-shaking if not used

New API

  • Geometry - a base class to store geometry in CPU memory arrays
  • SphereGeometry, PlaneGeometry and similar - extended classes to fill arrays of the base class with appropriate vertex and index data
  • Mesh.fromGeometry - a static function to create a Mesh instance from the Geometry

Deprecated API

  • createMesh - replaced by Mesh.fromGeometry
  • createSphere, createPlane and similar - replaced by SphereGeometry and similar classes

All deprecate functions are still fully functional, and print a deprecated warning.

Upgrading guide

Before:

const cylinderMesh = pc.createCylinder(app.graphicsDevice, { capSegments: 200 });

Now:

const cylinderMesh = pc.Mesh.fromGeometry(app.graphicsDevice, new pc.CylinderGeometry({ capSegments: 200 }));

@mvaligursky mvaligursky self-assigned this Apr 18, 2024
@mvaligursky mvaligursky added feature area: graphics Graphics related issue labels Apr 18, 2024
@mvaligursky mvaligursky requested a review from a team April 18, 2024 11:01
@LeXXik
Copy link
Contributor

LeXXik commented Apr 18, 2024

Can this be used as input for the particle emitter? I remember there was an issue previously.

@mvaligursky
Copy link
Contributor Author

Can this be used as input for the particle emitter? I remember there was an issue previously.

I have not investigated, but I suspect the particle emitter requires a Mesh. So nothing would have changed regarding particles.

@kpal81xd
Copy link
Contributor

Nice look way cleaner now

@kpal81xd
Copy link
Contributor

@mvaligursky Whats the intension of keeping geometry utils separate? why not add those to the geometry base class?

@mvaligursky
Copy link
Contributor Author

@mvaligursky Whats the intension of keeping geometry utils separate? why not add those to the geometry base class?

Those are already public functions, and sometimes also called with arrays of positions / indices without having to ues Geometry class. So the base class just calls them as one of the customers of them.

@mvaligursky mvaligursky merged commit 15172d8 into main Apr 19, 2024
7 checks passed
@mvaligursky mvaligursky deleted the mv-geometry branch April 19, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants