ycwn / nvidia-texture-tools

Automatically exported from code.google.com/p/nvidia-texture-tools
Other
0 stars 0 forks source link

Wrong pitch calculation #168

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Open a R8G8B8A8 texture with mips down to 1X1
2. calculate the size for the last(1X1) mipmap
3. the size should be 4, but get 8

What is the expected output? What do you see instead?
the size for the last mipmap(1x1) should be 4, but get 8

Please use labels and text to provide additional information.

in DirectDrawSurface.cpp, it says:

        uint pitch = computePitch(w, header.pf.bitcount, 8); // Asuming 8 bit alignment, which is the same D3DX expects.

but in face, the last arg(8) is in bytes not in bits in computePitch():

inline uint computePitch(uint w, uint bitsize, uint alignment)
{
    return ((w * bitsize +  8 * alignment - 1) / (8 * alignment)) * alignment;
}

Original issue reported on code.google.com by wuyu....@gmail.com on 23 Aug 2011 at 6:49

GoogleCodeExporter commented 9 years ago
according to MSDN's suggestion:
http://msdn.microsoft.com/en-us/library/bb943991(v=vs.85).aspx

For other formats, compute the pitch as:

( width * bits-per-pixel + 7 ) / 8

You divide by 8 for byte alignment.

Original comment by wuyu....@gmail.com on 23 Aug 2011 at 7:39

GoogleCodeExporter commented 9 years ago
Uh, looks like the alignment argument is expected to be in bytes, not in bits, 
so it should be:

computePitch(w, header.pf.bitcount, 1);

I'll add comments to make that more clear.

Original comment by cast...@gmail.com on 26 Aug 2011 at 11:33

GoogleCodeExporter commented 9 years ago
The documentation was wrong as well. Now it correctly indicates that the pitch 
alignment is given in bytes instead of bits:

http://code.google.com/p/nvidia-texture-tools/wiki/ApiDocumentation?ts=131435883
8&updated=ApiDocumentation#Pixel_Format_Conversion

Original comment by cast...@gmail.com on 26 Aug 2011 at 11:41

GoogleCodeExporter commented 9 years ago
Uh, it's more complicated than I thought, some code expected it in bytes, other 
in bits. We have some code in The Witness that expects 1-bit textures to be 
1-bit aligned, so I'll reword the docs again and fix the code the other way 
around.

Original comment by cast...@gmail.com on 26 Aug 2011 at 11:46

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1266.

Original comment by cast...@gmail.com on 26 Aug 2011 at 12:11

GoogleCodeExporter commented 9 years ago
OK, I've checked in some changes. I haven't done much testing, but hopefully 
it's not horribly broken!

Original comment by cast...@gmail.com on 26 Aug 2011 at 12:15

GoogleCodeExporter commented 9 years ago
With revision r1266, using RGB output format along with mipmap generation 
crashes due to buffer overflow. After getting back to r1263, I no longer 
experienced this problem. I suspect this new issue arose due to the 
modifications in the pitch calculation code.

Original comment by pmjo...@gmail.com on 2 Sep 2011 at 11:27

GoogleCodeExporter commented 9 years ago
I just got this change and am getting the same issue.  This is because of the 
zero padding done at the end of each mip. Towards the end of CompressRGB.cpp, 
we have: 
stream.align(compressionOptions.pitchAlignment);
But based on what align() is doing, it looks like it expects byte alignment, 
but the change makes it so it passes in bit alignment. 
It works if I do this: 
stream.align(compressionOptions.pitchAlignment / 8);

Original comment by hallchr...@gmail.com on 13 Sep 2011 at 4:23

GoogleCodeExporter commented 9 years ago
Yeah, I don't know what I was thinking. The pitch alignment must be given in 
bytes, I thought one of our formats expected bit alignment, but I was clearly 
wrong since the bit stream did not support that. Also the DDS headers expect 
the pitch to be specified in bytes. It should all be fixed with my next checkin.

Original comment by cast...@gmail.com on 13 Sep 2011 at 5:03

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1269.

Original comment by cast...@gmail.com on 13 Sep 2011 at 5:08

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1361.

Original comment by cast...@gmail.com on 5 Feb 2013 at 7:35