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

GLSL, Vulkan, and portable materials #733

Closed
pjcozzi opened this issue Sep 28, 2016 · 13 comments
Closed

GLSL, Vulkan, and portable materials #733

pjcozzi opened this issue Sep 28, 2016 · 13 comments

Comments

@pjcozzi
Copy link
Member

pjcozzi commented Sep 28, 2016

For consideration post 1.0.1...

glTF currently requires that renderers be able to use embedded/referenced GLSL shaders. I'd like us to consider moving this to an extension for the following reasons:

  • As we focus on PBR (issues), it may very well become core spec, and this is the direction the industry should move
  • The GLSL payload is problematic for supporting different versions of WebGL, OpenGL, and Vulkan (Version numbers and profile specifiers in shaders #587), and makes it unreasonable to implement a D3D renderer
  • Many users are only interested in glTF for geometry / geometry + animations; we even made this a validation level in the validator. They can't use the GLSL payload because their engine is a deferred renderer, has its own light sources, etc.
  • Many users are already patching the GLSL after it is downloaded, e.g., Cesium derives a new shader for picking.

To stay with the spirit of glTF's simplicity, it will be key that the material description payload be reasonable to implement and that plenty of sample code is available.

I'm open to proposals from the community on how this would be designed (GLSL extension + PBR work already in progress). I am also happy to propose ideas.

@pjcozzi pjcozzi added specification 1.1 PBR Physically Based Rendering labels Sep 28, 2016
@lexaknyazev
Copy link
Member

A big +1 from me.
Aligning Vulkan and GL (all flavors) uniforms already doesn't feel good (#717).

Spec updates are very simple here (first thoughts):

  • Remove techniques, shaders, and programs from the core spec.
  • Define KHR_gltf_vulkan and likewise GL extensions for gLTF root (for storing shaders, programs, and techniques) and for material objects (for actually using them).

With such scheme, API specifics don't affect asset's compatibility as long as there are high-level (like PBR) material properties.

Also, having API specifics in separate objects doesn't require us to align APIs with each other (think of UBOs, framebuffers, renderpasses, transform feedback, etc).

@lexaknyazev
Copy link
Member

lexaknyazev commented Sep 29, 2016

Other API-related stuff:

  • accessor
    • componentType: valid values depend on API (ES 3.0 or WebGL 2.0 add HALF_FLOAT, INT, UNSIGNED_INT, INT_2_10_10_10_REV, and UNSIGNED_INT_2_10_10_10_REV).
    • type: same as previous (ES 3.0 or WebGL 2.0 add INT, IVEC2, IVEC3, IVEC4, MAT2x3, MAT2x4, MAT3x2, MAT3x4, MAT4x2, MAT4x3).
    • Vulkan has a very different approach for attributes formats, maybe we should use it for all APIs instead of type / componentType combinations.
    • We can safely add accessor.divisor and mesh.primitive.instanceCount to leverage hardware instancing when using custom shaders. For high-level (e.g., PBR materials) usage, maybe we could come up with some convention on storing per-instance uniforms (via arrays / UBOs). This feature is supported by all modern APIs (ES 2.0 / WebGL 1.0 with extension, ES 3.0 / WebGL 2.0, Vulkan, D3D9-12).
  • asset.profile could be an array, so when API-related extensions are present, it contains all supported APIs names. Such array could be empty when only high-level (e.g., PBR materials) materials are used.
  • bufferView.target currently uses ES 2.0 targets. For other APIs valid values are different. Maybe, we could remove that property and let runtime figure out proper target depending on bufferView usage?
  • glTF.glExtensionsUsed could be placed into the corresponding API-related object.
  • mesh.primitive.indices: WebGL 2.0 behaves as though PRIMITIVE_RESTART_FIXED_INDEX state is always enabled; other APIs could have different behavior. Maybe, it would be better for compatibility to avoid extreme values.
  • mesh.primitive.mode: valid values depend on used API, Vulkan's enum is not the same as GL.
  • sampler and texture: tons of API bindings there.

Anyway, I think that moving away from a specific API is a good thing for broad glTF adoption, but we still need to keep in mind that glTF aims to be a runtime format.

@pjcozzi What do you think?

@pjcozzi
Copy link
Member Author

pjcozzi commented Oct 3, 2016

Spec updates are very simple here (first thoughts)...

Agreed.

Do you think it is possible to do this in a backwards compatible way? For example, when KHR_gltf_webgl is defined, the materials/programs/etc are just as they are today?

With such scheme, API specifics don't affect asset's compatibility as long as there are high-level (like PBR) material properties.

Also agreed. You bring up a lot of good points for making glTF broadly tuned for multiple graphics APIs. I feel like it is a lot to try to tackle everything at once. Do you think it is a reasonable approach to focus on:

  • First, moving the current GL-specific materials to an extension,
  • Second (and really concurrently given the already ongoing work), defining the PBR extension and proposing it as core spec,
  • Third, generalizing glTF across APIs. Given (1) and (2), glTF can already be used across APIs and folks are already doing Vulkan renderers. This would allow those users to really tune glTF for the API as you point out above, GLSL, Vulkan, and portable materials #733 (comment)

@lexaknyazev
Copy link
Member

Do you think it is possible to do this in a backwards compatible way? For example, when KHR_gltf_webgl is defined, the materials/programs/etc are just as they are today?

I think we must very accurately estimate real usage of the current (1.0) spec version in terms of existing content and runtime/pipeline maturity. Backwards compatibility isn't always a good thing: think of the whole desktop GL history; OpenGL ES 1.1 -> 2.0 transition was much cleaner, imho. Making drastic changes at the beginning of the format's life is easier than making them later.

I feel like it is a lot to try to tackle everything at once.

Considering growing industry attention to glTF, I think we should make next format's version as future-proof as possible and freeze major/minor version number for several years. See related thoughts from the PNG spec:

The chunk naming conventions allow safe, flexible extension of the PNG format. This mechanism is much better than a format version number, because it works on a feature-by-feature basis rather than being an overall indicator. Decoders can process newer files if and only if the files use no unknown critical features (as indicated by finding unknown critical chunks). Unknown ancillary chunks can be safely ignored. We decided against having an overall format version number because experience has shown that format version numbers hurt portability as much as they help. Version numbers tend to be set unnecessarily high, leading to older decoders rejecting files that they could have processed (this was a serious problem for several years after the GIF89 spec came out, for example). Furthermore, private extensions can be made either critical or ancillary, and standard decoders should react appropriately; overall version numbers are no help for private extensions.

A hypothetical chunk for vector graphics would be a critical chunk, since if ignored, important parts of the intended image would be missing. A chunk carrying the Mandelbrot set coordinates for a fractal image would be ancillary, since other applications could display the image without understanding what the image represents. In general, a chunk type should be made critical only if it is impossible to display a reasonable representation of the intended image without interpreting that chunk.


glTF can already be used across APIs and folks are already doing Vulkan renderers

Those renderers have to use not-so-good hacks and quirks. Supplying custom shaders for them is almost broken (with current glTF core). I don't think we should encourage such approaches.

This would allow those users to really tune glTF for the API

For an asset to be portable, such "tuning" for Khronos APIs should be defined as ratified glTF extension, not as user/vendor custom fields.


I think we should start with a clear definition of "spec version" vs "spec profile". Do we want any API profile to be a part of core glTF spec? If so, could new profiles be defined for already existing spec version? The other approach is to remove all API references from core.

@mre4ce
Copy link

mre4ce commented Oct 7, 2016

Do we have a list of all shader options we want to support? It could become a pretty large matrix.

For instance, these are some of the obvious cases:

  • GLSL OpenGL
  • GLSL Vulkan
  • SPIR-V OpenGL
  • SPIR-V Vulkan
  • HLSL

Then there's all the different versions of the shader languages. I assume we pick a couple of common ones.

I also need multi-view (GL_OVR_multiview2) variants of the shaders. This is obviously an important extension for VR but there may be many other extensions that turn out to be important to people. Supporting such extensions can significantly blow up the support matrix.

And I guess we will support "fat" glTF files that include all shader variants that are supported which means all variants need to be able to coexist.

@lexaknyazev
Copy link
Member

Do we have a list of all shader options we want to support? It could become a pretty large matrix.

Not yet; we need to set the scope. Besides your five options, there's also ESSL (1.0 and 3.0), which is the only way for WebGL.

there may be many other extensions that turn out to be important to people. Supporting such extensions can significantly blow up the support matrix.

We could think of "feature/capability flags" for each provided "technique". It could be up to runtime to choose proper technique/program.

And I guess we will support "fat" glTF files that include all shader variants that are supported which means all variants need to be able to coexist.

Exactly. That's why all technique/shader/etc objects should be provided in extensions.

@emackey
Copy link
Member

emackey commented Oct 7, 2016

Rather than creating a lot of "fat" glTF files, I would rather see (a) a shaderless PBR glTF file that can be rendered similarly on any platform using the rendering engine's own PBR shader, or, failing that, (b) a client issues a REST request clearly stating which shader flavor is desirable for that platform, and gets a response with only the correct shader language baked in.

I think (a) is more practical than (b) in general. For web-based clients using (b), of course WebGL (GLSL/ESSL) shaders would be requested exclusively. Only native clients (desktop or mobile non-web) would request glTF with anything other than WebGL shaders, and those clients are more likely to have glTF files bundled into the installer from the start.

But ideally, if we can make robust PBR support, we can relegate customized shaders to the land of lesser-used extensions. It would mean every glTF reader would ship with its own PBR shader, but I think this would be a win across the board.

@lexaknyazev
Copy link
Member

Only native clients (desktop or mobile non-web) would request glTF with anything other than WebGL shaders, and those clients are more likely to have glTF files bundled into the installer from the start.

They still could have different versions of the same shader to support more platforms. Other option could be to provide multiple flavors of the same asset instead of one "fat" glTF.

But ideally, if we can make robust PBR support, we can relegate customized shaders to the land of lesser-used extensions.

Couldn't agree more. Possible GLSL variations make glTF too fragile.

@mre4ce
Copy link

mre4ce commented Oct 8, 2016

That's what we did in the ovrscene format. Shaders were not part of the format and the scene could effectively only use one of four shaders:

1. vertex color (no textures)
2. diffuse only (diffuse)
3. light mapped (diffuse + lightmap)
4. reflection mapped (diffuse + lightmap + normal + specular + power + reflection cube)

And then each surface could use one of four blend options:

1. opaque
2. perforated
3. transparent
4. additive

https://developer3.oculus.com/documentation/mobilesdk/latest/concepts/mobile-fbx-overview/

I like the flexibility that comes with storing the shaders in the glTF format, but all different shader variants makes it a mess. If we can untangle that as a set of extensions then we have the best of both worlds.

@pjcozzi
Copy link
Member Author

pjcozzi commented Oct 12, 2016

Sorry for the slow response; I was out all last week. It looks like this discussion has converged nicely:

If I understand correctly, we all agree that making PBR core in glTF would be the best approach, then extensions would allow API-specific shader payloads.

I still feel like doing this all at once would be a lot of work. I would suggest doing PBR first (which appears to be moving quickly) and mapping out what the API-specific extensions would look like, using the current WebGL/GLSL payloads as a concrete example and perhaps WebGL 2 / Vulkan, but not doing all of them now.

Also, @mre4ce if you are interested in PBR discussions, check out these issues. In particular, #696 is a great discussion that we need to converge on.

@pjcozzi
Copy link
Member Author

pjcozzi commented Feb 3, 2017

For glTF 2.0, this is resolved with KHR_technique_webgl in #826. Instead of labeling this issue resolved and closing, I am just going to remove the 2.0 since it has useful discussion for Vulkan and other use cases.

@pjcozzi
Copy link
Member Author

pjcozzi commented Feb 4, 2017

Metal Roughness PBR was added to the core spec in #830.

@donmccurdy donmccurdy changed the title Do not require GLSL to be part of a glTF asset GLSL, Vulkan, and portable materials Sep 19, 2018
@donmccurdy
Copy link
Contributor

Tagging as "archive" and closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants