search

CesiumGS / obj2gltf

Convert OBJ assets to glTF
Apache License 2.0
1.7k stars 305 forks source link

Short circuits the image combining if diffuse and alpha textures are the same #205

Closed tfili closed 5 years ago

tfili commented 5 years ago

Some materials seem to have the diffuse and alpha textures set to the same file, so no need to combine them. Also added a test.

tfili commented 5 years ago

@lilleyse I added them. This should be good to go. It looks like in certain circumstances it wasn't using the returned texture to check transparency so I fixed that too.

lilleyse commented 5 years ago

After testing and looking at the code closer I realized that https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/205#discussion_r305466284 was actually an unnecesary suggestion and it was already setting the blend mode correctly because it sets transparent if an alpha texture exists. I think reverting back to https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/205/commits/b0de7d1bc1373855b2b7197588d427b1e89f8383 is ok but adding the test change in https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/205#discussion_r305466284.

Sorry for slowing this down.

tfili commented 5 years ago

@lilleyse Changed it back but kept the tests. Everything still passes.

tfili commented 5 years ago

@lilleyse Bump