Skip to content

Comments

Throw exception when unsupported extension is required#2628

Open
javagl wants to merge 1 commit intojMonkeyEngine:masterfrom
javagl:gltf-extension-required-check
Open

Throw exception when unsupported extension is required#2628
javagl wants to merge 1 commit intojMonkeyEngine:masterfrom
javagl:gltf-extension-required-check

Conversation

@javagl
Copy link
Contributor

@javagl javagl commented Feb 22, 2026

Fixes #2617

Current state

Until now, the glTF loader only printed a SEVERE error message when an unsupported extension was declared in the extensionsRequired of a glTF asset. As a result, it could happen that ...

  • the loader bailed out later when trying to handle such an extension object
  • worse: it could happen that the asset was in an ~"invalid state", causing errors while rendering

In both cases, the error could show up as a arbitrary exception during loading or rendering. It could be a NullPointerException, IndedOutOfBoundsException, or whatever - with no clear indication for what caused this exception.

New state

This PR changes the behavior as follows:

  • When an unsupported extension is declared in extensionsRequired, then the default behavior is that the loader immediately bails out with an AssetLoadException, with an appropriate error message to indicate the reason for the error
  • There is a GltfModelKey#setStrict(boolean) function. When explicitly setting gltfModelKey.setStrict(false), then any unsupported extension will still (only) cause a SEVERE error message (and likely, but not necessarily, cause an exception later...)

Testing

I'm not entirely sure about the best test procedure here. I tested it locally, in two ways:

I tested it by loading the SheenWoodLeatherSofa that was mentioned in #2385 . It now causes the exception

....
Caused by: com.jme3.asset.AssetLoadException: Extension EXT_texture_webp is required for this file.
	at com.jme3.scene.plugins.gltf.CustomContentManager.init(CustomContentManager.java:137)
	at com.jme3.scene.plugins.gltf.GltfLoader.loadFromStream(GltfLoader.java:200)

Instead of that random NPE that it caused until now.

I also created a dedicated tests for the extension declaration handling. For this, I created the following asset, which is just the (embedded) version of a single triangle, with extension declarations for testing:

{
  "extensionsUsed": [
    "KHR_texture_transform",
    "EXAMPLE_unsupported_extension"
  ],
  "extensionsRequired": [
    "KHR_texture_transform",
    "EXAMPLE_unsupported_extension"
  ],
  "scene" : 0,
  "scenes" : [
    {
      "nodes" : [ 0 ]
    }
  ],
  
  "nodes" : [
    {
      "mesh" : 0
    }
  ],
  
  "meshes" : [
    {
      "primitives" : [ {
        "attributes" : {
          "POSITION" : 1
        },
        "indices" : 0
      } ]
    }
  ],

  "buffers" : [
    {
      "uri" : "data:application/octet-stream;base64,AAABAAIAAAAAAAAAAAAAAAAAAAAAAIA/AAAAAAAAAAAAAAAAAACAPwAAAAA=",
      "byteLength" : 44
    }
  ],
  "bufferViews" : [
    {
      "buffer" : 0,
      "byteOffset" : 0,
      "byteLength" : 6,
      "target" : 34963
    },
    {
      "buffer" : 0,
      "byteOffset" : 8,
      "byteLength" : 36,
      "target" : 34962
    }
  ],
  "accessors" : [
    {
      "bufferView" : 0,
      "byteOffset" : 0,
      "componentType" : 5123,
      "count" : 3,
      "type" : "SCALAR",
      "max" : [ 2 ],
      "min" : [ 0 ]
    },
    {
      "bufferView" : 1,
      "byteOffset" : 0,
      "componentType" : 5126,
      "count" : 3,
      "type" : "VEC3",
      "max" : [ 1.0, 1.0, 0.0 ],
      "min" : [ 0.0, 0.0, 0.0 ]
    }
  ],
  
  "asset" : {
    "version" : "2.0"
  }
}`

The default behavior here is also the expected one:

Caused by: com.jme3.asset.AssetLoadException: Extension EXAMPLE_unsupported_extension is required for this file.
	at com.jme3.scene.plugins.gltf.CustomContentManager.init(CustomContentManager.java:137)
	at com.jme3.scene.plugins.gltf.GltfLoader.loadFromStream(GltfLoader.java:200)
	... 12 more

When loading this with gltfModelKey.setStrict(false), then the result is only a log message:
SCHWERWIEGEND: Extension EXAMPLE_unsupported_extension is required for this file. The behavior of the loader is unspecified.


The main point here is: I'm not sure how to "consolidate" these tests. That TriangleUnsupportedExtensionRequired.gltf file is currently not checked in (but it is small, so it could probably be added...).

And I did the tests with manual tweaks to the TestGltfLoading class. I can always check in "my local, current state" of this class. But I wondered whether there is a way to streamline the different tests that should be done, without someone having to twiddle with // commented-out lines.

However, for the time being, here is my current local state of the TestGltfLoading, in case someone wants to try it out locally:

/*
 * Copyright (c) 2009-2022 jMonkeyEngine
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are
 * met:
 *
 * * Redistributions of source code must retain the above copyright
 *   notice, this list of conditions and the following disclaimer.
 *
 * * Redistributions in binary form must reproduce the above copyright
 *   notice, this list of conditions and the following disclaimer in the
 *   documentation and/or other materials provided with the distribution.
 *
 * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
 *   may be used to endorse or promote products derived from this software
 *   without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
 * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
 * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
 * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
 * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
 * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
 * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
 * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
package jme3test.model;

import com.jme3.anim.AnimComposer;
import com.jme3.app.*;
import com.jme3.asset.plugins.FileLocator;
import com.jme3.asset.plugins.UrlLocator;
import com.jme3.bounding.BoundingBox;
import com.jme3.input.KeyInput;
import com.jme3.input.controls.ActionListener;
import com.jme3.input.controls.KeyTrigger;
import com.jme3.math.*;
import com.jme3.renderer.Limits;
import com.jme3.scene.*;
import com.jme3.scene.debug.custom.ArmatureDebugAppState;
import com.jme3.scene.plugins.gltf.GltfModelKey;
import jme3test.model.anim.EraseTimer;

import java.io.File;
import java.util.*;

public class TestGltfLoading extends SimpleApplication {

    private final Node autoRotate = new Node("autoRotate");
    private final List<Spatial> assets = new ArrayList<>();
    private Node probeNode;
    private float time = 0;
    private int assetIndex = 0;
    private boolean useAutoRotate = false;
    private final static String indentString = "\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t";
    private final int duration = 1;
    private boolean playAnim = true;
    private ChaseCameraAppState chaseCam;

    private final Queue<String> anims = new LinkedList<>();
    private AnimComposer composer;

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

    /**
     * WARNING This test case will try to load models from $HOME/glTF-Sample-Models, if the models is not
     * found there, it will automatically try to load it from the repository
     * https://github.com/KhronosGroup/glTF-Sample-Models .
     * 
     * Depending on the your connection speed and github rate limiting, this can be quite slow.
     */
    @Override
    public void simpleInitApp() {

        ArmatureDebugAppState armatureDebugappState = new ArmatureDebugAppState();
        getStateManager().attach(armatureDebugappState);
        setTimer(new EraseTimer());

        String folder = System.getProperty("user.home") + "/glTF-Sample-Models";
        if (new File(folder).exists()) {
            assetManager.registerLocator(folder, FileLocator.class);
        }
        assetManager.registerLocator(
                "https://raw.githubusercontent.com/KhronosGroup/glTF-Sample-Assets/refs/heads/main/",
                UrlLocator.class);

        cam.setFrustumPerspective(45f, (float) cam.getWidth() / cam.getHeight(), 0.1f, 100f);
        renderer.setDefaultAnisotropicFilter(Math.min(renderer.getLimits().get(Limits.TextureAnisotropy), 8));
        setPauseOnLostFocus(false);

        flyCam.setMoveSpeed(5);
        flyCam.setDragToRotate(true);
        flyCam.setEnabled(false);
        viewPort.setBackgroundColor(new ColorRGBA().setAsSrgb(0.2f, 0.2f, 0.2f, 1.0f));
        rootNode.attachChild(autoRotate);
        probeNode = (Node) assetManager.loadModel("Scenes/defaultProbe.j3o");
        autoRotate.attachChild(probeNode);

        chaseCam = new ChaseCameraAppState();
        getStateManager().attach(chaseCam);

        // loadModelSample("Duck", "gltf");
        // loadModelSample("Duck", "glb");
        // loadModelSample("ABeautifulGame", "gltf");
        // loadModelSample("Avocado", "glb");
        // loadModelSample("Avocado", "gltf");
        // loadModelSample("CesiumMilkTruck", "glb");
        // loadModelSample("VirtualCity", "glb");
        // loadModelSample("BrainStem", "glb");
        // loadModelSample("Lantern", "glb");
        // loadModelSample("RiggedFigure", "glb");
        // loadModelSample("SciFiHelmet", "gltf");
        //loadModelSample("DamagedHelmet", "gltf");
        // loadModelSample("AnimatedCube", "gltf");
        // loadModelSample("AntiqueCamera", "glb");
        // loadModelSample("AnimatedMorphCube", "glb");

        // DRACO SAMPLES

        // loadModelSample("Avocado", "draco");
        // loadModelSample("BarramundiFish", "draco");
        // loadModelSample("BoomBox", "draco");
        // loadModelSample("CesiumMilkTruck", "draco");
        // loadModelSample("Corset", "draco");
        // loadModelSample("Lantern", "draco");
        // loadModelSample("MorphPrimitivesTest", "draco");
        // loadModelSample("WaterBottle", "draco");
        
        // Draco skinning samples
        //loadModelSample("BrainStem", "draco");
        //loadModelSample("BrainStem", "glb");

        //loadModelSample("CesiumMan", "draco");
        //loadModelSample("CesiumMan", "glb");

        // loadModelSample("RiggedFigure", "draco");
        //loadModelSample("RiggedFigure", "glb");

        //loadModelSample("RiggedSimple", "draco");
        //loadModelSample("RiggedSimple", "glb");

        // Test for normalized texture coordinates in draco
        //loadModelFromPath("Models/gltf/unitSquare11x11_unsignedShortTexCoords-draco.glb");
        
        // Test for unsupported extension handling
        boolean strict = false;
        loadModelFromPath("Models/gltf/TriangleUnsupportedExtensionRequired.gltf", strict);
        
        //loadModelSample("SheenWoodLeatherSofa", "glb");
        
        //loadModelFromPath("Models/gltf/TriangleWithBin/TriangleWithBin.gltf");
        //loadModelFromPath("Models/gltf/TriangleWithGlbin/TriangleWithGlbin.gltf");
        //loadModelFromPath("Models/gltf/TriangleWithNoExtension/TriangleWithNoExtension.gltf");

        // Uses EXT_texture_webp - not supported yet 
        //loadModelSample("SunglassesKhronos", "draco");
        
        // Probably invalid model
        // See https://github.com/KhronosGroup/glTF-Sample-Assets/issues/264
        // loadModelSample("VirtualCity", "draco");
        
        probeNode.attachChild(assets.get(0));

        inputManager.addMapping("autorotate", new KeyTrigger(KeyInput.KEY_SPACE));
        inputManager.addListener(new ActionListener() {
            @Override
            public void onAction(String name, boolean isPressed, float tpf) {
                if (isPressed) {
                    useAutoRotate = !useAutoRotate;
                }
            }
        }, "autorotate");

        inputManager.addMapping("toggleAnim", new KeyTrigger(KeyInput.KEY_RETURN));

        inputManager.addListener(new ActionListener() {
            @Override
            public void onAction(String name, boolean isPressed, float tpf) {
                if (isPressed) {
                    playAnim = !playAnim;
                    if (playAnim) {
                        playFirstAnim(rootNode);
                    } else {
                        stopAnim(rootNode);
                    }
                }
            }
        }, "toggleAnim");
        inputManager.addMapping("nextAnim", new KeyTrigger(KeyInput.KEY_RIGHT));
        inputManager.addListener(new ActionListener() {
            @Override
            public void onAction(String name, boolean isPressed, float tpf) {
                if (isPressed && composer != null) {
                    String anim = anims.poll();
                    anims.add(anim);
                    composer.setCurrentAction(anim);
                }
            }
        }, "nextAnim");

        dumpScene(rootNode, 0);

        // stateManager.attach(new DetailedProfilerState());
    }

    private void loadModelSample(String name, String type) {
        String path = "Models/" + name;
        String ext = "gltf";
        switch (type) {
            case "draco":
                path += "/glTF-Draco/";
                ext = "gltf";
                break;
            case "glb":
                path += "/glTF-Binary/";
                ext = "glb";
                break;
            default:
                path += "/glTF/";
                ext = "gltf";
                break;
        }
        path += name + "." + ext;
        loadModelFromPath(path, true);
    }

    private void loadModelFromPath(String path, boolean strict) {

        Spatial s = loadModel(path, new Vector3f(0, 0, 0), 1f, strict);

        BoundingBox bbox = (BoundingBox) s.getWorldBound();
        float maxExtent = Math.max(bbox.getXExtent(), Math.max(bbox.getYExtent(), bbox.getZExtent()));
        if (maxExtent < 10f) {
            s.scale(10f / maxExtent);
            maxExtent = 10f;
        }
        float distance = 50f;

        chaseCam.setTarget(s);
        chaseCam.setInvertHorizontalAxis(true);
        chaseCam.setInvertVerticalAxis(true);
        chaseCam.setZoomSpeed(1.5f);
        chaseCam.setMinVerticalRotation(-FastMath.HALF_PI);
        chaseCam.setRotationSpeed(3);
        chaseCam.setDefaultDistance(distance);
        chaseCam.setMaxDistance(distance * 10);
        chaseCam.setDefaultVerticalRotation(0.3f);

    }

    private Spatial loadModel(String path, Vector3f offset, float scale, boolean strict) {
        return loadModel(path, offset, new Vector3f(scale, scale, scale), strict);
    }

    private Spatial loadModel(String path, Vector3f offset, Vector3f scale, boolean strict) {
        System.out.println("Loading model: " + path);
        GltfModelKey k = new GltfModelKey(path);
        k.setStrict(strict);
        // k.setKeepSkeletonPose(true);
        long t = System.currentTimeMillis();
        Spatial s = assetManager.loadModel(k);
        System.out.println("Load time : " + (System.currentTimeMillis() - t) + " ms");

        s.scale(scale.x, scale.y, scale.z);
        s.move(offset);
        assets.add(s);
        if (playAnim) {
            playFirstAnim(s);
        }

        return s;
    }

    private void playFirstAnim(Spatial s) {

        AnimComposer control = s.getControl(AnimComposer.class);
        if (control != null) {
            anims.clear();
            for (String name : control.getAnimClipsNames()) {
                anims.add(name);
            }
            if (anims.isEmpty()) {
                return;
            }
            String anim = anims.poll();
            anims.add(anim);
            control.setCurrentAction(anim);
            composer = control;
        }
        if (s instanceof Node) {
            Node n = (Node) s;
            for (Spatial spatial : n.getChildren()) {
                playFirstAnim(spatial);
            }
        }
    }

    private void stopAnim(Spatial s) {

        AnimComposer control = s.getControl(AnimComposer.class);
        if (control != null) {
            control.reset();
        }
        if (s instanceof Node) {
            Node n = (Node) s;
            for (Spatial spatial : n.getChildren()) {
                stopAnim(spatial);
            }
        }
    }

    @Override
    public void simpleUpdate(float tpf) {
        if (!useAutoRotate) {
            return;
        }
        time += tpf;
        // autoRotate.rotate(0, tpf * 0.5f, 0);
        if (time > duration) {
            // morphIndex++;
            // setMorphTarget(morphIndex);
            assets.get(assetIndex).removeFromParent();
            assetIndex = (assetIndex + 1) % assets.size();
            // if (assetIndex == 0) {
            // duration = 10;
            // }
            probeNode.attachChild(assets.get(assetIndex));
            time = 0;
        }
    }

    private void dumpScene(Spatial s, int indent) {
        System.err.println(indentString.substring(0, indent) + s.getName() + " ("
                + s.getClass().getSimpleName() + ") / " + s.getLocalTransform().getTranslation().toString()
                + ", " + s.getLocalTransform().getRotation().toString() + ", "
                + s.getLocalTransform().getScale().toString());
        if (s instanceof Node) {
            Node n = (Node) s;
            for (Spatial spatial : n.getChildren()) {
                dumpScene(spatial, indent + 1);
            }
        }
    }
}

* encounters an asset that contains an extension in its <code>extensionsRequired</code> declaration that
* is not supported.
*/
private boolean strictExtensionCheck;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private boolean strictExtensionCheck;
private boolean strictExtensionCheck = true;

better setting it here imo

@riccardobl
Copy link
Member

Thanks!

The main point here is: I'm not sure how to "consolidate" these tests. That TriangleUnsupportedExtensionRequired.gltf file is currently not checked in (but it is small, so it could probably be added...).

I think there is no problem in adding this to an unit test here .

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.

Handling of glTF extensions (used or required)

2 participants