search

KhronosGroup / glTF-Blender-IO

Blender glTF 2.0 importer and exporter
https://docs.blender.org/manual/en/latest/addons/import_export/scene_gltf2.html
Apache License 2.0
1.48k stars 316 forks source link

Windows test failure #965

Closed emackey closed 4 years ago

emackey commented 4 years ago

Running this project's unit tests locally on a Windows box has apparently been broken for some time now. Specifically the round-trip tests all fail, claiming they can't find texture files.

This morning I got around to git-bisecting the problem, and it pointed to this being introduced with commit 19c1280a1342766f, which is one of @scurest's image importer upgrades. Prior to that, the imported images were apparently being copied to a temp file or something, and there was no error reading the temp file (but it was inefficient). One key difference is the path to the temp file was absolute, but in the new version, the path to the source image file is relative.

An exception is thrown from this line here:

https://github.com/KhronosGroup/glTF-Blender-IO/blob/d3f00cfd9cb328604753c10ae2e175c54a67ec5b/addons/io_scene_gltf2/blender/imp/gltf2_blender_image.py#L61

This could well be a Blender bug, because the path appears to be correct, and Blender is launched from the correct folder. One such command line is shown here (adjust the output folder path for your own machine of course, and run from the CMD window in the tests folder).

blender28 -b --addons io_scene_gltf2 -noaudio --python roundtrip_gltf.py -- roundtrip/01_alpha_blend/01_alpha_blend.gltf D:\Git\glTF-Blender-IO\tests_out\roundtrip\01_alpha_blend\outblender28

Luckily the fix (or workaround) seems easy, just ask Python to make the path absolute before handing it off to Blender. I've tested and found two places where a fix is effective:

  1. We could modify importer line 61 to wrap path as os.path.abspath(path)

or:

  1. We could modify test's roundtrip line 29 to change argv[0] to os.path.abspath(argv[0])

I'm inclined to ask for the change in the importer itself, rather than in just the tests, in case this fixes some edge case issue with relative imports of glTF files on Windows machines. But I'm OK with either.

scurest commented 4 years ago

I'm inclined to ask for the change in the importer itself, rather than in just the tests, in case this fixes some edge case issue with relative imports of glTF files on Windows machines

+1