playcanvas / playcanvas-gltf

glTF 2.0 support for the PlayCanvas Engine
MIT License
102 stars 31 forks source link

KHR_materials_pbrSpecularGlossiness extended model does not display properly #27

Closed cx20 closed 6 years ago

cx20 commented 6 years ago

I tried a model added to glTF-Sample-Models.

However, it seems that it is not displayed properly in glTF loader of PlayCanvas.

Babylon.js PlayCanvas
image image

https://github.com/KhronosGroup/glTF-Sample-Models/tree/spec-gloss-vs-metal-rough/2.0/SpecGlossVsMetalRough

kungfooman commented 6 years ago

It boils down to material.specular.set(1, 1, 1); as wrong default setting

https://github.com/playcanvas/playcanvas-gltf/blob/master/src/playcanvas-gltf.js#L345

            if (specData.hasOwnProperty('specularFactor')) {
                color = specData.specularFactor;
                // Convert from linear space to sRGB space
                material.specular.set(Math.pow(color[0], 1 / 2.2), Math.pow(color[1], 1 / 2.2), Math.pow(color[2], 1 / 2.2));
                material.specularTint = true;
            } else {
                material.specular.set(1, 1, 1); // <<<<< bug
                material.specularTint = false;
            }

It makes all the models I apply that specular color to highly metallic. The default material specular color is [0,0,0,1], which makes the exported model from blender looks kinda like it looks in blender.

So to fix the metallic look you can either comment the default specular color setting out or set it like material.specular.set(0, 0, 0); in playcanvas-gltf code

willeastcott commented 6 years ago

I don't quite understand. Check out the default specular color when specularFactor is not specified, according to the extension spec:

https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_materials_pbrSpecularGlossiness#specularfactor

It's stated as [1.0, 1.0, 1.0].

willeastcott commented 6 years ago

Looping in @MackeyK24 (who implemented support for KHR_materials_pbrSpecularGlossiness).

MackeyK24 commented 6 years ago

I think default specular color is 1, 1, 1

But apparently is should be 0,0,0 IF specularFactor is null or undefined

kungfooman commented 6 years ago

I made a video how specular.set(1,1,1) looks for different materials, maybe PlayCanvas itself got it upside-down? I have no clue what the "standard" is. Some days ago I still used pc.version==1.6 and I recall that ONLY [1,1,1] was metallic, everything below (like 0.99) was completely "non-specular"

https://www.youtube.com/watch?v=Ts3R3sp7LqM

edit: engine used in video is pc.version==1.7, just build few days ago

MackeyK24 commented 6 years ago

Sorry I can’t tell what’s going in the video. Can you show the bottles from example above... so I can see what it should look like and what PlayCanvas GLTF is showing it as

MackeyK24 commented 6 years ago

I think we (playcanvas-gltf) implementation of material.specular is backwards...

Using Unity as an Example: I made a Standard Specular Setup:

If i use LIGHTER colors as the Specular EVERYTHING IS WHITER... Full White makes it REALLY WHITE LOSES ALL ORIGINAL DIFFUSE COLORING

IF i use DARKER color is shows the original diffuse color proper like.

So i think the PlayCanvas GlTF material pipeline has the specular color inverted when material.useMetalness = false (SPECULAR WORKFLOW)

Even though the GLTF spec says WHITE... DEFAULT TO BLACK... Works great from unity as well.

Also this is ONLY if specularFactor NOT present... The only time this should happen is if the specularFactor is not the DEFAULT value that the EXPORTER says it should be...

For example My UnityGLTF Exporter check use a default specular color value of 0.2, 0.2, 0.2 ... the Unity Default Specular...

But anyways... Defaulting to BLACK seems to address the problem for me

cx20 commented 6 years ago

I tried display test again with the latest PlayCanvas library and glTF Loader. However, the display result does not seem to have changed. Is the usage wrong? https://cdn.rawgit.com/cx20/gltf-test/6bb8a978885954ce92df33528d569adcbdea8a6e/examples/playcanvas/index.html?category=tutorialModels&model=SpecGlossVsMetalRough&scale=10&type=glTF

MackeyK24 commented 6 years ago

Well i tried again and again... But i cant even get the playcanvas-gltf viewer to load ANYTHING.

Can you please zip up your test project with the playcanvas gltf viewer you are using and the model you are trying to test... and send to me so i can take a look at it.

MackeyK24 commented 6 years ago

FYI... I double checked Babylon.JS GLTF loader... They are pretty much doing the same thing:


        private _loadSpecularGlossinessPropertiesAsync(context: string, material: ILoaderMaterial, properties: IKHRMaterialsPbrSpecularGlossiness, babylonMaterial: Material): Promise<void> {
            if (!(babylonMaterial instanceof PBRMaterial)) {
                throw new Error(`${context}: Material type not supported`);
            }

            const promises = new Array<Promise<any>>();

            babylonMaterial.metallic = null;
            babylonMaterial.roughness = null;

            if (properties.diffuseFactor) {
                babylonMaterial.albedoColor = Color3.FromArray(properties.diffuseFactor);
                babylonMaterial.alpha = properties.diffuseFactor[3];
            }
            else {
                babylonMaterial.albedoColor = Color3.White();
            }

            babylonMaterial.reflectivityColor = properties.specularFactor ? Color3.FromArray(properties.specularFactor) : Color3.White();
            babylonMaterial.microSurface = properties.glossinessFactor == undefined ? 1 : properties.glossinessFactor;

            if (properties.diffuseTexture) {
                promises.push(this._loader.loadTextureInfoAsync(`${context}/diffuseTexture`, properties.diffuseTexture, texture => {
                    babylonMaterial.albedoTexture = texture;
                    return Promise.resolve();
                }));
            }

            if (properties.specularGlossinessTexture) {
                promises.push(this._loader.loadTextureInfoAsync(`${context}/specularGlossinessTexture`, properties.specularGlossinessTexture, texture => {
                    babylonMaterial.reflectivityTexture = texture;
                    return Promise.resolve();
                }));

                babylonMaterial.reflectivityTexture.hasAlpha = true;
                babylonMaterial.useMicroSurfaceFromReflectivityMapAlpha = true;
            }

            return Promise.all(promises).then(() => {});
        }

So you can see this line:

babylonMaterial.reflectivityColor = properties.specularFactor ? Color3.FromArray(properties.specularFactor) : Color3.White();

Does the same thing we are doing in PlayCanvas-GLTF.... So the problem MUST be something else.

MackeyK24 commented 6 years ago

I think it has to do with weather or not you have a specular glossiness texture or not. But i put the playcanvas-gltf default specularFactor back to white (1,1,1)...

If anything, we should just remove the ELSE PART and let it use the default. Try this instead:

if (specData.hasOwnProperty('specularFactor')) {
                color = specData.specularFactor;
                // Convert from linear space to sRGB space
                material.specular.set(Math.pow(color[0], 1 / 2.2), Math.pow(color[1], 1 / 2.2), Math.pow(color[2], 1 / 2.2));
                material.specularTint = true;
            }

Just at least to see what happens...

cx20 commented 6 years ago

@MackeyK24 I used the following test models and libraries. model: https://github.com/cx20/gltf-test/tree/master/tutorialModels/SpecGlossVsMetalRough/glTF library: https://github.com/cx20/gltf-test/tree/master/libs/playcanvas/v1.8.0-dev viewer: https://github.com/cx20/gltf-test/tree/master/examples/playcanvas Example: https://cx20.github.io/gltf-test/examples/playcanvas/index.html?category=tutorialModels&model=SpecGlossVsMetalRough&scale=10&type=glTF

image

cx20 commented 6 years ago

@MackeyK24 I tried it locally by changing to the following code, but the result did not seem to change.

if (specData.hasOwnProperty('specularFactor')) {
    color = specData.specularFactor;
    // Convert from linear space to sRGB space
    material.specular.set(Math.pow(color[0], 1 / 2.2), Math.pow(color[1], 1 / 2.2), Math.pow(color[2], 1 / 2.2));
    material.specularTint = true;
}
MackeyK24 commented 6 years ago

I have a question about material values that are in linear space.

When using PBR Specular the spec says the DiffuseFactor, SpecularFactor and GlossinessFactor are to be serialized with Linear space values... So it would be up to the implementation to convert those values if need be for that Engine.

But when looking at the regular PBE Metallic Roughness Spec it DOES NOT SAY specifically that any of those values are in linear space.

Does anyone know of the that PBR Metallic Roughness vales should be serialized in linear color space like PBR Specular Glossiness does... More specific... should Metalness and Roughness be in linear space... what about all the colors... should they be in linear space as well...

Seems funny that the specular would require linear space and the metallic roughness does not... But i cant tell... Any help would be awesome :)

cx20 commented 6 years ago

@MackeyK24 Unfortunately I am not familiar with the specifications of glTF, so I can not give technical advice.

@emackey Do you have any advice?

MackeyK24 commented 6 years ago

Also... I think we should put that default spec color in PlayCanvas-gltf.js back to white... 1,1,1

emackey commented 6 years ago

Does anyone know of the that PBR Metallic Roughness vales should be serialized in linear color space like PBR Specular Glossiness does... More specific... should Metalness and Roughness be in linear space... what about all the colors... should they be in linear space as well...

Yes, the "factors" are all in linear space. This is mentioned briefly in the spec for the Metallic-Roughness Material. It is then repeated for each of the factor specs: baseColorFactor, metallicFactor, and roughnessFactor each say:

This value is linear.

emackey commented 6 years ago

https://github.com/playcanvas/playcanvas-gltf/blob/1233416f665905fe6b11ebfb8b192662c6f6ad67/src/playcanvas-gltf.js#L353-L356

This looks problematic to me. My suggestion here is not a complete solution, but seems to make things better:

material.specularMap = material.glossMap = resources.textures[specularGlossinessTexture.index];
material.specularMapChannel = 'rgb';
material.glossMapChannel = 'a';

It still looks like there's a colorspace conversion gone wrong somewhere, in addition to this.

emackey commented 6 years ago

Here are the water bottles with labels on them: SpecGlossVsMetalRough.glb (via https://github.com/KhronosGroup/glTF-Sample-Models/pull/192)

Also that floor produces very different "background" colors behind the bottles. You should try taking the shots with a solid-color floor, otherwise the bottles may look different just on account of different amounts of contrast against the floor.

I don't have any contacts at PlayCanvas, but you could try opening an issue on their GitHub repo here, asking for a flag or setting to indicate that you're passing in an sRGB specular map.

MackeyK24 commented 6 years ago

Hey @emackey ... The reason the background plane with grid is on there. I am actually creating a set of PlayCanvas game development tools for Unity, called Canvas Tools.

What you are seeing is a TOTAL unity scene export from Unity to PlayCanvas.... Including:

1.. Background plane with grid the EXACT matching output. 2... Cameras... with EXACT placement and rotation 3... Lights... Including SKYBOX Image based lighting with Environment Maps

Then the two Water bottles i was using to WORK OUT the Color Space Rendering issues for Specular Glossiness.

Not a Sample SpecBottle vs MetalBottle... But more of a complete scene render that looks ALMOST identical to the original unity scene... with nothing more than press the EXPORT SCENE button.

Also supports things like:

Shadows Scripts Components (Rigid Body, Particle Systems, Etc...) Multiple Regular Translation Animations Export in GLTF Format Multiple Skeletal Animation Export in GLTF Format

And more to come...

Will making creating PlayCanvas Engine Only Games a BREEZE... Using all the power of Unity Editor for the actual scene layout and attaching script components etc.

MackeyK24 commented 6 years ago

Here is a link to draft of my GLTF CVTOOLS_unity_metadata extension is created... Check it out :)

MackeyK24 commented 6 years ago

I got the PBR Specular Fix as well... is PR #31 no need to try add a property to indicate the GLTF specular textures.

I finally figured out HOW FREAKING EASY is to replace a shader chunk to handle the issue.. Works Like a charm.

https://github.com/playcanvas/playcanvas-gltf/pull/31/commits/243cd9943f7a73ad41f0386406c864dc3f45b63e

MackeyK24 commented 6 years ago

Using the Specular Shader Chunk from PR #31

Note Left bottle PBR Metallic Roughness and Right Bottle Specular Glossiness...Sorry i dont have Fancy labels above each bottle :)

screen shot 2018-09-25 at 8 19 28 pm

cx20 commented 6 years ago

@MackeyK24 I tried the latest version, but it seems the same result https://github.com/playcanvas/playcanvas-gltf/issues/27#issuecomment-421521351 as before. Is it a usage problem?

emackey commented 6 years ago

@MackeyK24 That export process all sounds very cool! My main interest in commenting here was to try to help (if I can) the PlayCanvas glTF loader (and other glTF loaders) to be compatible with the KHR_materials_pbrSpecularGlossiness extension. It seems that particular extension is in need of improved support across much of the glTF ecosystem. It's known to work in BabylonJS, and it should begin working in next month's release of Cesium, but several other frameworks are still struggling to support it fully.

MackeyK24 commented 6 years ago

Yo @cx20 ... can you please make a small web project with the model you are loading and the PlayCanvas-gltf.js file you are using ... I will try to play with exact project setup you have... just zip up a folder so I can just run http-server to test

cx20 commented 6 years ago

@MackeyK24 I have created a small project. http://jsdo.it/cx20/Q3iT http://jsdo.it/cx20/Q3iT/download ... zip

MackeyK24 commented 6 years ago

I found your problem... @emackey gave details about this part. But basically we need to RGB channels of the specularGlossinesTexture as the PlayCanvas SpecularMap (RGB) and we need to map the ALPHA from the specularGlossinesTexture to the PlayCanvas GlossMap (A)

I made a PR #35 to fix the issues... I tried your sample and just replaced the playcanvas-gltf.js with mine... It works Great :)

MackeyK24 commented 6 years ago

Yo @willeastcott can you please check out PR #35 as soon as you can for @cx20

Thanks Bro :)

MackeyK24 commented 6 years ago

Yo @cx20 ... After the PR #35 goes thru... please try again and let me know what happens :)

willeastcott commented 6 years ago

Done!

cx20 commented 6 years ago

I updated the library and confirmed that the problem was solved. https://cdn.rawgit.com/cx20/gltf-test/bfffcbe783e9c1f195f607e4191759edd4c3eae4/examples/playcanvas/index.html?category=tutorialModels&model=SpecGlossVsMetalRough&scale=10&type=glTF image

MackeyK24 commented 6 years ago

Very Kool :)