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

Creating an additional option for creating texture arrays instead of deducing from layer count #596

Closed
toomuchvoltage opened this issue Jul 6, 2022 · 7 comments · Fixed by #602

Comments

@toomuchvoltage
Copy link
Contributor

toomuchvoltage commented Jul 6, 2022

Currently toktx seems to create arrayed textures only when layers is specified to be larger than one:

    createInfo.numLayers = options.layers;
    createInfo.isArray = options.layers > 1;

This causes issues when compressed textures are generated in bulk and assigned to various slots on a variable count descriptor set of sampler2DArrays. The issue mainly surfaces on AMD drivers (where the .z component of textureSize() ends up being undefined in the event the combined sampler is backed by a texture2D and not a texture2DArray).

I propose a separate option of --is-array that sets the flag inside KTX metadata to ensure that single layer texture arrays are possible (when potentially provided alongside larger/taller arrayed textures).

@toomuchvoltage toomuchvoltage changed the title Creating an additional option for creating arrays instead of deducing from layer count Creating an additional option for creating texture arrays instead of deducing from layer count Jul 6, 2022
@MarkCallow
Copy link
Collaborator

Oops. --layers should have been used for arrays only. I wonder how many people actually use --layers 1 when not wanting an array. In other words, is it too late to change behavior now? If so, a new option will be needed.

@toomuchvoltage
Copy link
Contributor Author

toomuchvoltage commented Jul 8, 2022

Thank you @MarkCallow for attending to this. Well I guess at this stage we might have to do a wider community poll?
(I can personally attest to being more responsive to the email surveys myself (if that is possible) 🙂)

@toomuchvoltage
Copy link
Contributor Author

Hi @MarkCallow, hope all is well. Just wanted to check-in and see if you've made a decision either way regarding the tool. If so, please let me know so that I can align my local fix towards your preference in advance.

Much appreciate,
Baktash.

@MarkCallow
Copy link
Collaborator

I'm leaning towards using --layers. Its doc would change from

KTX file is for an array texture with number of layers where number > 1.

to

... number >= 1

I sent a survey to the Khronos WG responsible for KTX. I don't have a good place to make a general survey of users. I doubt most would see anything in the Wiki or GitHub questions.

@toomuchvoltage
Copy link
Contributor Author

Perfect, thank you for the update: I'll assume that to be the new behavior. Hopefully, with a new update coming soon... I can just replace my build with yours. Much appreciated!

@MarkCallow
Copy link
Collaborator

I'm implementing this change. What behavior do you prefer when --layers 0 is specified on the command line:

  1. silently create a non-array texture?
  2. warn about possible mistake in option specification and create a non-array texture?
  3. exit with an error?

@ghost
Copy link

ghost commented Jul 24, 2022

Exit with an error, it was probably unintended.

MarkCallow added a commit that referenced this issue Jul 25, 2022
Fixes #596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.
MarkCallow added a commit that referenced this issue Jul 27, 2022
Fixes #596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 21, 2024
Fixes KhronosGroup#596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
Fixes KhronosGroup#596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
Fixes KhronosGroup#596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
Fixes KhronosGroup#596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
KaperD pushed a commit to KaperD/KTX-Software that referenced this issue Feb 22, 2024
Fixes KhronosGroup#596.

Add checks for valid argument values for depth, layers and levels.

Update toktx documentation.
   * Add note about how to disable 2-component normal map conversion.
   * Reformat some of the source.

* Move input_swizzle to scapp.h, support it in ktxsc and move common encode code to scapp.h.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants