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

DDS loader crash fix #21

Closed
wants to merge 1 commit into from
Closed

DDS loader crash fix #21

wants to merge 1 commit into from

Conversation

NickWaanders
Copy link

Fix for crash when loading a dds file of 3136x3104 with full mip-chain. The calculation to calculate the mip size was different from what two separate dds compressors seem to be using.

Repro case: create a DDS texture of the given resolution, and create a full mip chain for it. Try to load it, and during loading the application will crash because it is reading memory it shouldn't. I used
both TheCompressonator and Paint.NET to export the DDS, both failed to load.

Fix for crash when loading a dds file of 3136x3104 with full mipchain. The calculation to calculate the mip size was different from what two separate dds compressors are using.
@bkaradzic
Copy link
Owner

bkaradzic commented Mar 27, 2019

  1. What about KTX code path?

  2. Does it happen if texture is compressed with texturec?

  3. Which BC causes issue?

@NickWaanders
Copy link
Author

Good questions, I found a few more issues with my pull request because of them. I'm going to look into it a bit more.

  1. I haven't tested ktx, as I don't regularly use this codepath. I will need to test this a bit more.
  2. When a texture is compressed with texturec, it seems to work, though I am not able to load the dds with directxtex, nor Paint.NET. Could be because it saves in DX10 format, need to test more.
  3. This was using BC3, and the texture has a DXT5 pixel format.

The texture I am trying to load is here: https://www.dropbox.com/s/l3xwi5msfo5ahfp/sizetest.dds?dl=0

I'll need to look into the issues above a bit more, so I'm fine if you want to reject this pull request for now. Or you can leave it open, up to you. :)

@bkaradzic
Copy link
Owner

Yeah, I want more testing... But good you attached texture so I can take look at it.

@bkaradzic bkaradzic closed this Mar 27, 2019
@bkaradzic
Copy link
Owner

Btw, this sizetest is just black texture? Or should I expect to see something?

@bkaradzic
Copy link
Owner

Do some programmer art so that when I fix it I can see that it's not skewed or messed up in some other way.

@NickWaanders
Copy link
Author

Here's a folder with a bunch of images:
https://www.dropbox.com/sh/hv7ws4fgoi15r1n/AADg6_4sjZFub6R3KJNBaSh3a?dl=0

To test I copied the files into examples/runtime/textures and run example-06-bump, and replace the filename on line 156: m_textureColor = loadTexture("textures/sizetest.png");

  • sizetest.png --> input image. Now has some lines and text. Loads OK
  • sizetest_compressonator.dds --> dds file generated by the compressonator. Crash on load.
  • sizetest_paintnet.dds --> dds file generated by paint.net (possibly through a dds plugin). Crash on load.
  • sizetest_texturec.dds --> dds file generated by texturec. Loads OK. Though I can't seem to open this file in directxtex, nor paint.net (possibly because of DX10 pixel format and additional header struct?)
  • sizetest_texturec.ktx --> ktx file generated by texturec. Loads OK.

Could it be that the non DX10 format has a different formula for storing mipmaps? My original fix was to not round the width/height to a multiple of 4 at every halving-step, but just do the rounding for each mipmap width/height only. That seems to load the DXT5 format dds's correctly.

@NickWaanders
Copy link
Author

Also, just to give some more contexts on that last remark about making it a multiple of 4, here are each mip size exported by the compressonator, versus the mipsize exported by texturec. Note that starting at mip 6, the resolutions and data size of the mipdata start to diverge because texturec rounds the width and height to a multiple of 4 for each step, and continues with that rounded number, and the compressonator doesn't:
`
Compressonator:
Mip 0, Texture size = 9734144 Bytes, width = 3136 px height = 3104 px
Mip 1, Texture size = 2433536 Bytes, width = 1568 px height = 1552 px
Mip 2, Texture size = 608384 Bytes, width = 784 px height = 776 px
Mip 3, Texture size = 152096 Bytes, width = 392 px height = 388 px
Mip 4, Texture size = 38416 Bytes, width = 196 px height = 194 px
Mip 5, Texture size = 10000 Bytes, width = 98 px height = 97 px
Mip 6, Texture size = 2496 Bytes, width = 49 px height = 48 px
Mip 7, Texture size = 576 Bytes, width = 24 px height = 24 px
Mip 8, Texture size = 144 Bytes, width = 12 px height = 12 px
Mip 9, Texture size = 64 Bytes, width = 6 px height = 6 px
Mip 10, Texture size = 16 Bytes, width = 3 px height = 3 px
Mip 11, Texture size = 16 Bytes, width = 1 px height = 1 px

texturec:
Mip 0, Texture size = 9734144 Bytes, width = 3136 px, height = 3104 px
Mip 1, Texture size = 2433536 Bytes, width = 1568 px, height = 1552 px
Mip 2, Texture size = 608384 Bytes, width = 784 px, height = 776 px
Mip 3, Texture size = 152096 Bytes, width = 392 px, height = 388 px
Mip 4, Texture size = 38416 Bytes, width = 196 px, height = 196 px
Mip 5, Texture size = 10000 Bytes, width = 100 px, height = 100 px
Mip 6, Texture size = 2704 Bytes, width = 52 px, height = 52 px
Mip 7, Texture size = 784 Bytes, width = 28 px, height = 28 px
Mip 8, Texture size = 256 Bytes, width = 16 px, height = 16 px
Mip 9, Texture size = 64 Bytes, width = 8 px, height = 8 px
Mip 10, Texture size = 16 Bytes, width = 4 px, height = 4 px
Mip 11, Texture size = 16 Bytes, width = 4 px, height = 4 px
`

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

Successfully merging this pull request may close these issues.

2 participants