Re: [2/8] d3dx9: Check the size of a DDS file in D3DXGetImageInfoFromFileInMemory.
Am Mittwoch, 9. Mai 2012, 23:32:05 schrieb Józef Kucia:
+ *pitch = width * format_desc->bytes_per_pixel; + *size = *pitch * height; Usually surface pitches are either 4 or 8 byte aligned(depending on the API and pool). I recommend to write some tests for this. The most likely problematic formats are D3DFMT_P8, any 16 bit format like R5G6B5 and 24 bit R8G8B8. Maybe you're supposed to honor the pitch_or_linear_size member of the DDSURFACEDESC.
On Thu, May 10, 2012 at 10:32 AM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Am Mittwoch, 9. Mai 2012, 23:32:05 schrieb Józef Kucia:
+ *pitch = width * format_desc->bytes_per_pixel; + *size = *pitch * height; Usually surface pitches are either 4 or 8 byte aligned(depending on the API and pool). I recommend to write some tests for this. The most likely problematic formats are D3DFMT_P8, any 16 bit format like R5G6B5 and 24 bit R8G8B8. Maybe you're supposed to honor the pitch_or_linear_size member of the DDSURFACEDESC.
Surfaces pitches doesn't seem to be 4 or 8 byte aligned in DDS files. MSDN recommends to not use pitch_or_linear_size and gives formulas for computing pitches [1]. When it comes to formats mentioned by you, D3DFMT_P8 doesn't seem to be supported in DDS files. The formula from this patch also works for the dds_16bit file which stores pixels in D3DFMT_X1R5G5B5 format. Besides that, the patches series seems to work pretty well with at least two real applications. On the other hand, writing more tests is a good idea. [1] - http://msdn.microsoft.com/en-us/library/windows/desktop/bb943991%28v=vs.85%2...
Am Donnerstag, 10. Mai 2012, 11:12:06 schrieb Józef Kucia:
Surfaces pitches doesn't seem to be 4 or 8 byte aligned in DDS files. MSDN recommends to not use pitch_or_linear_size and gives formulas for computing pitches [1]. When it comes to formats mentioned by you, D3DFMT_P8 doesn't seem to be supported in DDS files. The formula from this patch also works for the dds_16bit file which stores pixels in D3DFMT_X1R5G5B5 format. Besides that, the patches series seems to work pretty well with at least two real applications. Since I've burned my hands with DDSURFACEDESC2 pitch handling in the past(e.g. bug 21238), I highly recommend to test it. Knowing msdn, the above page makes me less confident, not more :-\
The most likely situation where an error occurs is when a different API or tool(e.g. gdi32 or ddraw) uses a specific pitch and the application developers are not aware of it and just forward the data 1:1. Some test suggestions: *) See if the pitch_or_linearsize member is used at all *) Non-DWORD aligned pitches *) Pitches that are not pixel-size aligned *) pitch < width * byte_per_pixel *) Negative pitches / pitches > 2^31(Remember the LONG vs DWORD) *) A too small linear size for compressed surfaces(e.g. < block size)
On Thu, May 10, 2012 at 8:39 PM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Since I've burned my hands with DDSURFACEDESC2 pitch handling in the past(e.g. bug 21238), I highly recommend to test it. Knowing msdn, the above page makes me less confident, not more :-\
The most likely situation where an error occurs is when a different API or tool(e.g. gdi32 or ddraw) uses a specific pitch and the application developers are not aware of it and just forward the data 1:1.
Some test suggestions:
*) See if the pitch_or_linearsize member is used at all *) Non-DWORD aligned pitches *) Pitches that are not pixel-size aligned *) pitch < width * byte_per_pixel *) Negative pitches / pitches > 2^31(Remember the LONG vs DWORD) *) A too small linear size for compressed surfaces(e.g. < block size)
Thanks for the suggestions. Do you think this patch series can get in before tests for pitch handling? The patch series is big enough and I don't want to make it any bigger. The patch series also makes d3dx9 to pass almost all tests for DDS file format support. Besides that, there are many things left and improvements to be done until the DDS support in d3dx9 will be completed. Amongst things to do the most problematic seems to be DXTn compressor and decompressor. So I think these tests could be added in the next series of patches. What do you think?
Am Donnerstag, 10. Mai 2012, 21:44:56 schrieb Józef Kucia:
Do you think this patch series can get in before tests for pitch handling? The patch series is big enough and I don't want to make it any bigger. That's fine with me if it makes your work easier.
On 10 May 2012 20:39, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Some test suggestions:
*) See if the pitch_or_linearsize member is used at all *) Non-DWORD aligned pitches *) Pitches that are not pixel-size aligned *) pitch < width * byte_per_pixel *) Negative pitches / pitches > 2^31(Remember the LONG vs DWORD) *) A too small linear size for compressed surfaces(e.g. < block size)
If we're testing things anyway, integer overflows are always cute to test for as well, in particular for these kinds of image handling functions where you end up multiplying potentially untrusted width, height and bpp. We probably don't handle that correctly at all.
participants (3)
-
Henri Verbeet -
Józef Kucia -
Stefan Dösinger