CesiumGS / obj2gltf

Convert OBJ assets to glTF
Apache License 2.0
1.71k stars 307 forks source link

conversion error #47

Closed andyli closed 7 years ago

andyli commented 7 years ago

I've installed obj2gltf v0.1.7 using npm install -g obj2gltf. My node version is v7.5.0. When I try to convert my file wall_and_door.obj, which you can find at https://github.com/andyli/aframe-test/tree/1fe0f0cc8af332b9a8c7880437a8678e99e3b6e2/wall_and_door, there is an error:

D:\andy\workspace\VRWorkshop\wall_and_door>obj2gltf wall_and_door.obj

C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\node_modules\cesium\Source\ThirdParty\crunch.js:43
    var Module;if(!Module)Module=(typeof Module!=="undefined"?Module:null)||{};var moduleOverrides={};for(var key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}var ENVIRONMENT_IS_WEB=typeof window==="object";var ENVIRONMENT_IS_WORKER=typeof importScripts==="function";var ENVIRONMENT_IS_NODE=typeof process==="object"&&typeof require==="function"&&!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_WORKER;var ENVIRONMENT_IS_SHELL=!ENVIRONMENT_IS_WEB&&!ENVIRONMENT_IS_NODE&&!ENVIRONMENT_IS_WORKER;if(ENVIRONMENT_IS_NODE){if(!Module["print"])Module["print"]=function print(x){process["stdout"].write(x+"\n")};if(!Module["printErr"])Module["printErr"]=function printErr(x){process["stderr"].write(x+"\n")};var nodeFS=require("fs");var nodePath=require("path");Module["read"]=function read(filename,binary){filename=nodePath["normalize"](filename);var ret=nodeFS["readFileSync"](filename);if(!ret&&filename!=no
Error
    at new DeveloperError (C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\node_modules\cesium\Source\Core\DeveloperError.js:44:19)
    at Function.Cartesian3.normalize (C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\node_modules\cesium\Source\Core\Cartesian3.js:422:19)
    at LineStream.<anonymous> (C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\lib\obj.js:192:41)
    at emitOne (events.js:96:13)
    at LineStream.emit (events.js:189:7)
    at readableAddChunk (_stream_readable.js:176:18)
    at LineStream.Readable.push (_stream_readable.js:134:10)
    at LineStream.Transform.push (_stream_transform.js:128:32)
    at LineStream._pushBuffer (C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\node_modules\byline\lib\byline.js:126:17)
    at LineStream._transform (C:\ProgramData\nvm\v7.5.0\node_modules\obj2gltf\node_modules\byline\lib\byline.js:117:8)
    at LineStream.Transform._read (_stream_transform.js:167:10)
    at LineStream.Transform._write (_stream_transform.js:155:12)
    at doWrite (_stream_writable.js:329:12)
    at writeOrBuffer (_stream_writable.js:315:5)
    at LineStream.Writable.write (_stream_writable.js:241:11)
    at ReadStream.ondata (_stream_readable.js:555:20)
shehzan10 commented 7 years ago

I believe this is because you have a vertex normal that has a magnitude of 0 on line 13.

emackey commented 7 years ago

Perhaps the converter could do something more useful than throw an uncaught DeveloperError? Can it explain to the user what problem was found, and where?

Better yet, can't the converter just print out a warning and work around the problem with UNIT_Z or something? Real-world data is lumpy and has minor problems sprinkled around. Ordinary users want tools that can tolerate a few bad vectors. You could print a warning, you could put UNIT_Z or UNIT_Y, or you could even copy the zero-length normal through to the output along with the warning. Sure the model won't be perfect but the user will appreciate having it much more than a stack trace. The user would load the bad model into something else, see the misdirected normal vectors, and go fix their model instead of filing bug reports like this one. OK, rant over now, thanks for reading this far :smile:

shehzan10 commented 7 years ago

@emackey This issue has been popping up and we have an issue open for this https://github.com/AnalyticalGraphicsInc/gltf-pipeline/issues/242. I think this should get fixed in the near future.

lilleyse commented 7 years ago

This converts correctly once #49 is in, and mostly in part due to this fix: https://github.com/AnalyticalGraphicsInc/cesium/pull/5032

lilleyse commented 7 years ago

Fixed in #49