Resize BoxTextured image to 256 so it is PowerOfTwo #137
Conversation
|
Oops didn't add the NPOT example |
|
Keeping the NPOT example is important. I'd even consider to omit the new NPOT example, and instead keep the original BoxTextured one with its NPOT textures: The point is that this is likely one of the first models that implementors will use for testing textures, and the "gotchas" that you mentioned should hit the implementor as early as possible. Otherwise, they could be tempted to say: "Fine, textures are working, next topic..." - and finally ship a renderer that only works with POT textures... But that's just an opinion, let's see what others think. |
|
|
||
| This model uses a Non Power-Of-Two texture with REPEAT mode wrapping. This is an edge case that is technically a valid gltf model, but needs some renderer work to resize the texture before uploading to the GPU. According to the spec: | ||
|
|
||
| Non-Power-Of-Two Texture Implementation Note: glTF does not guarantee that a texture's dimensions are a power-of-two. At runtime, if a texture's width or height is not a power-of-two, the texture needs to be resized so its dimensions are powers-of-two if the sampler the texture references |
There was a problem hiding this comment.
The last statement is incomplete (compare with the spec). It may be better to put a link instead.
|
There's a merge conflict here. |
|
Merge resolved. Any other thoughts on this? #137 (comment) |
|
Up to you all. I think the standard basic box textured should be aPOT
example but I understand their reasoning for wanting to hit all edge cases
for example one. If that's what we want we can close this
…On Mon, Jan 8, 2018 at 8:05 AM Ed Mackey ***@***.***> wrote:
Merge resolved. Any other thoughts on this? #137 (comment)
<#137 (comment)>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#137 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJ1b9hdDzJFNQQbuy8tXix_BKPSHgJdks5tIjzlgaJpZM4QxwKs>
.
|
|
@javagl I think it's more in line with the other examples here, to keep the basic box simple, and call out features like NPOT with separate examples. This way, engine developers can more easily isolate particular features for testing. So, I think we should merge this. |
|
@emackey Sure, as long as there also is a non-POT-example. (Now there is a dedicated one. Otherwise, it could be part of one of the more general texture feature samples) |
|
We will add some tests to glTF-Asset-Generator as a bonus. |
|
Merged. Thanks again @msfeldstein and reviewers. |
The model currently has some issues on mobile as it uses a non power of two texture with a REPEAT wrap mode, which is not supported by mobile gpus. While this is technically a valid gltf, it can cause lots of gotchas for people working on implementing a renderer, so maybe we should keep this example simple, and add a NonPowerOfTwo model for people to specifically be able to test this to implement the edge case