MTLLoader improvements

[jonnenauha]

• Deprecate badly named setBaseUrl in favor of setTexturePath
• If .path is set but .texturePath is not, select .path to be passed into MaterialCreator.
• If neither is set, try to auto detect base path from .mtl source URL. the .load() function now passes the source URL into .parse(). (reverted)
• Don't override already found diffuse texture, makes behavior the same as bump map logic.
• Make single quotes consistent and other style cleanup.
• Add documentation to what setPath/setTexturePath actually do, also more to load/parse.
• These changes do not change previous behavior of the loader. Updating users will see a deprecation warning if their code calls setBaseUrl but the value is still set.
• Don't break parsed absolute URL texture references.

I think auto detecting the texture base path is pretty neat. You can go from this

mtlLoader.setPath("my/relative/path/");
// mandatory duplicate call for textures to load correctly
mtlLoader.setBaseUrl("my/relative/path/");
mtlLoader.load("my.mtl");
.... or
// mtl loads but textures break
mtlLoader.load("my/relative/path/my.mtl");

to

mtlLoader.setPath("my/relative/path/");
// explicit texture path not set, will use setPath as base
mtlLoader.load("my.mtl");
... or
// just works, texture path "my/relative/path/" resolved automatically
mtlLoader.load("my/relative/path/my.mtl");

[mrdoob]

That last logic is dangerous though... What if the real path was images/images/test.png and the user did setPath('images/') and then load('images/test.png')?

[jonnenauha]

Hmm, not sure what you mean with that example. You'll be calling .load() with a .mtl reference, not a .png. I guess that was a typo?

In cases where user call either or both setPath or setBaseUrl (now renamed) this has the same behavior as before. The texture path is preferred, then mtl base path is selected. If none of those are set it will auto resolve it to the dir of the mtl URL passed into load().

The only case where this auto resolve does change behavior is this file structure;

my.png
assets/my.mtl

If you called load("assets/my.mtl") the previous code would have "correctly" loaded referenced my.png from the root if setPath or setBaseUrl was not called. I would hesitate to guess this setup would be extremely rare. In fact i think this would have resulted in a texture ref "undefinedmy.png" because the creator.baseUrl would have been undefined and there is a blind concat :)

Could you describe a directory/file setup where you suspect the code might do the wrong thing when it was working with the previous code? I cant think of one.

Edit: Did you mean this?

loader.setPath("images/");
loader.load("images/test.mtl");
// would fetch "images/images/test.mtl" and textures from "undefined{textureRef]"
// youd have to set
loader.setBaseUrl("/images/");
// for "images/images/test.mtl" and textures from "images/{textureRef]"

[mrdoob]

I'll study the code thoroughly tomorrow 😇

[jonnenauha]

ok, let me know if you find a failure case.

We can also remove the auto resolve, it just seems a lot easier for the end user. With the auto resolve the only case where you would need to call setTexturePath If you have textures outside of the .mtl directory. I mean in its parent folders, if you have them in a subfolder next to the .mtl file, the auto resolve would still work correctly (texture ref in mtl "textures/test.png").

[jonnenauha]

Btw. should also fix not to prefix base path to the found texture ref if its a absolute URL. This would mangle and break it.

Simple if (ref.toLowerCase().indexOf("http://") === -1 && ref.toLowerCase().indexOf("https://") === -1 ) { prefixWithBase }; would suffice. I can see someone putting full/absolute URLs also as textures (seen this at work many times).

[mrdoob]

Hmm, I just have the feeling that the auto resolve will create problems at some point and we'll spend a lot of time trying to make sure it works in all the possible cases. We've been in similar situations before and the solution was always to let the user control that.

Everything else looks great though!

[jonnenauha]

Alright. I'll remove it and document that setPath or setTexturePath needs to be set for textures to load successfully before calling load/parse. I'll also add the abs URL check for found refs, its a good addition.

[jonnenauha]

@mrdoob there we go.

Auto resolve removed. Added bunch of documentation. Fixed breaking parsed absolute URLs. And fixed deprecation warnings in the obj + mtl loader example.

[mrdoob]

Almost there!

We have also been trying to move away from these kind of public methods whenever possible (people would start using them and get mad when they get removed).

Considering that this method is mainly used inside THREE.MTLLoader.MaterialCreator we could just define it locally.

[jonnenauha]

Yeh, updated.

[mrdoob]

How about... if ( /^https?:\/\//i.test( url ) )?

[jonnenauha]

Hmm, I'm somehow hesitant to create regexps on the spot my self, but the function is not called that often ;) If you'd prefer it i can change it, gimme a sec.

[jonnenauha]

Done, it is a lot cleaner to read, toLowerCase is not free either so meh :)

[mrdoob]

Sweet!

[mrdoob]

Thanks!