KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
874 stars 229 forks source link

no alpha when PNG has tRNS chunk #356

Closed arpu closed 3 years ago

arpu commented 3 years ago

this issue comes on a user generated glb Model

thx to @zeux for debugging

toktx warning: Expanding paletted image to RGB 8-bit

he points out to this code https://github.com/KhronosGroup/KTX-Software/blob/master/tools/toktx/pngimage.cc#L93

Bildschirmfoto von 2021-01-13 11-49-14 Bildschirmfoto von 2021-01-13 11-43-37

problematic PNG file: brush_Waveform_baseColor

MarkCallow commented 3 years ago

I'm not understanding what the issue is here. Please explain. The code pointed to is only executed if the PNG colortype is RGB. In that case, by my understanding of the PNG spec. there is only transparency if a color key is defined. So the fact that there is no alpha channel unless it is defined is the intended behavior. Are you wanting an all opaque alpha channel to be generated?

zeux commented 3 years ago

The correct reference to the problematic code is https://github.com/KhronosGroup/KTX-Software/blob/master/tools/toktx/pngimage.cc#L102

PNG palette can contain alpha; please refer to http://www.libpng.org/pub/png/book/chapter08.html#png.ch08.div.5.2. The code above assumes that palette doesn't contain alpha, and on the image above alpha is discarded.

(white-on-white is hard to see but Chrome will show the image on a black background if opened in a separate tab)

MarkCallow commented 3 years ago

While investigating this I discovered a more deep-rooted problem. You'll never get alpha in the output texture when there is a tRNS chunk. This is because the lodepng function being used to inspect the PNG file des not read anything but the IHDR chunk. Once that is fixed, the problem initially reported here, of checking for a color key instead of palette transparency for indexed color type, will come into play.

MarkCallow commented 3 years ago

What is the origin of the "problematic" PNG file? I want to include it in the toktx test suite. To do so I need to know the license and attribution.

miguelangelorosario commented 3 years ago

Hey...I am the owner of that model...the issuue with this might be mainly because the geometry( brushstrokes) besides the models are made with Tilt Brush , the sketch is then uploaded from tiltbrush to sketchfab...the shaders from tiltbrush are not kept in the upload , so i have to texture them via sketchfab 3d editor until the model looks cool...in the sketchfab editor in the opacity i can upload images without alpha and set opacity to luminance of the image...now the downloaded converted gltf has that opacity texture with alpha...this might cause some bugging at the end...you can use that flash png to test things but i dont know if thatts a good testing example because of the process how it was converted...

miguelangelorosario commented 3 years ago

this is the model on sketchfab https://sketchfab.com/3d-models/the-ultimate-cern-3d-zoom-in-0bc1922c55bb4b33ac7cb58d889acd8b actually there is a bug with the uploads to sketchfab and the conversion to gltf...textures are mirror flipped ( UV flipped) in the conversion...this is an issue i have pointed out to the dev team from Sketchfab and they are investigating...so for example the background image and the asset textures are uploaded with correct orientation...but on sketchfab the gltf conversion flips them head over...when one downloads the gltf from sketchfab one can find all textures in the folder are flipped...i am pointing this out because anything that you might test with the textureson this model might get erratic results...

MarkCallow commented 3 years ago

Thanks @miguelangelorosario. The problem reported here is definitely a bug in toktx and has nothing to do with how you prepared the texture, except that the end result is an indexed color (paletted) PNG with transparency.

I want to include it in the toktx test suite to test conversion of indexed color textures with transparency. For that purpose the orientation and indeed the image content doesn't matter.

miguelangelorosario commented 3 years ago

yes include it if it helps to test things...