donmccurdy / glTF-Transform

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

Support resizing textures to nearest powers of two #1213

Closed Yason83 closed 11 months ago

Yason83 commented 11 months ago

Dear glTF-Transform Team,

I hope this message finds you well. First and foremost, I would like to extend my gratitude for your continuous efforts in improving the glTF-Transform tool. Your work significantly contributes to the 3D modeling and rendering community.

I am writing to request an enhancement for the resize module. Specifically, I am interested in a feature that checks and corrects textures with non-power-of-two dimensions. The desired functionality would include parameters to resize the texture either upwards, downwards, or to the nearest power-of-two dimension.

For instance, if a texture is sized at 1000x100, the resize operation could be as follows:

Downward to 512x512 Upward to 1024x1024 Nearest to 1024x1024

Additionally, it would be highly beneficial to have an option to make textures square, aiding in overcoming issues like the one discussed in Issue #1007.

Implementing these features would greatly enhance the usability of glTF-Transform for many users who deal with a variety of texture dimensions, especially in complex 3D models.

Thank you for considering this request. I am looking forward to any possibilities of seeing this enhancement in a future update.

donmccurdy commented 11 months ago

Hi @Yason83 – Thanks for your comments, and for the request! I think this is a good idea. There's a similar feature built into the toktx CLI command now, but it isn't generalized for other texture formats yet. Probably the place to add this feature would be the textureCompress function, because resizing implicitly means re-compressing the texture as well.

Perhaps the current "resize" option could be extended with resizePowerOfTwo:

interface TextureCompressOptions {
  ...
  resize?: vec2,
  resizePowerOfTwo?: 'nearest' | 'ceil' | 'floor',
}

Additionally, it would be highly beneficial to have an option to make textures square, aiding in overcoming issues like the one discussed in https://github.com/donmccurdy/glTF-Transform/issues/1007.

Do square textures avoid the issue? It looked to me like the texture in that issue was already square.

Yason83 commented 11 months ago

Do square textures avoid the issue? It looked to me like the texture in that issue was already square.

Please forgive me, not absolving, this is my mistake.

donmccurdy commented 11 months ago

All good! Maybe we'd stick to 'nearest' | 'ceil' | 'floor' as the options for now then.

Yason83 commented 11 months ago

All good! Maybe we'd stick to 'nearest' | 'ceil' | 'floor' as the options for now then.,

Yes its will be perfect!!!

donmccurdy commented 11 months ago

Work in progress:

donmccurdy commented 11 months ago

Merged! I'm currently working toward the v4.0 release and not planning to make a v3.11 release, so it may be a little while before this is generally available. If you'd like to try it out in the meantime, you can install from the @next dist tag. For example:

npm install --global @gltf-transform/cli@next
Yason83 commented 11 months ago

Thank you, the tool works quite well. However, I have noticed a few issues that might need attention. When using the command gltf-transform resize input.glb out.glb --width 512 --height 512 --power-of-two nearest it seems that the parameters --width 512 --height 512 are being ignored. Perhaps it would be beneficial to apply these parameters sequentially or to provide an error message if they are not applicable?

Additionally, I observed an interesting behavior. With the command gltf-transform resize input.glb out.glb --width 512 --height 512 texture named 1900_1030, originally sized at 1900x1030 pixels, changed to 511x277 pixels after applying the command.

I am attaching the file for your reference. input.zip

donmccurdy commented 11 months ago

--width 512 --height 512 --power-of-two nearest

Yes, they're intended to be mutually exclusive and --power-of-two currently overrides --width and --height. Perhaps a warning would help here, there's also some wording in gltf-transform resize --help.

originally sized at 1900x1030 pixels, changed to 511x277 pixels

Hm that's certainly not intended! I see the same issue here, and will take a look.

Note that the intention for --width and --height is to set an outer bound while preserving aspect ratio. So those two options will not result in a 512x512 texture necessarily, but only 512 on the longer side. Since this is a bulk operation that affects textures of potentially many sizes in the file, I felt that was safer.

donmccurdy commented 11 months ago

I've filed a new issue for the 512px vs 511px discrepancy, https://github.com/donmccurdy/glTF-Transform/issues/1225, and opened a PR warning when --width/--height are used in conflict with --power-of-two: https://github.com/donmccurdy/glTF-Transform/pull/1226