jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.82k stars 1.12k forks source link

Texture Channel Packing #999

Open MeFisto94 opened 5 years ago

MeFisto94 commented 5 years ago

So I've intentionally kept this description a bit more broad, since there is discussion going on here

In the unreal documentation they outline their format as:
R = Metallic
G = Roughness
B = Ambient Occlusion

And in another UE article it’s:
R = Ambient Occlusion
G = Metallic
B = Roughness

Whereas ours is:
G = Roughness
B = Metallic

See: https://github.com/jMonkeyEngine/jmonkeyengine/blob/d57c362ec3b510c1ba6356f719efa3b1576b95c6/jme3-core/src/main/resources/Common/MatDefs/Light/PBRLighting.j3md#L30

GLTF Spec also uses "our" format and yet that leads to problems when trying to export a GLTF Project from blender instead of hand crafting Textures to our format. See https://hub.jmonkeyengine.org/t/environment-cam-and-light-probe-question/41241

Under Texture Requirements they also pack Ambient Occlusion into the R channel, which we don't do yet, opinions?

Maybe the GLTF Exporter is broken or our importer is broken? It definitely looks like Roughness and Metallicness is swapped somewhere for users importing from blender.

I didn't want this issue to end like this, I was expecting the GLTF Spec to say something different (namely the Unreal Definition 1), we should still keep track of this in an issue. I'll report back if I find more information

MeFisto94 commented 5 years ago

As expected: https://github.com/KhronosGroup/glTF-Blender-Exporter/blob/7933fbfdd80628875910c5da36fd28e40474fb43/scripts/addons/io_scene_gltf2/gltf2_generate.py#L2292

They talk about a MetallicRoughness Texture here and even the spec talks about a MetallicRoughness Texture even though it's packed as Roughness Metallic, not sure if they are hence only confusing the users to create the texture or if they bake textures from channel inputs manually, in which case the exporter is broken.

Edit: Okay so the situation is as follows:

  1. GLTF uses AO, Roughness, Metallicness as their packing format
  2. The Industry uses/used AO, Metallicness, Roughness before (see here section Substance Painter, there are no templates for GLTF)
  3. When I look at images like this it looks like even the "GLTF" Node of the Blender Addon uses 1 and not 2. When exporting, it should convert the textures, but it doesn't. Or it shouldn't be as misleading and call that texture RoughnessMetallicness

So at this point I have to leverage @pspeed42 @Nehon and other knowledgeable monkeys on what to do now:

It's easy to add a material parameter "UseMetallicnessRoughness", but we can't automatically set this for all GLTF files, only for the broken ones of Blender this would be correct. We should definitely talk to the blender exporter guys about this. Or do you think this is worth pointing out to Khronos that the world uses MR instead of RM?

Ali-RS commented 5 years ago

Maybe the GLTF Exporter is broken or our importer is broken? It definitely looks like Roughness and Metallicness is swapped somewhere for users importing from blender.

At least I know it is broken in their new glTF-Blender-IO add on. https://github.com/KhronosGroup/glTF-Blender-IO/issues/200

donmccurdy commented 5 years ago

Per https://support.allegorithmic.com/documentation/spdoc/unreal-engine-4-144998459.html, I was under the impression UE4 used occlusion/roughness/metallic as R/G/B? Same applies to three.js, and recent versions of Substance Painter can export directly to glTF. But I think you're correct that there is a bug in the Blender Exporter.

riccardobl commented 4 years ago

So, it seems this can be closed now.

yaRnMcDonuts commented 4 years ago

I did notice one more thing with JME's .gltf>j3o converter

If a gltf model has its AO value packed in the MR map, it looks like the JME converter currently just sets 'LightMapAsAOMap' to true and also defines the MetallicRoughness map as the light map for the auto-generated material.

So we may now also want to update the gltf > j3o converter to just flag the new boolean 'AoPackedInMRMap' to true instead.

riccardobl commented 4 years ago

Can you open a new issue for this?

MeFisto94 commented 4 years ago

FYI I was mostly referring to what indeed was a bug in the blender exporter at that time: Metallic and Roughness were swapped. But at the time of opening the issue it was (at first) not clear. I am uncertain whether we should open a new issue or not, because apparently the new issue is like a regression of #1403 then or should've been included there to support that feature.

So if you plan to do that small change in the gltf importer soon, we don't need a new issue and can track here.

yaRnMcDonuts commented 4 years ago

I tried to look at the code for the JME gltf converter, but could not figure out where the code lies that needs changed. I will go ahead and make a new issue and hopefully someone else who is more knowledgeable with the GLTF converter can figure it out or point me in the proper direction.