aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.54k stars 3.92k forks source link

Color management issues for .obj + .mtl #5307

Open nightgryphon opened 1 year ago

nightgryphon commented 1 year ago

A-Frame Version: 1.4.2 Platform / Device: chrome for windows different versions, oculus quest2 browser

I'm trying to export scene from Blender to my AFrame based web page for VR. For some reasons i prefer to use obj+mtl format. I found a few issues regarding loading model to AFrame:

  1. All the materials during load assumed to have sRGB colors and converted to linear color space using convertSRGBToLinear(). BUT blender export the exact colors i wish to see in scene and i can not find a way to disable this mandatory color "correction" which ruine the model. Take a look at MTLLoader.js line 387 method createMaterial_( materialName )
  2. For models which use material more than once all material colors become black or about black. I'm not sure about reason but a think that's because that correction apply multiple times to same material.
  3. The diffuse map textures become dark (also color space "correction") until i set renderer="colorManagement: true"

Please can you help me to overcome this obj+mtl loading issues or add an option to disable unnecessary color "corrections" for \<a-obj-model>?

mrxz commented 1 year ago

Things have changed with regards to colour management in both three.js and A-Frame recently, so it might be a good idea to try and get things working on master. Also note that while obj+mtl is supported (and should ideally work out-of-the box), it's precisely colour management that can be undefined, hence also the following warning from the Three.js docs:

⚠️ WARNING: Many formats for 3D models do not correctly or consistently define color space information. While three.js attempts to handle most cases, problems are common with older file formats. For best results, use glTF 2.0 (GLTFLoader) and test 3D models in online viewers early to confirm the asset itself is correct. (https://threejs.org/docs/#manual/en/introduction/Color-management)

In any case, I believe the diffuse colour is taking the following journey:

  1. You configure the desired colour in Blender (in sRGB)
  2. Blender internally converts it to scene linear
  3. Blender keeps the colours linear when exporting to .obj+.mlt
  4. Three.js assumes the colours to be sRGB and converts them to linear
  5. (If colorManagement: true) A-Frame's obj-model component converts the colour again from sRGB to linear
  6. (If colorManagement: true) In case a material is shared across meshes obj-model performs step 5 multiple times (your point 2)
  7. In the shader the diffuse colour is either outputted as-is (colorManagement: false) or converted from linear -> sRGB (colorManagement: true)

On master both steps 5 and 6 are gone and colorManagement: true is the default. Per Three.js documentation the diffuse colour should be in 'Linear-sRGB' which step 7 then properly converts back into sRGB. The only problem remaining is the discrepancy between Blender and Three.js about whether or not colours in .mtl are sRGB or linear-sRGB. I'm afraid that is something that isn't easily solved as there doesn't seem to be any "right" interpretation which one it should be :-/

My advice would be to consider switching to .gltf/.glb. Otherwise:

nightgryphon commented 1 year ago

The issue not depends of colorManagement: true setting. It happens for both true and false setting.

I've switched to aframe-master.js but the issue at step #5 is still there. I've checked the code trace and unconditional color conversion code is unchanged.

AFRAME/src/components/obj-model.js line 60 call materials.preload(); which calls AFRAME/nodemodules/super-three/examples/jsm/loaders/MTLLoader.js method createMaterial( materialName ) @ line 324 which perform unconditional color conversion @ line 387

case 'kd':
        // Diffuse color (color under white light) using RGB values
        params.color = new Color().fromArray( value ).convertSRGBToLinear();

It seems the issue belongs to used super-three module example. As that code is provided as just an example may be it is good idea to import OBJLoader.js MTLLoader.js from super-three/examples to aframe itself and fix the things?

Also it can be good to add an option to specify which color space to use while loading model so it will be possible to override default behavior in case of troubles.

mrxz commented 1 year ago

There are a couple of issues at play simultaneously, some of which do depend on colorManagement: true (or at least they should). The second point you listed about the material getting progressively darker the more its shared between meshes should only happen with 1.4.2 and colorManagement: true. On master this should not occur regardless of the colorManagement setting.

If you're on A-Frame master and have colorManagement: true the only remaining issue should be that your .mtl file contains linear-sRGB colours, where Three.js expects sRGB colours. (This issue manifest between steps 3-4)

It seems the issue belongs to used super-three module example. As that code is provided as just an example may be it is good idea to import OBJLoader.js MTLLoader.js from super-three/examples to aframe itself and fix the things?

The issue would belong to three.js, as the super-three fork of A-Frame doesn't change anything about the loaders. However, whether this is actually a bug depends on if the colours in .mlt files are supposed to be linear-sRGB or sRGB. From skimming the specification (http://paulbourke.net/dataformats/mtl/) this seems pretty much undefined, I'm afraid. :-/

Here's an interesting remark from a Three.js discussion about colours in .mlt files, indicating that it might be a bug in Blender's .obj exporter: https://github.com/mrdoob/three.js/issues/23283#issuecomment-1018829533

Also it can be good to add an option to specify which color space is used within loaded model so aframe can provide correct color management based on user settings instead of guessing.

That could be a nice solution, just note that "a model" might use more than one colour space (e.g. sRGB diffuse textures and linear-sRGB vertex colours). So such an option should specifically apply to the MTLLoader and only the material colours contained within. Since A-Frame does little more than wrap Three.js, this option would first need to be available upstream in three.js before it can be expose in A-Frame.

Easiest solution remains to switch over to .gltf/.glb as Blender has an exporter for it built-in and there's no ambiguity about colour spaces. Otherwise the quickest route would be to either manually convert the colours back after loading or patching the MTLLoader in a fork.

There is also still an issue of loosing color after setting the envMap so i has to restore color every time after setting envMap

The envMap and color properties of a Material shouldn't impact each other. Doesn't seem (directly) related to the color management issues, so could you create a separate issue for it. Ideally with a small example reproducing the issue?

nightgryphon commented 1 year ago

i've rechecked the envMap issue and it seems it is part of color conversion thing. Sorry for misinforming.

dmarcos commented 1 year ago

Thanks all for clarifying

dmarcos commented 1 year ago

Should we close this?

nightgryphon commented 1 year ago

It seems in 'master' colors become better but something went wrong with normal maps from .MTL ( 'norm' .mtl command). Will recheck tomorrow...

nightgryphon commented 1 year ago

I've rechecked normal maps and they looks ok. Norm maps become about invisible in particular scene because of changed colors/lights.

But issue with mandatory converting colors in AFRAME/node_modules/super-three/examples/jsm/loaders/MTLLoader.js still exists. Is it possible to move OBJLoader.js / MTLLoader.js to AFRAME tree and fix that?

mrxz commented 1 year ago

Is it possible to move OBJLoader.js / MTLLoader.js to AFRAME tree and fix that?

Making custom changes to three.js leads to more maintenance, and can create incompatibilities with libraries written against vanilla three.js. Given that the issue is either in Three.js or Blender (depending on how you look at it), the proper route would be opening a bug report on (one of) those projects. Three.js at least seems open to the idea of a flag on the loader, so that might be a good start. Once fixed upstream, it can eventually be exposed in A-Frame as well as a flag on obj-model.

Until then, I think a note/warning to the obj-model documentation should be added. As a workaround you can export as .gltf/.glb, use a component to convert colours back after loading or include your own version of MTLLoader.js in your project, replacing the bundled version.

With Blender being such a common tool, it would be nice to see Blender -> .obj/.mlt -> A-Frame work out of the box. At the same time any Other tool/3D asset pack/etc... -> .obj/.mlt -> A-Frame should also work out of the box, and these two are sadly at odds. At least in the case of Blender you have to option to export as glTF.

nightgryphon commented 1 year ago

Huh. Will try both ways. I've wrote the message to that THREE discussion but it lasts more than year and not seems to be fixed soon so i also wish to try to build AFrame with custom fixed MTLLoader.js. Unfortunately i'm not enough familiar with nodejs. Please can you help and point me to correct manual how to substitute specific node module for all for libraries which reference it in project?

mrxz commented 1 year ago

Please can you help and point me to correct manual how to substitute specific node module for all for libraries which reference it in project?

With the way A-Frame is bundled you can't easily override dependencies the npm way. In your case I would simply grab a slightly older copy of MTLLoader.js from the now-removed three.js/examples/js/loaders directory: https://github.com/mrdoob/three.js/blob/r147/examples/js/loaders/MTLLoader.js. Make your changes and load it after loading aframe. That should do the trick

In case your project uses ES6 modules, you can grab a recent copy of MTLLoader from examples/jsm/loaders. Then import the MLTLoader in your project and assign it to the global THREE.MTLLoader.

mrxz commented 1 year ago

Hard to tell without knowing your full setup, but in general you don't want to explicitly refer to super-three. The error is correct in the sense that neither three nor super-three export MTLLoader (its part of the examples, after all). It's A-Frame that adds these onto the THREE global, so something like the following should work:

AFRAME.THREE.MTLLoader = MyMTLLoader;

Alternatively you could use window.THREE but since you need to be sure AFRAME has loaded, might as well access THREE via it.

dmarcos commented 9 months ago

Is there anything to be done here?