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

Add joinPrimitives() #707

Merged
merged 5 commits into from Jan 26, 2023
Merged

Add joinPrimitives() #707

merged 5 commits into from Jan 26, 2023

Conversation

donmccurdy
Copy link
Owner

@donmccurdy donmccurdy commented Oct 18, 2022

Given a list of compatible Mesh Primitives, returns new Primitive containing their vertex data. Compatibility requires that all Primitives share the same Materials, draw mode, and vertex attribute types. Primitives using morph targets cannot currently be joined.

Related:

To do:

  • Implementation
  • Unit tests

Example:

import { joinPrimitives } from '@gltf-transform/functions';

// Succeeds if Primitives are compatible, or throws an error.
const result = joinPrimitives(mesh.listPrimitives());

for (const prim of mesh.listPrimitives()) {
    prim.dispose();
}

mesh.addPrimitive(result);

@donmccurdy donmccurdy added feature New enhancement or request package:functions labels Oct 18, 2022
@donmccurdy donmccurdy added this to the v2.4 milestone Oct 18, 2022
@donmccurdy donmccurdy mentioned this pull request Dec 2, 2022
2 tasks
@drcmda
Copy link

drcmda commented Dec 21, 2022

in case it helps, these are the pruning steps in gltfjsx: https://github.com/pmndrs/gltfjsx/blob/master/src/utils/parser.js#L239-L310 pruning the graph results in smaller graphs but also improves performance since there are fewer nodes to parse and fewer matrix updates.

      /** Empty or no-property groups
       *    <group>
       *      <mesh geometry={nodes.foo} material={materials.bar} />
       *  Solution:
       *    <mesh geometry={nodes.foo} material={materials.bar} />
       */

      /** Double negative transforms
       *    <group rotation={[-Math.PI / 2, 0, 0]}>
       *      <group rotation={[Math.PI / 2, 0, 0]}>
       *        <mesh geometry={nodes.foo} material={materials.bar} />
       *  Solution:
       *    <mesh geometry={nodes.foo} material={materials.bar} />
       */
       
      /** Transform overlap      
       *    <group position={[10, 0, 0]} scale={2} rotation={[-Math.PI / 2, 0, 0]}>
       *      <mesh geometry={nodes.foo} material={materials.bar} />
       *  Solution:
       *    <mesh geometry={nodes.foo} material={materials.bar} position={[10, 0, 0]} scale={2} rotation={[-Math.PI / 2, 0, 0]} />
       */       
       
      /** Lack of content
       *    <group position={[10, 0, 0]} scale={2} rotation={[-Math.PI / 2, 0, 0]}>
       *      <group position={[10, 0, 0]} scale={2} rotation={[-Math.PI / 2, 0, 0]}>
       *        <group position={[10, 0, 0]} scale={2} rotation={[-Math.PI / 2, 0, 0]} />
       * Solution:
       *   (delete the sub graph)
       */

currently it is only a one-pass solution which is not enough. given:

export function Model(props) {
  const { nodes, materials } = useGLTF('/scene_test-transformed.glb')
  return (
    <group {...props} dispose={null}>
      <group>
        <group rotation={[-Math.PI / 2, 0, 0]}>
          <group>
            <group rotation={[Math.PI / 2, 0, 0]}>
              <group position={[0.16, 0.79, -1.97]} rotation={[-0.54, 0.93, -1.12]} scale={0.5}>
                <mesh geometry={nodes.Object_4.geometry} material={materials.Texture} />
              </group>
            </group>
          </group>
        </group>
      </group>
    </group>
  )
}

it will currently prune it as:

export function Model(props) {
  const { nodes, materials } = useGLTF('/scene_test-transformed.glb')
  return (
    <group {...props} dispose={null}>
      <group rotation={[-Math.PI / 2, 0, 0]}>
        <group rotation={[Math.PI / 2, 0, 0]}>
          <mesh geometry={nodes.Object_4.geometry} material={materials.Texture} position={[0.16, 0.79, -1.97]} rotation={[-0.54, 0.93, -1.12]} scale={0.5} />
        </group>
      </group>
    </group>
  )
}

which would be smaller if it had a backup pass:

export function Model(props) {
  const { nodes, materials } = useGLTF('/scene_test-transformed.glb')
  return (
    <group {...props} dispose={null}>
      <mesh geometry={nodes.Object_4.geometry} material={materials.Texture} position={[0.16, 0.79, -1.97]} rotation={[-0.54, 0.93, -1.12]} scale={0.5} />
    </group>
  )
}

i think graph pruning would make more sense as a gltf-transform step and i would love to remove it.

@drcmda
Copy link

drcmda commented Dec 21, 2022

scene_test.glb.zip

this a model for testing (the one above)

@donmccurdy donmccurdy modified the milestones: v2.4, v2.5 Dec 27, 2022
@donmccurdy donmccurdy modified the milestones: v2.5, v3.0 Jan 4, 2023
@donmccurdy donmccurdy marked this pull request as ready for review January 26, 2023 01:49
@donmccurdy
Copy link
Owner Author

donmccurdy commented Jan 26, 2023

@drcmda — the depth of the scene graph can be reduced with the new flatten() transform in the v3 prerelease:

import { flatten } from '@gltf-transform/functions';

await document.transform(flatten());
before after
Screenshot 2023-01-25 at 8 50 49 PM Screenshot 2023-01-25 at 8 50 54 PM

This PR for joinPrimitives(prims) is still open though, as I'm trying to go a bit further and support merging mesh primitives to reduce the draw calls after the scene graph has been flattened.

@donmccurdy donmccurdy merged commit 0e99c42 into main Jan 26, 2023
@donmccurdy donmccurdy deleted the feat/join-primitives branch January 26, 2023 19:57
@drcmda
Copy link

drcmda commented Jan 26, 2023

@donmccurdy wow, nice work! if you have a minute to spare, could you go a little into how it's doing it and to which extend?

ps. this will be live in alpha 05?

@donmccurdy
Copy link
Owner Author

@drcmda sure thing, here's the description for flatten()

Flattens the scene graph, leaving Nodes with Meshes, Cameras, and other
attachments as direct children of the Scene. Skeletons and their
descendants are left in their original Node structure.

Animation targeting a Node or its parents will prevent that Node from being
moved.

It computes the world matrix for each meaningful node, then moves it to the scene root without moving the node around in world space.

I'm open to leaving some escape hatches to preserve more structure where needed, perhaps based on node names, but for now this is aiming for the minimum possible nesting without breaking animation, which will be helpful once merging meshes is supported.

Should land in alpha 05, yes. 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New enhancement or request package:functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants