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

Bad quality for texture compressed with default etc1s command #1062

Closed
marwie opened this issue Aug 12, 2023 · 8 comments
Closed

Bad quality for texture compressed with default etc1s command #1062

marwie opened this issue Aug 12, 2023 · 8 comments
Labels
bug Something isn't working external Problems or limitations traced back to other tools. package:cli

Comments

@marwie
Copy link
Contributor

marwie commented Aug 12, 2023

Describe the bug
Compressing the metallic-roughness texture with etc1s results in terrible quality

To Reproduce
Steps to reproduce the behavior:

  1. Download zip watch.zip
  2. Run gltf-transform etc1s "Watch.glb" "Watch.etc1s.glb

Expected behavior
Better texture quality 😅
screenshot below is compressed with https://github.khronos.org/glTF-Compressor-Release/ default etc1s settings
image

Versions:

  • Version: 3.5.1
  • Environment: cli

Additional context

Before compression
image

After compression
image

@marwie marwie added the bug Something isn't working label Aug 12, 2023
marwie added a commit to needle-tools/needle-engine-samples that referenced this issue Aug 12, 2023
@donmccurdy
Copy link
Owner

donmccurdy commented Aug 14, 2023

The relevant KTX Software call:

toktx --genmipmap --bcmp --assign_oetf linear --assign_primaries none metallicRoughness.ktx2 metallicRoughness.jpg

glTF Transform uses default KTX Software settings for the uastc and etc1s commands. When certain flags are unambiguously required for a material slot (e.g. to indicate non-color textures) those flags are added by default, but the more subjective flags are kept to their defaults.

While I'd certainly be interested to learn if you find ETC1S settings that give acceptable results here, it's my opinion that while ETC1S can sometimes give decent visual quality for non-color textures, it often cannot. You may want ETC1S for color textures and UASTC (or traditional image compression) for the rest. I try not to take a strong stance on what optimization steps users "should" choose, but the flags chosen for the all-in-one optimize command are more opinionated:

// Texture compression.
if (opts.textureCompress === 'ktx2') {
const slotsUASTC = '{normalTexture,occlusionTexture,metallicRoughnessTexture}';
transforms.push(
toktx({ mode: Mode.UASTC, slots: slotsUASTC, level: 4, rdo: 4, zstd: 18 }),
toktx({ mode: Mode.ETC1S, quality: 255 }),
);
} else if (opts.textureCompress !== false) {
transforms.push(
textureCompress({
encoder: sharp,
targetFormat: opts.textureCompress === 'auto' ? undefined : opts.textureCompress,
resize: [opts.textureSize, opts.textureSize],
}),
);
}

That's equivalent to what the KTX2 Artist Guide recommends as well —

gltf-transform uastc input.glb output1.glb --level 4 --rdo 4 --slots "{normalTexture,occlusionTexture,metallicRoughnessTexture}" --zstd 18 --verbose
gltf-transform etc1s output1.glb output2.glb --quality 255 --verbose

@hybridherbst
Copy link

We're so far pretty happy with the default settings for ETC1S, and the file and GPU size savings are so considerable that they offset the quality in almost all cases.
(That being said, we do provide a choice for people inside Needle Engine compression for what slots should be compressed with what)

This texture absolutely seems to be an outlier – it's the first time we see ETC1S go so horribly wrong. Yes, there can be artifacts, but not something that looks like 128px downsampling...
Do you know where this should be reported upstream? The ktxtools repo? Ideally defaults between different Khronos projects should probably match

@donmccurdy
Copy link
Owner

donmccurdy commented Aug 16, 2023

As far as I can see, the ETC1S default settings in the new Khronos glTF Compressor for web are essentially identical to what glTF Transform uses, with one exception — the web app doesn't generate mipmaps, probably because libktx does not support them. If we test the toktx command above, omitting the --genmipmap flag, you'll get the full quality back. That's certainly a surprise to me. I'm not sure why generating mipmaps should affect the quality at all, much less affect it so heavily.

glTF Transform doesn't expose the option to disable mips, though I'm open to doing that. But it'd be great to figure out why mipmaps matter here, and whether that's a bug. Perhaps a bug report on the KTX-Software repository would be a good next step, yes.

@donmccurdy donmccurdy added the external Problems or limitations traced back to other tools. label Aug 16, 2023
@donmccurdy donmccurdy added this to the v3.5 milestone Aug 16, 2023
@hybridherbst
Copy link

hybridherbst commented Aug 16, 2023

That's great additional info, thanks for checking. I agree that disabling mipmaps is no solution here.

So basically the bug seems to be that with mipmaps enabled, the highest mipmap (and all lower ones) change content, whereas an identical highest mipmap to "no mipmaps" would probably be expected. I'll report that to the KTX-Software repo.

@donmccurdy donmccurdy modified the milestones: v3.5, v3.6, Backlog Aug 22, 2023
@donmccurdy
Copy link
Owner

@hybridherbst
Copy link

Some additional findings related to gltf.report: it looks like there is an ok-ish mip 0 in the file, but mips degrade very fast. Seems that gltf.report doesn't display mip 0 but instead uses a lower mip (which I think is fine because the displayed image is small), and that lower mip is broken for this file.

@donmccurdy
Copy link
Owner

Seems to be nothing we can do on this side, upstream discussion in:

@donmccurdy donmccurdy closed this as not planned Won't fix, can't repro, duplicate, stale Feb 18, 2024
@hybridherbst
Copy link

Do you want to continue the discussion regarding the --assign-oetf command in the ktx-software repo? I'm confused by Mark and your answer collliding, and especially that without the --assign-oetf linear part the texture still comes out correct but looks much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external Problems or limitations traced back to other tools. package:cli
Projects
None yet
Development

No branches or pull requests

3 participants