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

GltfLoader issues #2277

Open
capdevon opened this issue Jun 1, 2024 · 8 comments
Open

GltfLoader issues #2277

capdevon opened this issue Jun 1, 2024 · 8 comments

Comments

@capdevon
Copy link
Contributor

capdevon commented Jun 1, 2024

The recent changes to the GLTF loader have introduced several problems:

  1. Duplicate Controllers: The new loader creates duplicate AnimComposer and SkinningControl controllers for each geometry in the model. (see here)
  2. Nested Geometry: Each geometry is now encapsulated within two parent nodes, which can complicate scene hierarchy management.
  3. Backward Compatibility: These changes may cause compatibility issues with existing code that relies on the previous behavior of the GLTF loader. (see here)

Other issues reported by adi.barda: (see here)

  • Blured colors - maybe normal map is missing?
  • Sometime missing animations - maybe related to 1
@stephengold
Copy link
Member

Please provide detailed instructions for reproducing this issue.
Has anyone identified which recent changes caused this issue?

@capdevon
Copy link
Contributor Author

capdevon commented Jun 1, 2024

Maybe this: #2103

@stephengold
Copy link
Member

Would reverting 2103 solve this issue?

@capdevon
Copy link
Contributor Author

capdevon commented Jun 1, 2024

Would reverting 2103 solve this issue?

Probably yes.

Please provide detailed instructions for reproducing this issue.

Here is the 3D Model for the test file

jME-3.7.0
image

jME-3.6.1-stable
image

@stephengold
Copy link
Member

Using a simple test app, I showed that the issue of YBot loading with multiple AnimComposers pre-dates PR 2103. I'll do a bisection search to find where the issue was introduced.

Here is the test app:

import com.jme3.anim.AnimComposer;
import com.jme3.app.SimpleApplication;
import com.jme3.scene.Node;
import com.jme3.scene.Spatial;
import com.jme3.scene.control.Control;
import com.jme3.scene.plugins.gltf.GltfModelKey;
import java.util.List;

public class TestYBot extends SimpleApplication {

    public static void main(String[] args) {
        TestYBot app = new TestYBot();
        app.start();
    }

    @Override
    public void simpleInitApp() {
        GltfModelKey modelKey = new GltfModelKey("YBot.gltf");
        Spatial modelRoot = assetManager.loadModel(modelKey);
        int numComposers = countControls(modelRoot, AnimComposer.class);
        System.out.println("numComposers = " + numComposers);
        stop();
    }

    /**
     * Count all controls of the specified type in the specified subtree of a
     * scene graph. Note: recursive!
     *
     * @param <T> subclass of Control
     * @param subtree the subtree to analyze (may be null, unaffected)
     * @param controlType the subclass of Control to search for
     * @return the count (&ge;0)
     */
    private static <T extends Control> int countControls(
            Spatial subtree, Class<T> controlType) {
        int result = 0;

        if (subtree != null) {
            int numControls = subtree.getNumControls();
            for (int controlI = 0; controlI < numControls; ++controlI) {
                Control control = subtree.getControl(controlI);
                if (controlType.isAssignableFrom(control.getClass())) {
                    ++result;
                }
            }
        }

        if (subtree instanceof Node) {
            Node node = (Node) subtree;
            List<Spatial> children = node.getChildren();
            for (Spatial child : children) {
                result += countControls(child, controlType);
            }
        }

        return result;
    }
}

@stephengold
Copy link
Member

The issue was introduced by PR #2060.

With "master" branch at f915e56, the test app prints numComposers = 1.
With "master" branch at 4c87531, the test app prints numComposers = 2.

@riccardobl
Copy link
Member

  1. Nested Geometry: Each geometry is now encapsulated within two parent nodes, which can complicate scene hierarchy management.
  2. Backward Compatibility: These changes may cause compatibility issues with existing code that relies on the previous behavior of the GLTF loader. (see here)

As pointed out in the pr #2103 , this is to solve several issue and inconsistencies with the scenegraph of imported models , it is not backward compatible, but the previous behavior was bugged and source of issues

@stephengold
Copy link
Member

The current behavior, with multiple anim composers per model, is clearly bugged. And that change appears to be unrelated to PR 2103.

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

No branches or pull requests

3 participants