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

Support for compressed sRGB formats #6743

Merged
merged 7 commits into from
Jul 1, 2024
Merged

Support for compressed sRGB formats #6743

merged 7 commits into from
Jul 1, 2024

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Jun 21, 2024

  • Both WebGL and WebGPU supports sRGB version of compressed texture formats
  • compressed texture formats are used when the asset has { srgb = true; } option, similarly to non-compressed formats
  • glb parser requests srgb texture formats for base and emissive color textures, as those are stored in sRGB space
  • updated couple of fragment shaders in the examples: as glb supplied diffuse texture is not stored in sRGB format, which is automatically sampled as a linear value, the custom shader now needs to convert the result back to sRGB. The shader can no longer work completely in sRGB space.

partially implements #3715

@LeXXik
Copy link
Contributor

LeXXik commented Jun 21, 2024

By the way, is there a decode function for 4 component vectors? I remember we needed a texture with alpha channel, but seems $DECODE variants are only for vec3. Not a big deal, as we just manually do the same, just curious.

@mvaligursky
Copy link
Contributor Author

We don't have one as transparency is in linear space. But I guess if you store other data in the alpha channel (say a greyscale color), you might need to use a custom decode function.

@LeXXik
Copy link
Contributor

LeXXik commented Jun 21, 2024

Right, custom data. Alright, thanks.

src/framework/handlers/basis-worker.js Outdated Show resolved Hide resolved
src/platform/graphics/constants.js Outdated Show resolved Hide resolved
src/platform/graphics/constants.js Show resolved Hide resolved
src/platform/graphics/constants.js Outdated Show resolved Hide resolved
Co-authored-by: Will Eastcott <will@playcanvas.com>
src/platform/graphics/constants.js Outdated Show resolved Hide resolved
src/platform/graphics/constants.js Outdated Show resolved Hide resolved
src/framework/parsers/texture/basis.js Show resolved Hide resolved
src/platform/graphics/constants.js Show resolved Hide resolved
[PIXELFORMAT_RGBA4, { name: 'RGBA4', size: 2, srgb: true }],
[PIXELFORMAT_RGB8, { name: 'RGB8', size: 4, srgb: true }],
[PIXELFORMAT_RGBA8, { name: 'RGBA8', size: 4, srgb: true }],
[PIXELFORMAT_A8, { name: 'A8', size: 1, ldr: true }],
Copy link
Member

Choose a reason for hiding this comment

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

I still think it would be neater to flag the few hdr formats as hdr rather than the relatively many formats as ldr. (Integer formats aren't color formats and so shouldn't get either flag).

Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

Approved with some comments

@mvaligursky mvaligursky merged commit 5bf5df8 into main Jul 1, 2024
8 checks passed
@mvaligursky mvaligursky deleted the mv-compressed-srgb branch July 1, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants