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

Add alpha coverage details #864

Merged
merged 3 commits into from
Mar 31, 2017
Merged

Add alpha coverage details #864

merged 3 commits into from
Mar 31, 2017

Conversation

sbtron
Copy link
Contributor

@sbtron sbtron commented Mar 10, 2017

Adding details on alpha coverage as per #822
Also added doubleSided property from common materials extension to core spec since it can be used across multiple material models.

Doc: https://github.com/sbtron/glTF/tree/2.0Transparency/specification/2.0#alpha-coverage
Schemas:
https://github.com/sbtron/glTF/blob/2.0Transparency/specification/2.0/schema/material.schema.json
https://github.com/sbtron/glTF/blob/2.0Transparency/specification/2.0/schema/material.pbrMetallicRoughness.schema.json

@AurL @cedricpinson could you review and ensure this meets your needs so you no longer have to use the extras field.


When `alphaMode` is set to `MASK` the `alphaCutoff` property specifies the cutoff threshold. If the alpha value is greater than or equal to the `alphaCutoff` value then it is rendered as fully opaque, otherwise, it is rendered as fully transparent. `alphaCutoff` value is ignored for other modes.

The `doubleSided` property specified whether the material is double sided.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit out of place. Double sided has nothing to do with alpha coverage?

"doubleSided": {
"type": "boolean",
"default": false,
"description": "Indicates whether a material is double sided.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a material --> the material (to be consistent with the rest of the descriptions)

@AurL
Copy link

AurL commented Mar 13, 2017

@sbtron It looks good, we don't need the extra field anymore, and the doubleSided field will be useful for us as well. We will include these changes in the next update.
Thanks a lot for this.

@emackey
Copy link
Member

emackey commented Mar 13, 2017

Agreed glTF needs doubleSided particularly to enable/disable back-face culling. But we should be careful not to confuse this with the choice of single-sided lighting vs. double-sided lighting (specifically, should the back side of a material, if not culled, should render with the front side's normal vector or the opposite of that. Typically, thin translucent materials will use the front side's normal, and thicker opaque materials will flip the normal).

@bghgary
Copy link
Contributor

bghgary commented Mar 13, 2017

@emackey We should clarify this in the spec. Should this be an option or should we pick one for the spec?

@emackey
Copy link
Member

emackey commented Mar 13, 2017

I wonder if people would agree to an enum here? There could be one enum value to cull the back face, another value to render it with single-sided lighting (using the front face's normal vector) and another option to render it with double-sided lighting (flip the normal vector, after applying the normal map etc). Edit: Changed my mind, see below.

@emackey
Copy link
Member

emackey commented Mar 13, 2017

glTF 1.0 had backface culling as an option CULL_FACE in an enum in technique.states. It didn't need to address the single/double-sided lighting question, as that was an implementation detail of the supplied shader.

@bghgary
Copy link
Contributor

bghgary commented Mar 13, 2017

I don't have data here. How common is singled-sided lighting vs. double-sided lighting?

@emackey
Copy link
Member

emackey commented Mar 13, 2017

I don't have data from the outside world. My company does maintain a collection of aerospace-related models that use both lighting models for different circumstances. But I can't speak for the graphics industry in general.

Ye olde fixed-function OpenGL did this with a parameter named GL_LIGHT_MODEL_TWO_SIDE.

https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/glLightModel.xml

https://msdn.microsoft.com/en-us/library/windows/desktop/dd373582(v=vs.85).aspx

(And if I recall, back then it was rumored to be a small performance hit, so you would never turn on GL_LIGHT_MODEL_TWO_SIDE if you were also going to enable CULL_FACE.)

@emackey
Copy link
Member

emackey commented Mar 13, 2017

In WebGL we use gl_FrontFacing to detect which side is being rendered in the fragment shader. Any shader that doesn't pay attention to that effectively uses single-sided lighting when backface culling is turned off. That does keep the shader simpler if we ignore that, so I wouldn't be strongly opposed to omitting the option for two-sided lighting on double-sided materials.

@lexaknyazev
Copy link
Member

In #822, we agreed on using alpha for coverage only. Single-sided lighting with double-sided rendering refers to light transmission scenario which we wanted to avoid.

@emackey
Copy link
Member

emackey commented Mar 14, 2017

Single-sided lighting with double-sided rendering refers to light transmission scenario which we wanted to avoid.

True, but I may have opened a can of worms here. Take a look at the various sample PBR implementations that have been posted here, do any of them use gl_FrontFace in the fragment shader? I haven't found one that does. ThreeJS.MeshStandardMaterial doesn't support it. Two-sided lighting may be a concept that's fallen out of use, and would require new code paths to support.

In the name of simplicity for the runtime engine, then, we could just offer a boolean for backface culling, but not add the complexity of two-sided lighting. It could be added in an extension later, if needed.

@bghgary
Copy link
Contributor

bghgary commented Mar 14, 2017

Single-sided lighting with double-sided rendering refers to light transmission scenario which we wanted to avoid.

What about opaque materials? Should be able to support double-sided and flipped normals on the back?

@lexaknyazev
Copy link
Member

What about opaque materials? Should be able to support double-sided and flipped normals on the back?

Yes.

@bghgary
Copy link
Contributor

bghgary commented Mar 14, 2017

Should have been more clear with my previous comment. Is the following statement correct?

"Normals should always be flipped on the backside for lighting calculations when doubleSided property is true."

This is the exact opposite of what @emackey said with #864 (comment)?

@emackey
Copy link
Member

emackey commented Mar 14, 2017

What about opaque materials? Should be able to support double-sided and flipped normals on the back?

Yes.

No, please, do some research before you jump on this. My own examples were from the 1990's fixed-function days, and looking around today I have yet to find any shaders doing this in modern times. I now think this could become a roadblock to writing conformant glTF renderers.

Remember, the goal of glTF is to deliver something that's easy and practical to render -- not to uphold some purist notions about alpha being used only for coverage. Sometimes we need to be practical.

@emackey
Copy link
Member

emackey commented Mar 22, 2017

I hope I didn't derail this with my old memories of two-sided lighting. Having an option to enable/disable backface culling is needed, as is the alpha settings that make up the original point of this PR. Perhaps the lighting issue could be addressed separately.

@bghgary
Copy link
Contributor

bghgary commented Mar 22, 2017

I'm working with @BeardedGnome to ensure that it is reasonable to implement double-sided lighting in WebGL, using BabylonJS. If it is reasonable, then I think double-sided materials should imply double-sided lighting (i.e. the normals are flipped on the back side). Thoughts?

@emackey
Copy link
Member

emackey commented Mar 22, 2017

It's probably not a huge burden, I think you just test gl_FrontFacing in the fragment shader and then flip the back-facing normals. I just think it may be uncommon these days.

There will be 3D artists who insist on using alpha as non-coverage translucency, as they have no other means to achieve that effect, and some option for single-sided lighting on non-culled back faces will be quite important to them. I'm still under the impression that "modern" rendering engines generally only offer single-sided lighting, so they may expect this quite strongly.

@lexaknyazev
Copy link
Member

It's probably not a huge burden, I think you just test gl_FrontFacing in the fragment shader and then flip the back-facing normals. I just think it may be uncommon these days.

@emackey Just to summarize, are you OK with doubleSided bool flag?

There will be 3D artists who insist on using alpha as non-coverage translucency

I think that we should limit the scope of 2.0 material model to opaque (with coverage) materials only. Translucency effects are still important, so future glTF updates should address some of them.

@emackey
Copy link
Member

emackey commented Mar 22, 2017

are you OK with doubleSided bool flag?

I want the flag, but I think it would be clearer to name it backfaceCulling or such, although this would change the true/false sense of it.

I don't like the idea of switching both the lighting model and the culling option with a single boolean value, as they are quite separate concerns to the engine. In OpenGL/WebGL, the choice of culling is a simple state enable/disable, but the lighting model requires an extra if block with some vector math in the fragment shader, plus a uniform if you want control of that.

@bghgary
Copy link
Contributor

bghgary commented Mar 25, 2017

It looks like Unreal 4 supports two-sided materials with two-sided lighting and they don't have an option to turn two-sided lighting off. Here is what it looks like:

From the top (front of the plane):
image

From the bottom (back of the plane):
image

Here is what it takes to implement two-sided lighting in BabylonJS:
BabylonJS/Babylon.js#1930

Here is a live example of what it looks like in BabylonJS:
https://bghgary.github.io/glTF/two-sided-lighting

Given this, I think it's clear that two-sided lighting works and it should be okay for us to assume it is always on since Unreal does it too.

I'm okay with calling it backfaceCulling instead of doubleSided.

Thoughts?

@emackey
Copy link
Member

emackey commented Mar 27, 2017

Sounds good. Thanks for looking into UE4.

@bghgary
Copy link
Contributor

bghgary commented Mar 27, 2017

We are planning on leaving the name doubleSided since it implies backface culling is off and double-sided lighting is on. This is analogous to UE4 where they have a property called Two Sided on their materials.

image

@emackey
Copy link
Member

emackey commented Mar 27, 2017

If you're doing both from the same flag, then yes, that makes sense.

@McNopper
Copy link
Contributor

FYI, some tools (e.g. Blender) store a similar flag per mesh and not material.

@emackey
Copy link
Member

emackey commented Mar 30, 2017

Blender defaults to double-sided, but has a number of options that apply or don't apply to its various rendering engines. Blender Original, Blender Game, Cycles renderer, and the OpenGL/GLSL preview render, as well as the "Multitexture" preview render, all can behave differently or support options that others don't support inside of Blender itself. The OpenGL/GLSL one does make a distinction between double-sided lighting and backface culling, but it may be alone in that distinction.

blenderdoublesided

Anyway, I've agreed to having a single doubleSided flag in glTF. I don't think we should continue to hold up progress on 2.0 for this minor issue. It can always be tweaked with extensions if it turns into a problem later.

@sbtron
Copy link
Contributor Author

sbtron commented Mar 30, 2017

Added explanation of doublesided lighting to readme and schema. @emackey can you review?

@emackey
Copy link
Member

emackey commented Mar 31, 2017

Added explanation of doublesided lighting to readme and schema

The new explanation looks correct to me. Thanks!

@McNopper
Copy link
Contributor

Good for me as well.
My comment was more about, that the value is assigned to a mesh and not the material. But as I said, I am fine with this.

@emackey
Copy link
Member

emackey commented Mar 31, 2017

@McNopper Ah I see, but, I think the flag does "belong" on the material more than the mesh. Lightwave does this IIRC. Blender does some very odd things like associating its backface culling toggle with particular viewports instead of either meshes or materials. I think associating it with the material makes the most sense, particularly for engines that are trying to minimize render state changes.

>**Implementation Note for Real-Time Rasterizers:** Real-time rasterizers typically use depth buffers and mesh sorting to support alpha modes. The following describe the expected behavior for these types of renderers.
>* `OPAQUE` - A depth value is written for every pixel and mesh sorting is not required for correct output.
>* `MASK` - A depth value is not written for a pixel that is discarded after the alpha test. A depth value is written for all other pixels. Mesh sorting is not required for correct output.
>* `BLEND` - Support for this mode varies. There is no perfect and fast solution that works for all cases. Implementations should try to achieve the correct blending output for as many situations as possible. Whether depth value is written or whether to sort is up to the implementation. For example, implementations can discard pixels which have zero or close to zero alpha value to avoid sorting issues.

### Double Sided

The `doubleSided` property specified whether the material is double sided. When this value is false, back-face culling is enabled. When this value is true, back-face culling is disabled and double sided lighting is enabled. The back-face must have its normals reversed before the lighting equation is evaluated.
Copy link
Contributor

Choose a reason for hiding this comment

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

specified should be specifies

@sbtron sbtron merged commit 47ac954 into KhronosGroup:2.0 Mar 31, 2017
@sbtron sbtron deleted the 2.0Transparency branch April 6, 2017 23:31
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.

7 participants