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

Images data validation #21

Open
4 tasks
lexaknyazev opened this issue Sep 12, 2017 · 5 comments
Open
4 tasks

Images data validation #21

lexaknyazev opened this issue Sep 12, 2017 · 5 comments

Comments

@lexaknyazev
Copy link
Member

lexaknyazev commented Sep 12, 2017

Right now, Validator checks only that image dimensions are powers-of-two.

Next checks:

  • Detect when number of channels doesn't seem to match material usage (e.g., pure grayscale image used for Metal-Roughness texture).
  • Detect when image defines custom colorspace/gamma/intent.
  • Detect when image uses non-square pixels.
  • Detect EXIF Orientation flag in JPEG images.

Thoughts?

@javagl
Copy link

javagl commented Sep 13, 2017

Regarding the first sentence: There is an implementation note at https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#samplers saying

Non-Power-Of-Two Texture Implementation Note: glTF does not guarantee that a texture's dimensions are a power-of-two.

So the power-of-two test should probably not be done at all...?

I'll look at this and the other issues ASAP in more detail (but am not sure whether I can make helpful contributions)

@lexaknyazev
Copy link
Member Author

So the power-of-two test should probably not be done at all...?

OpenGL ES 2.0 hardware may refuse to render npot textures. I'd argue that this is a reasonable validation warning for runtime format.

@javagl
Copy link

javagl commented Sep 13, 2017

The full implementation note also says something about the conditions under which a texture has to be resized to be POT (maybe I should have quoted this as well, but you're likely more aware of and familiar with this than me anyhow).

I think that an asset that is perfectly valid in terms of the spec should not cause any warnings. The renderers have to be aware of the fact that the textures may be non-POT (and have to resize them anyhow, under certain conditions). But I don't have such a strong opinion here, maybe the warning is justified in order to increase this awareness...

@emackey
Copy link
Member

emackey commented Sep 13, 2017

Some packages have levels below error and warning, such as info, suggestion, and debug. Sometimes, an API is provided to suppress messages below a user-supplied threshold, for less verbose output (for example, only show me errrors and warnings, not info or suggestions).

To me, NPOT textures sound like info level:

Info: Using non-power of 2 textures causes performance implications for some platforms.

Or something to that effect. It shouldn't be a "warning" if the spec allows it, IMHO.

@emackey
Copy link
Member

emackey commented Sep 22, 2017

For what it's worth, VSCode's DiagnosticSeverity includes four values. In descending order of severity they are: Error, Warning, Information, and Hint.

This is used for example in their language server sample to report validation errors back to the text editor. The last one, Hint, does not even place a mark in the text editor itself, it just lists the hint in a separate "Problems" tab with a full list of all four levels of messages. The other three levels do place a squiggle-underline mark below the text in the editor that triggered the validation message, and in the case of Error, the squiggle is bright red.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants