donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.3k stars 145 forks source link

ktx resize not work as intended #1348

Closed yue4u closed 2 months ago

yue4u commented 2 months ago

Describe the bug

I'm not sure if it's a known issue but resize option in toktx is ignored when using ktx create command since it's --width and --height options are ignored for non-raw texture creation.

https://github.com/KhronosGroup/KTX-Software/blob/7a45c4e6159c0815e4663a529a1e718136a16e39/tools/ktx/command_create.cpp#L452-L457

To Reproduce Steps to reproduce the behavior:

  1. Run
import { Logger, NodeIO } from "@gltf-transform/core";
import { ALL_EXTENSIONS } from "@gltf-transform/extensions";
import { Mode, toktx } from "@gltf-transform/cli";
import fs from "node:fs/promises";

export const LOGGER = new Logger(Logger.Verbosity.DEBUG);

const io = new NodeIO().registerExtensions(ALL_EXTENSIONS).setLogger(LOGGER);

// https://github.com/KhronosGroup/glTF-Sample-Assets/blob/main/Models/Avocado/glTF-Binary/Avocado.glb
const document = await io.read("Avocado.glb");

await document.transform(
  toktx({
    mode: Mode.UASTC,
    resize: [1024, 1024],
  })
);

const glb = await io.writeBinary(document);
await fs.writeFile("./tmp/Avocado-ktx2.glb", glb);
  1. Grab ktx create --generate-mipmap --encode uastc --uastc-quality 2 --zstd 18 --assign-oetf linear --assign-primaries none --format R8G8B8_UNORM --width 1024 --height 1024 --threads 12 /var/folders/.../gltf-transform/foo.png /var/folders/.../gltf-transform/bar.ktx2 and run it directly.

  2. Following logs will be printed

    ktx create warning: Option --width is ignored for non-raw texture creation.
    ktx create warning: Option --height is ignored for non-raw texture creation.

    Also open Avocado-ktx2.glb at gltf.report shows the texture sizes are not changed.

Expected behavior

Textures are resized correctly or at least warnings are produced.

Versions:

Additional context

Related source code https://github.com/donmccurdy/glTF-Transform/blob/b1fee04/packages/cli/src/transforms/toktx.ts#L383-L392

donmccurdy commented 2 months ago

Thanks @yue4u! Proposed fix: