Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update KHR_materials_common for glTF 2.0 #1150

Conversation

donmccurdy
Copy link
Contributor

This PR includes changes to the draft KHR_materials_cmnBlinnPhong material from previous discussions, renaming it (back) to KHR_materials_common and including an explicit technique parameter to specify unlit or Lambert shading models.

Compared to the original KHR_materials_common extension from glTF 1.0, changes are limited but notable:

  1. Removed values wrapper around technique properties.
  2. Property names to use fooTexture / fooFactor nomenclature, for consistency with glTF 2.0.
  3. Merged blinn and phong techniques.
  4. Removed ambient term from all techniques.

A few particular areas I would like to call out for feedback:

  1. Can this extension still be called KHR_materials_common, or do we need to distinguish from the glTF 1.0 version?
  2. I am not confident that I understand the lighting equations for each technique; I adapted each from an older version by removing unused terms. Sanity-checking here would be appreciated.

/cc @McNopper @robertlong @zellski @msfeldstein @sbtron

@msfeldstein
Copy link

msfeldstein commented Nov 12, 2017

Very excited to see this move forward. My only concern is the question of can a client implement only part of an extension? There was some talk on twitter of making Unlit(constant) it's own extension. Our renderer currently supports only the constant technique of this extension and it's unclear if we want to support the rest, at least in the immediate future.

Some arguments made were that blinnphong/lambert can be closely emulated using PBR, and that if performance is the concern that should be up to the client to implement simpler shaders, not the model to specify.

Other arguments were that we should limit the number of extensions, keeping this all as one unit. I just worry that we will lose Unlit shading support in engines that don't want to deal with the complexity of 2 other new shading models.

@UX3D-nopper
Copy link
Contributor

Looks good and I think we should express, that a default/empty material is a pure black.

@UX3D-nopper
Copy link
Contributor

Think we should keep the extension name like this:

  • Name logic like the pbr ones
  • Maybe someone needs a "pure" KHR_materials_cmnPhong material

@zellski
Copy link
Contributor

zellski commented Nov 12, 2017

After some thought this morning, I remain a fan of this approach; the single extension. I am really worried about a proliferation of highly specific extensions. If there is one thing I want to feel when I see a .glb file in the wild, it is a reasonable assumption that it'll load everywhere. I realize the whole point of extensions is to fragment the format, but surely it's a power to be used with restraint.

I would rather see engines accept this one extension's effort to gather historic shader models, and do their best to accomodate what it expresses, than to burden authoring tools, converters, viewers, and ultimately end users, with the task of figuring out the "market penetration" of different but related extensions and guessing which -- or even which combination-- might yield which result where.

So @msfeldstein I think I would say, in the (surely fairly unusual) case of a renderer that can do unlit and PBR only, it's preferable to accept the extension and make a best effort than it is to give up, or for us to fragment the glTF landscape. In this context it seems to me that the argument in favour of an unlit-only extension boils down to an urge to discincentivize the use of lambert/phong. I think that's unnecessary. The future belongs to PBR; we don't need to carefully nurture that evolution. This extension exists to bring historic representations with us as we move on. Most engines will be able to render such models quite precisely; for those that can't or are not interested, it's still a lot better to get 90% of the way there.

@msfeldstein
Copy link

msfeldstein commented Nov 12, 2017

So one of the points brought up was the idea that you could approximate these shading models via PBR properties. What are the benefits of having phong specified in an extension, vs just authoring a PBR material that looks like phong?

Sorry if this has been brought up a bunch

@msfeldstein
Copy link

But yeah the more that i think about it keeping it all in one extension is best.

@Kupoman
Copy link
Contributor

Kupoman commented Nov 12, 2017

@msfeldstein Not all authoring tools support PBR well yet. In those cases I can see it being useful to export this extension but allow an engine to approximate the rendering with PBR.

+1 to a single extension covering multiple related models.

<summary>Example: Lambert</summary>
The following example defines a Lambert shaded material with a diffuse texture,
moderate shininess and red specular highlights.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a copy-and-paste error. Maybe just "The following example defines a Lambert shaded material with a diffuse texture." or some-such?

glTF material. When the extension is provided alongside a core glTF material
or another material extension, client implementations may choose the most
appropriate material for performance or other constraints.

Copy link
Contributor

@zellski zellski Nov 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been wondering if we should attempt to guide the way in which clients render situations where multiple material extensions are present, or this extension along with pbrMetallicRoughness. From one point of view, a filled-out PBR struct would always be primary, and e.g. BLINNPHONG might be sent along as a fallback for weak devices. From another point of view, a LAMBERT material would be the model creator's ideal aesthetic, but they also fill out the PBR struct with (very) approximate values for clients that don't accept the extension. Does each client guess? Do we guide the guesswork? Do we include a boolean flag or some-such in this extension to help suggest "this is a fallback" vs "this is the ideal rendering path"? (I think that's probably not a good idea, but who knows.)

color is calculated as:

```
color = emissiveTerm + occlusionTerm * diffuseTerm * max(N * L, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ambient is now calculated from diffuse, it should probably be:

color = emissiveTerm + occlusionTerm * diffuseTerm * (aL + max(N * L, 0))

The following code illustrates the computation:

```
color = emissiveTerm + occlusionTerm * (diffuseTerm * max(N * L, 0) + specularTerm * max(H * N, 0)^shineTerm)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modulo code style urges, with aL factor:

color = emissiveTerm +
    occlusionTerm * (
        diffuseTerm * (aL + max(N * L, 0)) +
        specularTerm * max(H * N, 0)^shineTerm)

Copy link
Contributor

@zellski zellski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sprinkled some random thoughts here and there. Generally very happy with where this is headed.

The displayed color is calculated as:

```
color = emissiveTerm + occlusionTerm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the math here is incorrect; I don't think the occlusionTerm should be added by itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

color = emissiveTerm + occlusionTerm * aL

Does that look correct? I'm probably not the person to be writing these equations.

@snagy
Copy link
Contributor

snagy commented Nov 13, 2017

Hi folks,

Here are some higher level thoughts on this extension; these aren't related to the quality of the extension, but really whether adding this to core is in glTF's long term interest.

I'm worried that this extension will pollute the glTF ecosystem with content that doesn't make coherent sense with other glTF content.

One of the goals of the glTF PBR materials was to enable different content to have a consistent set of parameters for the engines to interpret as appropriate. This enables predictable fallback to simpler lighting models (you can interpret existing glTF content easily in a blinn-phong or lambertian model: https://snagy.github.io/glTF-WebGL-PBR/ has quick examples of this). It also enables engines to push the envelope and start making more complex lighting models; there are path tracers which support glTF and can enable real reflections, bounce lighting, and true emissive surfaces using the content that's in the core spec. What does an unlit material mean in that context? What about a Lambertian material?

If we splinter the lighting model into multiple content-driven models, I think we may lose a lot of the flexibility for transference into different applications that glTF files currently have.

If the concern is that authoring tools don't uniformly support PBR today, the solution isn't to require the loading engines to do a runtime execution or conversion of other lighting models in perpetuity, it's to provide clear guidelines for exporters who would be implementing this extension to instead convert their values into PBR materials.

The big question for me is what the ultimate goal of defining a lighting model on a glTF file is; should the content define the lighting model to use across any engine that loads it, or is the engine expected to be interpreting content into its model? If it's the former, is it the content exporter's duty to populate coherent MR fallback values in engines that don't support this extension? Is it assumed that the content is always using alternate models as an artistic choice?

@donmccurdy
Copy link
Contributor Author

donmccurdy commented Nov 13, 2017

whether adding this to core is in glTF's long term interest.

Quick distinction — this is not intended to be part of the core glTF specification: it's an extension,
and (as with all extensions) we would encourage authoring tools to provide fallbacks rather than requiring support for any given extension.

We should also be clear that if a client cannot do something reasonable with the core PBR materials, it is nonconformant. Supporting extra extensions like this is optional, and not a replacement for core PBR.

If the concern is that authoring tools don't uniformly support PBR today...

This is not the concern at all, from my perspective. It is that certain environments (say, VR on mobile) are heavily resource-constrained, and unlit materials are a common need. It's possible to specify something that looks like an unlit material by zero-ing out unused parameters in a PBR model, but it is hard for the engine to optimize without a clear indicator that lighting is not wanted. My goal would be some way of authoring an asset that says, "this thing is unlit please optimize accordingly."

Here's an example from Sketchfab, previewed in my viewer:

preview as-is desired preview
screen shot 2017-11-13 at 1 08 47 pm screen shot 2017-11-13 at 1 08 26 pm

^That model isn't especially mobile-friendly, but it's still a clear case where the flexibility to treat a material as PBR is, in fact, undesirable. Arguably the Sketchfab file should have used the emissive channel and zeroed out diffuse, but all the same most runtimes are going to use a complex shader because the model claims to require a spec/gloss shading model.

As for Lambert and Blinn-Phong, I have little preference. We hope to avoid publishing three different material extensions (per discussion above) and so in my mind Lambert and Blinn-Phong are just yak-shaving in order to make unlit materials available. A KHR_materials_cmnConstant extension would be just fine for my needs, if we don't need the other two. Thoughts?

Think we should keep the extension name like this:

  • Name logic like the pbr ones
  • Maybe someone needs a "pure" KHR_materials_cmnPhong material

@UX3D-nopper I'm worried about the situation where the "correct" way of specifying an unlit material becomes extensions: { KHR_materials_cmnPhong: {technique: "CONSTANT" } }. This feels indirect, and from an authoring tool's perspective isn't obviously better than just writing metal/rough or spec/gloss PBR with only emissive. As described above, I actually see unlit materials as the critical purpose for this extension.

@donmccurdy
Copy link
Contributor Author

also, thanks for all of the inline comments! will get to those after letting discussion continue a bit on naming, scope, and long-term value.

@zellski
Copy link
Contributor

zellski commented Nov 13, 2017

In addition to Don's point... perhaps I'm biased from working a lot lately on this FBX to glTF converter, with its emphasis on older content, and perhaps this is a niche concern regardless -- but to me one of the purposes of this extension is the graceful representation, going forward, of pre-existing art. Needless to say, you can't just take a model with carefully constructed diffuse textures, slam those into baseColor and call it PBR. A converter cannot scrub the original artist's baked-in representations of roughness, glossiness, general brightness level, and other various lighting effects, from the textures. Perhaps in some cases there's source files that would let you re-render textures via a PBR workflow, but certainly not always. There's a lot of wonderful models out there, that were authored for a specific shading technique, and while it's entirely fine that those can't be represented in core glTF 2.0, this is precisely the extension I was hoping would allow them to be viewed as intended. It feels like deliberately circumscribing its scope to where that's not possible has little benefit and major cost.

@msfeldstein
Copy link

So i think that's what bothers me here... it seems like we're mixing backwards compatibility (phong/lambert) with needed functionality (unlit).

@msfeldstein
Copy link

I'm also worried that us offering this extension will let editors/exporters think they don't actually need to support full pbr and slow down uptake.

@donmccurdy
Copy link
Contributor Author

So i think that's what bothers me here... it seems like we're mixing backwards compatibility (phong/lambert) with needed functionality (unlit).

I would personally be OK With dropping lambert/phong, with the expectation that there probably will not be a KHR_ extension for either in the future — vendor and EXT_ extensions are of course entirely possible — and just doing KHR_materials_cmnConstant. One upside of this, we would not need two separate extensions for KHR_lights and KHR_lights_pbr (which has not firmly been decided, but we'd been leaning that way). But I would hope to get more feedback before making the decision to drop Phong and Lambert.

An alternative would be "demoting" this to a multi-vendor prefix, like EXT_materials_constant or EXT_materials_common, to discourage exporters/editors from treating it as a default.

@snagy
Copy link
Contributor

snagy commented Nov 13, 2017

Quick distinction — this is not intended to be part of the core glTF specification

Ahh, yeah, I misspoke and meant to say I'm concerned about making this a KHR extension, because of the percieved authority that distinction brings.

I'm ok with having this functionality exist as an extension, but it's important that content that's created with these materials also have a meaningful set of MR properties.

It feels like deliberately circumscribing its scope to where that's not possible has little benefit and major cost.

What do you mean by this, Per?

@donmccurdy
Copy link
Contributor Author

...little benefit and major cost.

What do you mean by this, Per?

I think we're considering different angles here, which is good — for assets circulating in places like Sketchfab and Google Poly, we'd generally prefer they use core PBR materials or have PBR fallbacks for maximum compatibility. But for content authors using glTF as their last mile delivery format for a runtime application, writing PBR fallbacks into assets intended only to be rendered unlit is an unwanted requirement at best, and may hurt performance at worst. Mobile-friendly materials should not need to be hacked into a runtime format, one way or another.

I'm ok with having this functionality exist as an extension, but it's important that content that's created with these materials also have a meaningful set of MR properties.

We've attempted to steer the ecosystem this way before, by making sure it was possible to include both metal/rough and spec/gloss PBR materials in a single asset. So far, I'm not sure that has been successful — Sketchfab appears to exclusively generate models with the spec/gloss extension and not core metal/rough, for example. I'm not sure what MSFT tools are doing currently but I think it is the same.

That being the case, any suggestions on how to phrase this or make it easier? For unlit, the fallback is practically built in, and we could ask (require? strongly advise?) authoring tools to zero out unused parameters of the core PBR material and mark the constant material as optional:

"materials": [
    {
        "name": "unlit_with_fallback",
        "emissiveFactor": [ 1.0, 0.0, 0.0 ],
        "emissiveTexture": { "index": 0 },
        "baseColorFactor": [ 0.0, 0.0, 0.0, 1.0 ],
        "metallicFactor": 0.0,
        "roughnessFactor": 0.0,
        "extensions": {
            "KHR_materials_common": {
                "technique": "CONSTANT"
            }
        }
    }
]

^This seems reasonable enough. The result should look correct whether the extension is supported or not. Phong is more trouble: .glb assets must now be shipped with both specular and metal/rough textures, adding filesize that won't be used.

@mlimper
Copy link
Contributor

mlimper commented Nov 16, 2017

Quick question, why are there different techniques for Blinn-Phong and Lambert? The proposal says

Lambert shading does not reflect specular highlights; therefore the common material properties specular and shininess are not used. All other properties from Table 1 apply.

Wouldn't then the result be identical if one simulates the Lambert technique by using Blinn-Phong with zero specular? Just curious, if this is really the case we could further simplify the extension.

@javagl
Copy link
Contributor

javagl commented Nov 16, 2017

@mlimper This sounds similar to the question at #947 (comment) (and there is some extensive discussion about this).

IIRC, the main argument eventually was this: If the type of the material is known, then implementations may choose a more efficient shader implementation. If the shader implementation only depends on the values of certain properties, then an implementation could be tempted to use a lambert shader for a material with specular==0, but then it would no longer be possible to set a value !=0 for the specular term at runtime (e.g. in some "material property editor window"). A more direct matching between the material type and the required shader implementation may avoid some glitches here.

(Feel free to correct or extend this statement if it's not correct - I haven't been able to closely track this PR and the related issues recently)

@mlimper
Copy link
Contributor

mlimper commented Nov 16, 2017

Thanks for the wrap-up! I was actually suspecting something like that - one could probably argue that the optimized client implementation could then as well select the more advanced shader lazily as soon as a value != 0 is selected in the UI, etc. ... I'd personally prefer to have a more simple format where possible, but that's just my two cents, don't want to open up another can of worms here.

Thanks to all the people working on this proposal, it's cool to see it getting in shape!

@snagy
Copy link
Contributor

snagy commented Nov 17, 2017

I think the consensus at the call on Thursday was to split Unlit into a separate extension (using the implementation from the unlit section of this extension). Anyone have any concerns about that?

WRT the remaining Phong and Lambert models, I think the big question for me is what we intend this extension to mean.

  • Is this extension an indicator that content that was not authored with a PBR workflow?
  • Or is this extension an indicator for desired visual look of the asset?

Basically, if I have an engine where I want to implement this extension, is the suggestion that I add Phong and Lambert lighting models and drop the content using them into my engine in those models, or is it a hint on how I should convert the phong surface properties into my PBR lighting model?

@zellski
Copy link
Contributor

zellski commented Nov 18, 2017

Consensus and momentum are more important than continued finessing, so by all means let's move forward. I'm still worried that @donmccurdy is right; that we only realistically get one shot at a KHR extension. But maybe that's OK; maybe labelling Phong/Lambert as EXT is the correct degree of deemphasis.

An upside is that it clarifies the purpose of the second extension. I think the dichotomy in your question is not quite right. I think every engine would do well to accept this extension and make a best effort. Penetration is important; nobody likes an error dialogue. And yet I would say that (unless we introduce further flags) the presence of the extension in a material should express explicit intent: this should be rendered with the simpler shader type if at all possible... it could be because it's old work, e.g. with a set of textures that were baked in pre-PBR era and between them contain many artifacts to simulate dynamic lighting, such that diffuse slammed into baseColor will be simply incorrect... or when it's new work, and the artist specifically wants to build a world with that canonical Lambert/Gouraud look, completely free of specularity, and wants the glTF to explicitly signal this intent.

@donmccurdy
Copy link
Contributor Author

Basically, if I have an engine where I want to implement this extension, is the suggestion that I add Phong and Lambert lighting models and drop the content using them into my engine in those models, or is it a hint on how I should convert the phong surface properties into my PBR lighting model?

My main interest is in the unlit extension, so I'd rather defer to others on this question, but to me that second scenario seems like something to avoid. If we expect that phong models brought into the glTF ecosystem should always (or almost always) be converted to PBR, then it would be better to put the burden of conversion onto the export tooling and ship models with the core PBR materials.

...the presence of the extension in a material should express explicit intent: this should be rendered with the simpler shader type if at all possible...

+1, I think that would be the point of a Phong/Lambert extension. At least three reasons to want that behavior:

  1. Preserve pre-PBR work as authored
  2. Use a pre-PBR look for stylistic reasons
  3. Use simpler shading model for performance

Of those, (3) is the case I'm really confident is important, and the unlit extension should address that well. Here's a previous attempt at a KHR_materials_cmnConstant extension for that purpose: #1095. I'll re-open that or something similar soon.

(1) and (2) are more in line with the phong/lambert extension. They seem like reasonable ideas, but aren't solving problems that I personally have... does anyone have strong opinions about phong/lambert and want to push it forward? Or should we just open the unlit extension for feedback, and leave phong/lambert for another time when/if it is needed?

@donmccurdy donmccurdy mentioned this pull request Nov 20, 2017
87 tasks
@robertlong
Copy link
Contributor

robertlong commented Nov 20, 2017

My main interest is in the unlit extension, so I'd rather defer to others on this question, but to me that second scenario seems like something to avoid. If we expect that phong models brought into the glTF ecosystem should always (or almost always) be converted to PBR, then it would be better to put the burden of conversion onto the export tooling and ship models with the core PBR materials.

I'm also primarily interested in the unlit extension for mobile VR performance and would like to push that extension forward separately. It seems fairly straight forward seeing as it has a simple fallback strategy and no additional schema.

The phong/lambert extension seems like it needs more work and I'm still not sure if we should do it. If engines need to implement the phong extension to accurately render a model, that worries me. This could fragment the glTF ecosystem at an early stage. If we can convert phong models to PBR that would be ideal. Otherwise we should make fallback PBR material values required in the extension.

@robertlong
Copy link
Contributor

Here's the start of an implementation of the unlit khr_materials_cmnConstant extension in ThreeJS and FBX2glTF.

@zellski
Copy link
Contributor

zellski commented Nov 22, 2017

Nice work @robertlong! Feel free to open a PR for that, I don't mind supporting under-discussion extensions, as long as we note they're a bit wobbly and are for immediate consumption, not future-proof.

@zellski
Copy link
Contributor

zellski commented Nov 22, 2017

On the question of fallback, @donmccurdy suggested:

"materials": [
    {
        "name": "unlit_with_fallback",
        "emissiveFactor": [ 1.0, 0.0, 0.0 ],
        "emissiveTexture": { "index": 0 },
        "baseColorFactor": [ 0.0, 0.0, 0.0, 1.0 ],
        "metallicFactor": 0.0,
        "roughnessFactor": 0.0,
        "extensions": {
            "KHR_materials_common": {
                "technique": "CONSTANT"
            }
        }
    }
]

This should work well in the presence of a constant emissive or an emissive texture, but I don't see it working for vertex coloured models (which are likely to be common in the domain of the unlit). If a loader/viewer is aware of the cmmConstant extension, they will know to switch to an unlit, vertex-colour-aware shader, and all will be well. But in a PBR environmemt, the influence of vertex colour is going to be in the realm that's been blacked out by that baseColorFactor (0, 0, 0, 1) and as far as I know we have no tools to express something like "vertex colour is emissive now"?

Just to be super-obvious about what I'm saying; here's the vertex coloured version of our in-house test van, with baseColorFactor channel R artificially set to zero. On the left, three.js using the unlit ("Base") material. On the right, with its "Standard" PBR material.
image

Hopefully I'm just missing something, otherwise I'm not sure how to proceed with the idea of generating a useful fallback...

zellski referenced this pull request in facebookincubator/FBX2glTF Nov 22, 2017
@zellski
Copy link
Contributor

zellski commented Nov 22, 2017

Finally, I've decided not to take on the job of pushing for a Lambert/Phong extension. Clearly it makes a bunch of people nervous, and furthermore I've chatted to a bunch of the artists I work with, who seemed to all feel that if it's important to someone to acquire a glTF export of a model, then that someone really ought to have the original source material -- or, failing that, at least the interest and ability to manually do the necessary cleanup work on old textures.

Furthermore, I finally realized that (rather obvious in retrospect) what makes old work look wrong in a modern environment isn't so much a matter of the shader, but of context: if everything else uses dynamic lights and shadow, then dropping an old model with baked shadows and global illumination effects into that scene would likely look quite surreal, no matter how the materials were treated. In this situation, if the model is to be redeemed, it really must be subjected to something akin to this painstaking process. No glTF extension will make the difference.

To continue my new habit of side-by-side images to illustrate trivial points:
blacksmith-lambert
blacksmith-pbr

This is Sketchfab's basic renderer and their PBR renderer (which is I think a superset of what glTF can express). It's clear that even with pure diffuse, this model would stick out like a sore thumb in any kind of subtler environment; there's tons of emissive-style (not actually an emissive texture) glow baked into the diffuse, there's shadows from one specific direction, and so forth.

So, with this weakened case for Lambert/Phong playing an important role in rehabilitating old work for a new future, I'm content with the unlit extension for now.

@zellski
Copy link
Contributor

zellski commented Nov 24, 2017

But in a PBR environmemt, the influence of vertex colour is going to be in the realm that's been blacked out by that baseColorFactor (0, 0, 0, 1) and as far as I know we have no tools to express something like "vertex colour is emissive now"?

I can't figure out a solution to vertex colours in a PBR fallback situation. We could set baseColorFactor to (1, 1, 1, 1), and just make sure not to output a baseColorTexture, and that would help some? But certainly it's not a real fallback in the sense of being "immune" to lighting conditions...

@donmccurdy
Copy link
Contributor Author

@zellski Here's an alternative — what if the KHR_materials_cmnConstant extension relies directly on baseColorFactor and baseColorTexture, ignoring emissive?

(A) Use baseColor as input to unlit material.

"materials": [
    {
        "name": "red_constant_material",
        "baseColorFactor": [ 1.0, 0.0, 0.0 ],
        "extensions": {
            "KHR_materials_cmnConstant": {}
        }
    }
]

That has major implications for fallback behavior – namely, if the extension isn't supported, the material will be lit. But then the vertex color fallback makes sense, and perhaps this addresses a concern @snagy mentioned on twitter:

I would rather have the model have coherent MR properties that let me bring it into different engines and light properly instead of dictating that it doesn't take additional lighting

Another option would be putting the unlit material's properties inside of the extension, rather than treating the extension as a boolean flag, and leaving authoring tools to define the fallback behavior as they see fit:

(B) Re-define emissive within the extension.

"materials": [
    {
        "name": "red_constant_material_with_fallback_to_diffuse",
        "emissiveFactor": [ 0.0, 0.0, 0.0 ],
        "baseColorFactor": [ 1.0, 0.0, 0.0 ],
        "extensions": {
            "KHR_materials_cmnConstant": {
              "emissiveFactor": [ 1.0, 0.0, 0.0 ],
             }
        }
    }
]

@donmccurdy donmccurdy mentioned this pull request Nov 24, 2017
@donmccurdy
Copy link
Contributor Author

PR for dedicated unlit extension: #1163

@andreasplesch
Copy link
Contributor

What happened to the goal of glTF being simply a compact transmission format and not concerned about a preference for a material/lighting model ? Perhaps in 2019 Metallic-Roughness will be the Phong of 2017 ? Tying itself to PBR feels unneccessary and perhaps a bit short-sighted. This was likely discussed at length, is there somewhere a summary of the motivation ?

@mlimper
Copy link
Contributor

mlimper commented Nov 28, 2017

@andreasplesch
The motivation behind our initial proposal was to achieve a consistent rendering across different platforms and, at the same time, to simplify things to the most commonly used parameters where possible. The glTF 1.0 / Collada approach of specifying techniques with (GLSL) shader code doesn't allow to use the same material in different environments without further efforts. With the more abstract PBR parameters, the same model can be rendered consistently in WebGL, DirectX and Vulkan renderers, for example.

With that said, glTF 2.0 is not fully backwards compatible with glTF 1.0, and following this path would allow future versions to break with the PBR model, if necessary, and then introduce a more state-of-the art concept whenever it's needed. This is a very pragmatic approach - but finding an expressive material description that works well with all kinds of renderers and doesn't get outdated for the next ten years may not be possible.

P.S.: Here's a dedicated thread where this topic was discussed as well: #746

@donmccurdy
Copy link
Contributor Author

@andreasplesch I think the concern is less about preferring PBR, and more about minimizing complexity of the format. Having more material types — especially official KHR_ ones — makes the glTF ecosystem less consistent and could make tooling less reliable. We might indeed want something like subsurface scattering by 2019, so balancing support for old standards vs newer without creating too much complexity is the goal.

@andreasplesch
Copy link
Contributor

andreasplesch commented Nov 29, 2017 via email

@zellski
Copy link
Contributor

zellski commented Dec 8, 2017

@donmccurdy I missed your reply earlier. I'm trying to work through the implications of A) and B).

For one thing, baseColorFactor and friends are in the PBR sub-struct, so we get something perhaps less natural:

(A) Use baseColor as input to unlit material.

"materials": [{
    "name": "red_tinted_constant_material",
    "pbrMetallicRougness": {
        "baseColorFactor": [ 1.0, 0.9, 0.9, 1.0 ],
        "baseColorTexture": { "index": 0, "texCoord": 0 },
        "roughnessFactor": 0.7,
        "metallicFactor": 0.3,
    },
    "emissiveTexture": { "index": 1, "texCoord": 0 },
    "extensions": {
        "KHR_materials_cmnConstant": {}
    }
}]

The fallback certainly become the obvious question; what does it mean, in a well-lit fallback environment, for the base colour to have been crated primarily to look right in unlit mode? We know that PBR albedo maps often don't exactly like emissive maps. I'm not experienced enough in this field to say how uneasily these two use cases would occupy the same texture, but it seems like it's at least a concern.

There's the additional implication that Option A obligates loaders that don't handle this unlit extension to provide a well-lit environment. But that's perhaps a basic requirement in PBR-land.

I'm kind of drawn to Option B, though. This separates the use cases entirely. In fact, is there really a reason to try to reuse naming schemes? We can craft something specifically for the purpose:

(B) define unlit colour entirely within the extension.

"materials": [{
    "name": "red_tinted_constant_material",
    "pbrMetallicRougness": " ... fallback omitted for brevity ... ",
    "emissiveTexture": { "index": 1, "texCoord": 0 },
    "extensions": {
        "KHR_materials_cmnConstant": {
            "colorFactor": [ 1.0, 0.9, 0.9, 1.0 ],
            "colorTexture": { "index": 2, "texCoord": 0 },
        }
    }
}]

This is appealing because of the clarity of the contract with the renderer: if you support the extension, you know exactly what to do with a single, dedicated, unlit RGB(A) map; if you don't, you can rest assured that the material configuration you read has not been influenced by the need to play a dual role.

This would admittedly walk us somewhat away from @snagy's philosophical direction, though.

@lexaknyazev
Copy link
Member

Maybe it's too late to even consider this, but what if we tweak core semantics a bit? Currently, these two materials are implicitly the same:

{
  "materials": [
    {
      "emissiveColor": [1, 0, 0]
    },
    {
      "emissiveColor": [1, 0, 0],
      "pbrMetallicRougness": {}
    }
  ]
}

pbrMetallicRoughness has default values for all its properties (baseColor and friends). But the material object itself doesn't have a schema-defined default value for pbrMetallicRoughness (like {}), so right now implementations should just assume it (according to schema description).

Maybe spec could distinct between these two cases.

@snagy
Copy link
Contributor

snagy commented Dec 8, 2017

This is appealing because of the clarity of the contract with the renderer: if you support the extension, you know exactly what to do with a single, dedicated, unlit RGB(A) map; if you don't, you can rest assured that the material configuration you read has not been influenced by the need to play a dual role.
This would admittedly walk us somewhat away from @snagy's philosophical direction, though.

I go back and forth on this.

  1. Putting it in emissive will instantly work in almost all the current gltf renderers (with a little specular highlighting in engines that don't support the extension). However, it probably won't really work correctly with a renderer that actually emits light from the emissive channel, and it can't be defined in vertex data.

  2. Putting it in base color will kinda work in renderers that don't support the extension; but, the visual look will diverge a lot more between renderers that are extension-aware and those that aren't.

  3. Defining it in a separate property is clean and lets the exporting program decide how non-supporting renderers will work. My main concerns with this were intent (what should a renderer do if they encounter both? is unlit an optimization or the preferred look?), and whether it actually gets us a win in most situations. Renderers still need to be aware of the extension to handle things properly, and instead of coming to a general consensus about the preferred way to handle the drawbacks of the above two cases, we're leaving it to the exporters to figure out their own solution and thus potentially making it more difficult for future renderers to consistently interpret content from different sources.

I lean towards base color, with the assumption that most renderers will support this extension, and base gives us unlit vertex colors.

@robertlong
Copy link
Contributor

robertlong commented Dec 9, 2017

I'm also leaning towards the second option that @snagy proposed. I believe having vertex color fallback is important and I also expect most engines to implement this extension. Can we close this PR and continue in #1163?

@zellski
Copy link
Contributor

zellski commented Dec 10, 2017

(@lexaknyazev It may have changed since, but at least for a while I'm pretty sure THREE.GLTFLoader was doing essentially that; the existence of pbrMetallicRougness (even when empty) triggered the creation of an internal PBR material, whereas in its absence I believe it fell back on Phong or even Lambert. It did feel fairly natural that leaving out the struct conspicuously named 'pbr...' would opt out of PBR. Then again, it feels this choice of defaults somewhat contradicts the fundamental assumption that glTF 2.0 is PBR.)

@donmccurdy
Copy link
Contributor Author

Thanks all! Closing this PR in favor of #1163.

@donmccurdy donmccurdy closed this Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.