Common Materials Extension #424
Comments
|
Related to #84 |
|
@pjcozzi yes to all of the above. did we decide to punt on environment maps? guess i missed the memo. for these two items, yes but it will take a little time to do these
|
See #266 |
|
@tparisi any update here? |
|
@tparisi Do we really need a separate transparency value? Shouldn't that just be part of the colors? |
|
thanks, after reading over the draft I would say this is exactly the kind of compact, basic material definition which we have been missing wihin glTF so far... definitely very helpful, thanks a lot for the great work! |
|
@mlimper any chance you could implement this soon? Your feedback would be valuable to help tighten up the spec. |
|
What we would like to implement is a very simple material model to transport colors of the parts - here, the CONSTANT, BLINN, PHONG, and LAMBERT models seem like a perfect match. I guess it could also be okay to use only PHONG or BLINN instead of using both, and maybe even LAMBERT could be left out, for the sake of simplicity, so that there is only CONSTANT and BLINN, for instance (since LAMBERT is just the diffuse part of PHONG / BLINN). Apart from that, my main question would be how exactly the lights fit in. We'll not want to use them within our system (although that doesn't necessarily mean they shouldn't be there), we'd only transport material information. One obvious possibility could move the lights out of this proposal and put them into a separate extension, but maybe that's too much and it's okay if they are optional. |
|
We also need prose for generating shaders with skinning. |
|
@mlimper let's not separate out lights, too complex. Let's instead define that an implementation's use of lights is optional, but if it does use them it needs to implement them correctly (or words to that effect) |
|
@tfili no we want to keep transparency separate. |
|
@pjcozzi @mlimper all requested changes are in except the lighting calculations, that's next. Please review the last few commits and LMK. I'm actually wondering about this 'optional lights' issue... it's sort of like cameras in the base glTF. What is a conforming implementation obligated to do? I feel like for both cameras and lights the use of them should be optional, but obviously, if they are used they need to be implemented to spec. Thoughts? |
|
First attempt at lighting calculations are in (see 4126edf) - please review |
Well, to be honest, I guess we would not implement lights from glTF, or at least they would get a very low priority, since we usually integrate data from multiple sources to show everythin in a common context. If we would also import lights with every asset, it would all become a huge mess. So, that would probably mean our implementation would not be 100% spec conformant, I guess... The conceptual problem here, and also with other things such as Cameras, is probably that glTF tries to be a format for both: Defining a single asset, and defining a complete scene setup. It's a problem of scope, lights and cameras often have a global scope, I guess, while meshes can be perfectly seen as something that can be integrated into an existing scene. However, it might not even be that clear for all cases: if a light is supposed to model the sun, it makes sense to ignore it when integrating an asset into an existing scene. In contrast, if a light is supposed to model one of the headlights of a car, attached to the model, it makes probably sense to load it, even if the current context already has lighting. @pjcozzi What is your approach in Cesium? What would happen if I would try to import an asset that has lights into an existing world? Possible conclusion: Some frameworks might want to ignore lights that have a global scope. Probably, all frameworks should be able to load lights that have a local scope, if they want to be spec-conformant. Therefore, one could add this scoping property to lights, to separate what is "scene data" and what is "asset data"... (maybe this also makes sense for other properties, such as camera viewpoints?) Another thing that could make sense is that the glTF spec provides a list of all existing features, such as "meshes", "lights", "cameras", ... . An implementation can then provide a list of features it supports. On top of that, one could identify certain feature sets as "support level X" or "profile Y", to ease communication about the state of an implementation. |
|
@tparisi Looking at |
|
@tfili good catch. Please make sure the converter does this and I will update the extension spec. |
|
@tparisi @pjcozzi In the converter generated shaders we look in the collada extra tags for double sided lighting and we respect that. It's used by a bunch exporters. Do we want to add this in the extension spec so we can generate the correct shaders or should I output it in the extras tag of the extension? |
|
I personally think we should just put double-sided into the |
|
I agree. Also one issue I'm running into is knowing if an image has transparency. When I get the glTF it just has the extension, so I have to generate techniques/shaders/etc, but I can't correctly detect the states at that point. I would have to wait for the image to load and figure it out. That is very difficult with our workflow. The converter however knows this already, so adding a property there would be much easier. For values that take a color or string, maybe have that be an color or object instead like this |
OK. |
|
As for if lights should be required, I suggest that we allow an asset to not have any lights, in which case, the engine is free to use their own lights, e.g., in Cesium, it would be the sun. If an asset contains lights, then the engine is required to generate a shader that uses those lights and any engine-specific lights it has. The Cesium team can make an open-source library to generate the shaders based on @tfili's work. In the short-term, lights in an asset are not a big deal for Cesium; longer term, we would use them for lights on satellites, aircrafts, trucks, etc. @tfili what do you think based on your experience implementing this extension so far? |
|
The intro says "BLINN" means "Blinn-Phong" but the Blinn section says it means "Blinn-Torrance-Sparrow." Which is it? Why not just stock Blinn-Phong? @tfili is looking at Three, Babylon, etc. to see what they implement, e.g., they might need to implement another a new material if glTF uses Blinn-Torrance-Sparrow and their engine doesn't. |
|
Ok, that works for me. I'm working on the implementation now. |
|
I've finished the converter and Cesium implementations. The things that should be updated in the spec are:
|
|
Nothing else from me. |
|
@tparisi see @tfili maps these to render states here: https://github.com/AnalyticalGraphicsInc/cesium/pull/3039/files#diff-8fd2167b1c1a0711731254f9aa4b45c9R532 |
|
OK, we'll open a pull request into master with the models. The issue is #448. |
|
I think the PR should be off the materials branch and then we'll merge back into master once everything is sorted out with the Three.js importer |
|
OK, we'll target the pull request to the materials branch. |
|
PR in #468 |
|
Just checked in language for |
|
if (khrMaterialsCommon.doubleSided) {
fragmentShader += ' if (gl_FrontFacing == false)\n';
fragmentShader += ' {\n';
fragmentShader += ' normal = -normal;\n';
fragmentShader += ' }\n';
} |
|
To finish this, I believe there are the following outstanding issues:
|
|
I think that's it |
|
We can close #510 and #509 - but #523 are we sure about removing type? see On Tue, Jan 12, 2016 at 1:36 AM, Max Limper notifications@github.com
Tony Parisi tparisi@gmail.com Read my books! Programming 3D Applications in HTML5 and |
|
If it would be updated in the examples, it would facilitate implementation and increase compatibility |
|
We are going to update the samples, see #523 (comment) |
|
Hello! |
|
It doesn't. It's meant to mimic Collada's common profile that supports basic lighting models (constant, lambert, blinn and phong). For more advanced techniques you'll have to provide shaders. |
|
Thanks for your reply.
provide shaders? You mean that I should use the shader generated by gltf converter? Or How can I provide shader? In my case, I just want to use my shader generated by my engine instead of using the shader provided by gltf converter. My shader supports normal map. |
|
Hi YYC You will need to create your own extension. Info here: https://github.com/KhronosGroup/glTF/blob/master/extensions/README.md Yours would be a "vendor extension." I can envision that we would someday add normal maps to the common materials extension. So maybe you can think about designing your extension for eventual inclusion in a future version of the KHR_ extension. We can't make about promises about that right now, but please keep the idea in mind while you are designing your feature and let's talk about it soon... |
|
OK, I get it, thanks a lot~ |
|
I believe this it is OK to close this as now duplicate with #1163 and friends. @donmccurdy please let me know if you think otherwise. |
https://github.com/KhronosGroup/glTF/tree/KHR_materials_common/extensions/Khronos/KHR_materials_common
Comments:
1and1.0are used. Arrays are sometimes on the same line and sometimes one element per line.index_of_refraction->indexOfRefraction. glTF uses camel case for property names.constant_attenuation,linear_attenuation,falloff_angle, andfalloff_exponent.CONSTANTneeds all those properties? How is each used?FLOAT_VEC4orSAMPLER_2D-> In glTF, what is sampler 2D? Shouldn't this be a string (texture id)? We need to precisely define where the texture coordinates come from, e.g., if bothdiffuseandspecularare texures, do they use the same or different set of texture coordinates?technique- why does this have a default? Perhaps it should be required. We should also make this separate from the table and say the table is for thevaluesobject."relective": "texture_envmap"- glTF does not support cube maps.Maybe, but I did to finish the core spec reference doc first. Will let you know later this week.
The text was updated successfully, but these errors were encountered: