mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
100.51k stars 35.21k forks source link

MMDLoader Still has colorization Issues #28336

Open danpaldev opened 2 months ago

danpaldev commented 2 months ago

Description

This is a follow up for an old issue (https://github.com/mrdoob/three.js/issues/26553) that was already closed but seems to be still having problems on r162

As the guy in the original issue reports, problem seems to be with PMX files. Every PMX model I loaded had washed out colors, so I tried to test this by using another renderer as a sanity check.

Using a C++ renderer from here: https://github.com/benikabocha/saba I get this: image

Using MMDLoader + ThreeJS image

As requested last time, I uploaded the PMX model here in order to make any possible debugging easier: https://drive.google.com/file/d/1NCDDCUkThV_PCZtsXpZ81rMzChUpi9Q1/view?usp=sharing

Reproduction steps

  1. Load MMD Loader
  2. Import a pmx model

Code

        import * as THREE from "three";

        import { GUI } from "three/addons/libs/lil-gui.module.min.js";

        import { OutlineEffect } from "three/addons/effects/OutlineEffect.js";
        import { MMDLoader } from "three/addons/loaders/MMDLoader.js";
        import { MMDAnimationHelper } from "three/addons/animation/MMDAnimationHelper.js";
        import { OrbitControls } from "three/addons/controls/OrbitControls.js";

        const modelFile = "models/mmd/cat_girl/ヨッシー式ラインクラフト.pmx";

        helper = new MMDAnimationHelper();

        const loader = new MMDLoader();

        loader.load(
          modelFile,
          function (object) {
            mesh = object;
            console.log(mesh);
            mesh.position.y = -10;

            scene.add(mesh);

            initGui();
          },
          onProgress,
          null
        );

Live example

No live example

Screenshots

image

image

Version

r162

Device

Desktop

Browser

Firefox

OS

MacOS

danpaldev commented 2 months ago

Here's the full code that I used for loading the model in a gist, I just didn't want to pollute the issue description: https://gist.github.com/danpaldev/0d4b62a9b1ebdce66151d400beeafef7

Mugen87 commented 2 months ago

Your asset has an emissive color applied. You can reset it to the default value 0x000000 like so:

mesh = object;

for ( const material of mesh.material ) {

    material.emissive.set( 0x000000 );

}
image

MMDLoader honors the ambient setting of MMD by applying it to emissive. There is a fix in place to avoid too bright materials when map+ambient is used.

https://github.com/mrdoob/three.js/blob/4ec57f5014e57199fff3827972474e43cc9450ba/examples/jsm/loaders/MMDLoader.js#L1146-L1154

@takahirox Maybe this fix does not always produce the expected result?

takahirox commented 2 months ago

Ambient color stuff is a known issue for me. MMD ambient is not perfectly compatible with Three.js regular materials. I'm thinking of requesting users to adjust color in their user code if they are not satisfied.

Another option in my mind would be, we have MMD specific custom shader material now. So we may be able to implement ambient support more properly in it. The current fallback was introduced when I was thinking of using regular Three.js material.

GitHubDragonFly commented 1 month ago

@Mugen87 changing emissive is a neat improvement even for my MMD Viewer which is still using r131 of three js.

I kind of wish it was mentioned before.

mrdoob commented 1 month ago

Can we remove the emissive code for the time being?

takahirox commented 1 month ago

Probably color problem with other models happens if we just remove the emissive hack. At least I remember that color looked very unexpected in some MMD standard(?) models if I ignored MMD material ambient while I was testing.

Perhaps we can write a note in MMDLoader document for the time being. And at some point we can properly implement MMD material ambient in MMD specific material. (PR is welcome!)

takahirox commented 1 month ago

By the way I'm wondering if this line is actually executed with those models. https://github.com/mrdoob/three.js/blob/4ec57f5014e57199fff3827972474e43cc9450ba/examples/jsm/loaders/MMDLoader.js#L1323 Can someone create a live online example for me?

Mugen87 commented 1 month ago

I have tested this locally when initially investigating the issue and I can confirm the line is executed. The ambient value in the model is (1,1,1) and it is scaled down by the mentioned line to (0.2, 0.2, 0.2).

takahirox commented 1 month ago

Thanks. 0.2 is just from my rule of thumbs. Using smaller number would be another option.

And I found that in the example AmbientLight is used. Wondering how it looks like if removing the ambient light. And does the c++ reference uses ambient (or similar) light, too?

mrdoob commented 1 month ago

Probably color problem with other models happens if we just remove the emissive hack. At least I remember that color looked very unexpected in some MMD standard(?) models if I ignored MMD material ambient while I was testing.

The MMDLoader was done before all the color spaces, color management and tone mapping work though.

Maybe the emissive hack is no longer needed now? However... even if it is needed, I would avoid using emissive as we're "corrupting" the model.

If someone exported the scene as gltf and then rendered it in blender, the character would be emitting light...

takahirox commented 1 month ago

If I remember correctly MMD material ambient color in MMD is handled very similarly to emissive color in regular 3D CG engines (I even don't remember the difference), so

the character would be emitting light...

so this might be expected result as model originally made in MMD.

But I need to review the MMD material spec to think how we can apply them more properly and make it aligned with our color space because I saw it last time years ago. (So comment or correction is very welcome from who knows MMD spec well.)

Maybe the emissive hack is no longer needed now?

Even if we end up with deciding to remove emissive hack, I think we need to reflect MMD material ambient color to some Three.js material parameters somehow. I remember that just ignoring MMD material ambient color produced very unexpected result in some MMD models.

danpaldev commented 1 month ago

Your asset has an emissive color applied. You can reset it to the default value 0x000000 like so:

mesh = object;

for ( const material of mesh.material ) {

  material.emissive.set( 0x000000 );

}
image

MMDLoader honors the ambient setting of MMD by applying it to emissive. There is a fix in place to avoid too bright materials when map+ambient is used.

https://github.com/mrdoob/three.js/blob/4ec57f5014e57199fff3827972474e43cc9450ba/examples/jsm/loaders/MMDLoader.js#L1146-L1154

@takahirox Maybe this fix does not always produce the expected result?

That did the trick, thanks for the tip. I guess this trick could be documented in the loader's docs in the meantime?

takahirox commented 1 month ago

@danpaldev Can you check how your models look like if removing the ambient light in your example?

danpaldev commented 1 month ago

@takahirox Ambient light off + material.emissive.set( 0x000000 ); image

Ambient light on + material.emissive.set( 0x000000 ); image

Ambient light off with an unmodified material.emissive image

takahirox commented 1 month ago

Thanks for sharing the results. I thought the washed out color problem would be caused by applying the both MMD material ambient + ambient light but it seems wrong. (By the way I noticed that the ambient light is not added to scene in your example)

I started to feel that

The ambient value in the model is (1,1,1)

something seems off. My understanding is MMD material ambient color is very similar to emissive color. So ambient value (1, 1, 1) would make the model always looks white but it doesn't in the reference. I think I need to review the MMD spec again...

takahirox commented 1 month ago

Ambient color in MMD seems to be affected by color map, like (diffuseColor * lighting + ambientColor) * colorMap.rgb. So makes sense why looking too bright (especially without emissive modification) if color map is specified in the current implementation.

Thinking how to simulate in Three.js and also how to make it look expected when exported.

takahirox commented 1 month ago

Progress...

image