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

Textures update for glTF 2.0 #860

Closed
wants to merge 4 commits into from
Closed

Textures update for glTF 2.0 #860

wants to merge 4 commits into from

Conversation

lexaknyazev
Copy link
Member

@lexaknyazev lexaknyazev commented Mar 1, 2017

This PR contains updated schemas and description of texture data handling for glTF 2.0.

Mini-FAQ

Why move internalformat to image?

Because this property is deeply bound with supplied data. One could switch different images while keeping the same texture object.

Why remove type and format?

These two properties can be precisely derived from one internalformat value. No need to complicate asset validation with checking all possibly wrong combinations.

Why use OpenGL ES 3.0 enums instead of ES 2.0?

They provide distinct values for each format/type combination.

Why source.uris and source.bufferViews are defined as arrays?

To support PNG/JPEG-based cubemaps and texture arrays in future 2.x releases without breaking JSON syntax.

Why texture.sampler is optional now?

Because all fields of sampler object were already optional. So exporters won't need to create an empty sampler object as before:

{
  "textures": [
    {
      "sampler": 0, ...
    }
  ],
  "samplers": [ { } ]
}

What do "expand" and "repack" mean in the second table?

Expand means populating non-provided texture channels by some rule. For example, to expand LUMINANCE into RGBA, client should emit (L, L, L, 1.0) for each texel. See more on that topic in OpenGL specs.

Repack means changing the order of packed components to adapt to another API's convention.

Note, that ANGLE does such transformations when running on D3D backend.


CC @pjcozzi @sbtron

@pjcozzi
Copy link
Member

pjcozzi commented Mar 8, 2017

For #835

@pjcozzi
Copy link
Member

pjcozzi commented Mar 8, 2017

@lilleyse @lasalvavida can you both please review this to confirm that there is a reasonable update path from our current implementations (pipeline and runtime) to this? This is a significant, but hopefully isolated change.


First image pixel corresponds to the lower left corner of the image.

> **Implementation Note**: With WebGL API, the first pixel transferred from the `TexImageSource` (i.e., HTML Image object) to the WebGL implementation corresponds to the upper left corner of the source. To achieve correct rendering, WebGL runtimes must flip Y axis by enabling `UNPACK_FLIP_Y_WEBGL` flag. This behavior was changed from glTF 1.0.
Copy link
Member

Choose a reason for hiding this comment

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

Once this is merged, please update #605 with changes like this, especially this one since it is breaking but the syntax doesn't change.


> **Implementation Note**: This increases portability of an asset, since not all image decoding libraries fully support custom color conversions. To achieve correct rendering, WebGL runtimes must disable such conversions by setting `UNPACK_COLORSPACE_CONVERSION_WEBGL` flag to `NONE`.

In the following example asset provides two versions of the same image: in sRGB and in linear colorspaces; so clients with hardware sRGB filtering support (or with enough fragment shader processing power) benefit from reduced banding, whilst low-power clients still get correct texel values by using linear-space image data directly.
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a conformant renderer is free to select any object in the sources array. Can the spec be more explicit about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

sources are different representations of the same image. So choice is ultimately up to engine (based on mimeType and internalformat).

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, let's just be super explicit about it in the spec.

### Samplers
#### Image Formats

glTF 2.0 allows a limited set of possible `internalformat` values based on OpenGL ES 3.0 enums.
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it possible that a glTF asset will contain a texture format that a WebGL 1.0 client will not be able to render?

| RGBA4 (`32854`) | RGBA | RGBA | RGBA4 | UNSIGNED_SHORT_4_4_4_4 |
| RGB5_A1 (`32855`) | RGBA | RGBA | RGB5_A1 | UNSIGNED_SHORT_5_5_5_1 |
| RGBA8 (`32856`) | RGBA | RGBA | RGBA8 | UNSIGNED_BYTE |
| SRGB8 (`35905`) | SRGB_EXT | RGB | SRGB8 | UNSIGNED_BYTE |
Copy link
Member

Choose a reason for hiding this comment

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

Does this require an extension? https://www.khronos.org/registry/webgl/extensions/EXT_sRGB/

The spec should clarify this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The best WebGL 1.0 runtime strategy is to use that extension, yes. Without it, the only correct option is to disable hardware filtering and perform it manually in GLSL (after manual colorspace conversion).

Supplying alternative image source with linear data could help such incapable clients. Quality will suffer from banding, obviously.

@lilleyse
Copy link
Contributor

lilleyse commented Mar 11, 2017

I don't see any problems here, just making conversions to the correct internalFormat, format, type whether it's a webgl1 or 2 context.

@lexaknyazev
Copy link
Member Author

Retargeting to post-2.0.

@lexaknyazev lexaknyazev closed this Jun 7, 2017
@lexaknyazev lexaknyazev deleted the textures-update branch January 27, 2019 18:09
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.

3 participants