javagl / JglTF

Java libraries related to glTF
MIT License
209 stars 61 forks source link

It seems that the binary format output by jgltf-browser 2.0 is incorrect #21

Closed cx20 closed 2 months ago

cx20 commented 6 years ago

I tried saving glTF file in binary format using jgltf-browser 2.0. The following is the procedure I tried.

  1. [Samples] - [Khronos Sample, v2.0] - [Duck] - [glTF (https://raw.githubusercontent.com/KhronosGroup/glTF-Sample-Models/master/2.0/Duck/glTF/Duck.gltf)]
  2. [File] - [Save as binary...] - [Duck.glb]
  3. [File] - [Open file...] - [Duck.glb]

The following error is displayed.

Loading error: java.io.IOException: Could not load glTF asset
java.util.concurrent.ExecutionException: java.io.IOException: Could not load glTF asset
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:192)
    at javax.swing.SwingWorker.get(SwingWorker.java:602)
    at de.javagl.swing.tasks.SwingTask.get(SwingTask.java:435)
    at de.javagl.jgltf.browser.GltfLoadingWorker.done(GltfLoadingWorker.java:170)
    at de.javagl.swing.tasks.SwingTask.callDone(SwingTask.java:473)
    at de.javagl.swing.tasks.SwingTask.access$200(SwingTask.java:74)
    at de.javagl.swing.tasks.SwingTask$SwingTaskWorker.done(SwingTask.java:108)
    at javax.swing.SwingWorker$5.run(SwingWorker.java:737)
    at javax.swing.SwingWorker$DoSubmitAccumulativeRunnable.run(SwingWorker.java:832)
    at sun.swing.AccumulativeRunnable.run(AccumulativeRunnable.java:112)
    at javax.swing.SwingWorker$DoSubmitAccumulativeRunnable.actionPerformed(SwingWorker.java:842)
    at javax.swing.Timer.fireActionPerformed(Timer.java:313)
    at javax.swing.Timer$DoPostEvent.run(Timer.java:245)
    at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:756)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:726)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:109)
    at java.awt.WaitDispatchSupport$2.run(WaitDispatchSupport.java:184)
    at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:229)
    at java.awt.WaitDispatchSupport$4.run(WaitDispatchSupport.java:227)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.awt.WaitDispatchSupport.enter(WaitDispatchSupport.java:227)
    at java.awt.Dialog.show(Dialog.java:1084)
    at java.awt.Component.show(Component.java:1671)
    at java.awt.Component.setVisible(Component.java:1623)
    at java.awt.Window.setVisible(Window.java:1014)
    at java.awt.Dialog.setVisible(Dialog.java:1005)
    at de.javagl.swing.tasks.DefaultSwingTaskView.show(DefaultSwingTaskView.java:70)
    at de.javagl.swing.tasks.SwingTaskExecutor.doExecute(SwingTaskExecutor.java:360)
    at de.javagl.swing.tasks.SwingTaskExecutor.execute(SwingTaskExecutor.java:318)
    at de.javagl.jgltf.browser.GltfLoadingWorker.load(GltfLoadingWorker.java:125)
    at de.javagl.jgltf.browser.GltfBrowserApplication.openUriInBackground(GltfBrowserApplication.java:559)
    at de.javagl.jgltf.browser.GltfBrowserApplication.openFile(GltfBrowserApplication.java:500)
    at de.javagl.jgltf.browser.GltfBrowserApplication.access$000(GltfBrowserApplication.java:76)
    at de.javagl.jgltf.browser.GltfBrowserApplication$1.actionPerformed(GltfBrowserApplication.java:106)
    at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
    at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2348)
    at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
    at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:259)
    at javax.swing.AbstractButton.doClick(AbstractButton.java:376)
    at javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:833)
    at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(BasicMenuItemUI.java:877)
    at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:289)
    at java.awt.Component.processMouseEvent(Component.java:6533)
    at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
    at java.awt.Component.processEvent(Component.java:6298)
    at java.awt.Container.processEvent(Container.java:2236)
    at java.awt.Component.dispatchEventImpl(Component.java:4889)
    at java.awt.Container.dispatchEventImpl(Container.java:2294)
    at java.awt.Component.dispatchEvent(Component.java:4711)
    at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4888)
    at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4525)
    at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4466)
    at java.awt.Container.dispatchEventImpl(Container.java:2280)
    at java.awt.Window.dispatchEventImpl(Window.java:2746)
    at java.awt.Component.dispatchEvent(Component.java:4711)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
    at java.awt.EventQueue.access$500(EventQueue.java:97)
    at java.awt.EventQueue$3.run(EventQueue.java:709)
    at java.awt.EventQueue$3.run(EventQueue.java:703)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
    at java.awt.EventQueue$4.run(EventQueue.java:731)
    at java.awt.EventQueue$4.run(EventQueue.java:729)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
Caused by: java.io.IOException: Could not load glTF asset
    at de.javagl.jgltf.browser.GltfAssetReaderThreaded.readGltfAsset(GltfAssetReaderThreaded.java:288)
    at de.javagl.jgltf.browser.GltfLoadingWorker.doInBackground(GltfLoadingWorker.java:162)
    at de.javagl.jgltf.browser.GltfLoadingWorker.doInBackground(GltfLoadingWorker.java:59)
    at de.javagl.swing.tasks.SwingTask.doTaskInBackground(SwingTask.java:335)
    at de.javagl.swing.tasks.SwingTask.access$000(SwingTask.java:74)
    at de.javagl.swing.tasks.SwingTask$SwingTaskWorker.doInBackground(SwingTask.java:84)
    at javax.swing.SwingWorker$1.call(SwingWorker.java:295)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at javax.swing.SwingWorker.run(SwingWorker.java:334)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
javagl commented 6 years ago

Thanks for this hint.

Part of this is a stupid copy+paste error ...

But even if the basic problem is fixed, there is still an issue with the resulting GLB files. Some of the offsets are .... well, off (similar to the recent issues in the COLLADA2GLTF-1.0-writer...). Additionally, the elements still refer to external files, e.g. the texture image, although this is inlined). I'll have to think about how this should be solved in general. Theoretically, there could be GLBs that refer to additional, external files, so I'll have to find a way to distinguish between these cases.

javagl commented 6 years ago

The bug was fixed in https://github.com/javagl/JglTF/commit/a7f214b11347371c79088572ecd9c31bad523e8a

The main problem (besides the copy+paste error) was that the chunkLength was set to be the length of the whole chunk (inculding the chunkLength and chunkType slots), but this is wrong. While the length field in the header refers to the whole file, the chunkLength is not the length of the whole chunk, but only the length of the chunk data.

(Actually, the IO is already tested with a unit test, but the GLB that the test output was compared against was already wrong - this is also fixed now)

Thanks again for bringing this up.

cx20 commented 6 years ago

@javagl Thank you for fixing.

I built the latest source and confirmed that Box.gltf can be saved as Box.glb by jgltf-browser. I confirmed that Box.glb can be displayed at https://gltf-viewer.donmccurdy.com/ and https://sandbox.babylonjs.com/ However, after converting Duck.gltf to Duck.glb, I tried to display it on the above site, but I got an error.

I do not know if it is related, but when trying to save Avocado.gltf as Avocado.glb, an error is output to jgltf-browser's log.

2017-10-03 00:48:07.193 普通            : Reading image 2 DONE
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
        at de.javagl.jgltf.model.io.v2.BinaryAssetCreatorV2.create(BinaryAssetCreatorV2.java:128)
        at de.javagl.jgltf.model.io.v2.GltfModelWriterV2.writeBinary(GltfModelWriterV2.java:88)
        at de.javagl.jgltf.model.io.GltfModelWriter.writeBinary(GltfModelWriter.java:148)
        at de.javagl.jgltf.model.io.GltfModelWriter.writeBinary(GltfModelWriter.java:120)
        at de.javagl.jgltf.browser.GltfBrowserApplication.saveBinary(GltfBrowserApplication.java:812)
        at de.javagl.jgltf.browser.GltfBrowserApplication.access$500(GltfBrowserApplication.java:76)
        at de.javagl.jgltf.browser.GltfBrowserApplication$5.lambda$actionPerformed$0(GltfBrowserApplication.java:217)
        at de.javagl.jgltf.browser.GltfBrowserApplication.saveAs(GltfBrowserApplication.java:733)
        at de.javagl.jgltf.browser.GltfBrowserApplication.access$300(GltfBrowserApplication.java:76)
        at de.javagl.jgltf.browser.GltfBrowserApplication$5.actionPerformed(GltfBrowserApplication.java:217)
        at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
        at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2348)
        at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:402)
        at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:259)
        at javax.swing.AbstractButton.doClick(AbstractButton.java:376)
        at javax.swing.plaf.basic.BasicMenuItemUI.doClick(BasicMenuItemUI.java:833)
        at javax.swing.plaf.basic.BasicMenuItemUI$Handler.mouseReleased(BasicMenuItemUI.java:877)
        at java.awt.AWTEventMulticaster.mouseReleased(AWTEventMulticaster.java:289)
        at java.awt.Component.processMouseEvent(Component.java:6533)
        at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
        at java.awt.Component.processEvent(Component.java:6298)
        at java.awt.Container.processEvent(Container.java:2236)
        at java.awt.Component.dispatchEventImpl(Component.java:4889)
        at java.awt.Container.dispatchEventImpl(Container.java:2294)
        at java.awt.Component.dispatchEvent(Component.java:4711)
        at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4888)
        at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4525)
        at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4466)
        at java.awt.Container.dispatchEventImpl(Container.java:2280)
        at java.awt.Window.dispatchEventImpl(Window.java:2746)
        at java.awt.Component.dispatchEvent(Component.java:4711)
        at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
        at java.awt.EventQueue.access$500(EventQueue.java:97)
        at java.awt.EventQueue$3.run(EventQueue.java:709)
        at java.awt.EventQueue$3.run(EventQueue.java:703)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86)
        at java.awt.EventQueue$4.run(EventQueue.java:731)
        at java.awt.EventQueue$4.run(EventQueue.java:729)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:76)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
        at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
javagl commented 6 years ago

Thanks again. These issues should be fixed with the commit https://github.com/javagl/JglTF/commit/6c178f1432fcaea3cb4fe0ed53d886585880ee63 (details in the commit comment).

I'll leave this issue open for now, because I have to assume that there may be further (hopefully minor) issues like these.

In order to eliminate these bugs, I'll probably try to set up a better testing procedure. Maybe something like

But today and tomorrow, I'll have to take a closer look at the new interpolation methods (cubic and catmull-rom).

cx20 commented 6 years ago

@javagl Thank you for the quick fix. I tried several models and seems to be able to convert without problems. I also confirmed that it can be displayed at https://gltf-viewer.donmccurdy.com/ and https://sandbox.babylonjs.com/ and Paint 3D. Box.glb, Triangle.glb, Duck.glb, MetalRoughSpheres.glb, etc...

javagl commented 6 years ago

Thanks, good to hear that!

I also just tested the roundtrip of "loading->saving as GLB -> loading" with the "Polly" model from https://github.com/KhronosGroup/glTF-Blender-Exporter/tree/master/polly and it seemed to work.

However, I'll leave this issue open for possible further fixes that might be necessary regarding the I/O, at least until I have some better test coverage.

cx20 commented 6 years ago

BTW, I tried project_polly.gltf displayed in other viewers, skinning animation seems not to be displayed correctly. I do not know if this is a model problem or a viewer problem. Should I report somewhere?

Three.js + glTF Loader + project_polly.gltf result: image

Babylon.js + glTF Loader + project_polly.gltf result: image

javagl commented 6 years ago

If this is the original polly model, then it would probably be best to report these issues in the babylon and three.js repos, respectively. The model is supposed to pass 2.0 validation (although I didn't check it again), so every viewer should be able to display it.

There are some challenges in this model, e.g. the transparent sign showing the name "POLLY". But in this sense, the model is really a "feature-benchmark", raising the bar slightly above the "Duck" or "SimpleTriangle" models ;-)

(It seems to be "properly" displayed in JglTF, at least regarding the geometry. The transparency is not handled yet, of course. And there is only with the odd, single point light, which looks boring :-( )

The spheres in the second screenshot are probably the eyes - ouch.

cx20 commented 6 years ago

Thanks for the advice. I would like to report the viewer's problem to Three.js and Babylon.js repos.

I think that it is almost correctly displayed in JglTF, but since it is dark overall, it feels good to set the lights a bit more bright. (or it would be nice to be able to set the light settings as options.)

The spheres in the second screenshot are probably the eyes - ouch.

I think so, too.

javagl commented 6 years ago

I think that it is almost correctly displayed in JglTF, but since it is dark overall, it feels good to set the lights a bit more bright. (or it would be nice to be able to set the light settings as options.)

Setting the light options would certainly be a second step. As you know, most viewers allow selecting an environment light map. One problem is: Purely metallic objects look pretty cool with environment maps, but with "simple" lights, they appear all black. (E.g. the floor of the polly model)

I hope that I can integrate (selectable) environment maps in JglTF, too. And of course, the other lights should not just be one single point light, but all sorts of lights that will soon be required for the KHR_Lights extension anyhow.

Although this should be interesting, it is not the top priority right now.

javagl commented 2 months ago

This is outdated. I opened https://github.com/javagl/JglTF/issues/116 to keep track of the goal of testing the IO more thoroughly.