KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.17k stars 1.14k forks source link

glTF 2.0: New/modified KHR_materials_common extension #947

Closed McNopper closed 7 years ago

McNopper commented 7 years ago

Using the pbrMetallicRoughness and pbrSpecularGlossiness material "syntax", I have adapted the common materials. Again here by example:

{
  "materials": [
    {
      "commonConstant": {
        "ambientFactor": [
          0.2,
          0.2,
          0.2
        ]
      }
    },
    {
      "commonLambert": {
        "ambientFactor": [
          0.2,
          0.2,
          0.2
        ],
        "diffuseFactor": [
          1,
          1,
          1,
          1
        ],
        "diffuseTexture": 0
      }
    },
    {
      "commonPhong": {
        "ambientFactor": [
          0.2,
          0.2,
          0.2
        ],
        "diffuseFactor": [
          1,
          1,
          1,
          1
        ],
        "diffuseTexture": 0,
        "specularFactor": [
          1,
          1,
          1,
          1
        ],
        "specularTexture": 1,
        "shininessFactor": 0.5,
        "shininessTexture": 2
      }
    },
    {
      "commonBlinn": {
        "ambientFactor": [
          0.2,
          0.2,
          0.2
        ],
        "diffuseFactor": [
          1,
          1,
          1,
          1
        ],
        "diffuseTexture": 0,
        "specularFactor": [
          1,
          1,
          1,
          1
        ],
        "specularTexture": 1,
        "shininessFactor": 0.5,
        "shininessTexture": 2
      }
    }
  ]
}

Like the current PBR materials, they extend the "base" materials. So e.g. emissive would be inherited.

andreasplesch commented 7 years ago

@McNopper : see https://github.com/UX3D-nopper/glTF/pull/1 . By official repo, did you mean this repo here ?

McNopper commented 7 years ago

Official repo is this one: https://github.com/KhronosGroup/glTF/

Process is like this: You make a fork, make changes and then do a pull request to the official, original one. If the changes are merged, the spec will be reviewed for ratification.

stevepg commented 7 years ago

not sure i've seen this discussed in any thread, but is the 'technique' really needed for materials_common? With the correct default values, blinn, constant and lambert are all achievable by simply providing the appropriate values e.g. if you want 'constant' (baked) material effect, simply provide the emissiveFactor/texture values ; the other terms will multiply out to 0.

McNopper commented 7 years ago

Current work in progress: https://github.com/UX3D-nopper/glTF/tree/master_lights_blinnphong/extensions/Khronos/KHR_materials_cmnBlinnPhong

steveghee commented 7 years ago

@McNopper - do you want us to fork this one or fork the master?

McNopper commented 7 years ago

Please work on this one, as far as I know this is the latest one. Like the light extension, of course still some stuff to specify.

andreasplesch commented 7 years ago

https://github.com/KhronosGroup/glTF/pull/1075 is https://github.com/UX3D-nopper/glTF/pull/1 against this repo.

stevepg commented 7 years ago

there appear t be two KHR_common material definitions ongoing.
we have a working exporter but need to know which format to generate. Can i just double check - is this the latest : https://github.com/UX3D-nopper/glTF/tree/master_lights_blinnphong/extensions/Khronos/KHR_materials_cmnBlinnPhong

donmccurdy commented 7 years ago

@stevepg the page you link to is the latest, yes. Viewers do not support this yet, and some spec discussion is ongoing, so it may be a bit early to implement that extension yet.

donmccurdy commented 7 years ago

We currently have:

@McNopper Any objection to closing this issue, to avoid confusion?

stevepg commented 7 years ago

@donmccurdy i have my own viewer and i need to get test data into it asap so i might need to punt on one of these. Personally i think cmnBlinnPhong is good enough for what we need - i think its better to have the material typed like this, instead of a generic _common with a type=blabla field.

I don't see much discussion on the specs to be honest ; there were questions a few weeks back, folks asked for input, there are pull requests waiting with that feedback.

donmccurdy commented 7 years ago

Oops, #1075 is the one I meant to reference. There are more recent comments there. What I mean to say is that it's not necessarily decided yet whether KHR_materials_cmnBlinnPhong or KHR_materials_common will be ratified, more likely the first after some further changes, but it will only be one or the other.

I would advise against putting either into a publicly-available exporter without at least an "experimental" option to enable/disable the extension, to avoid circulation of invalid models. The Blender exporter does something along these lines now.

stevepg commented 7 years ago

this is internal so i'm not worried about models getting out there. not yet. thanks for the pointer to the other thread. i'll head over there....

McNopper commented 7 years ago

Closing, as discussed in several threads.

chipweinberger commented 6 years ago

What is the current state of this extension? Which viewers support it and what format are they using? Thanks!

donmccurdy commented 6 years ago

I know you saw this already @chipweinberger, but copying zellski's response (https://github.com/KhronosGroup/glTF/issues/1207#issuecomment-356788223) here for future readers:

There's been copious discussion about what to replace KHR_common_materials with for glTF 2.0, e.g. #947 and #1095, culminating most immediately in #1163.

For lights, specifically, there's #945