CesiumGS / obj2gltf

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

Update triangulation algorithm #157

Closed lilleyse closed 6 years ago

lilleyse commented 6 years ago

Fixes https://github.com/AnalyticalGraphicsInc/obj2gltf/issues/147 by updating to the latest triangulation code used in Cesium. This fixes a model that wasn't triangulating correctly in master. This fix required updating Cesium so I updated other dependencies with it.

csxirc5h.i2b.obj

Before and after:

triangulated-incorrect triangulated-correct

There are still some back facing polygons but I confirmed they are present in the original model too. We don't yet do that sort of correction.

(Backfacing polygons on the steps)

steps

I checked that the new method still triangulates these models correctly:

https://github.com/AnalyticalGraphicsInc/obj2gltf/issues/33 https://github.com/AnalyticalGraphicsInc/obj2gltf/issues/50 https://github.com/AnalyticalGraphicsInc/obj2gltf/issues/79

One thing I'm not completely sure about is whether PolygonPipeline.computeWindingOrder2D is enough to check the winding order. Would it need to use the normal like the 3-sided case?

lilleyse commented 6 years ago

@hpinkos could you check this out? The new code starts here: https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/157/files#diff-608b0b654865416bb99fb5d336cf3307R297

hpinkos commented 6 years ago

I don't have write access to this repo so I can't merge it, but this looks good to me @lilleyse

lilleyse commented 6 years ago

Thanks @hpinkos. I'm going to go ahead and merge.