Nominom / BCnEncoder.NET

Cross-platform texture encoding libary for .NET. With support for BC1-3/DXT, BC4-5/RGTC and BC6-7/BPTC compression. Outputs files in ktx or dds formats.
The Unlicense
108 stars 16 forks source link

Incorrectly calculating byte size of dds mip in certain cases #25

Closed ptasev closed 3 years ago

ptasev commented 3 years ago

It seems the dds file loader assumes that pitch is always stored in place of the header field pitchOrLinearSize. This fails when linear size is stored since the computation essentially becomes width * height * height which can result in a number larger than int.Max that arrays support.

https://github.com/Nominom/BCnEncoder.NET/blob/master/BCnEnc.Net/Shared/DdsFile.cs#L80

onepiecefreak3 commented 3 years ago

How can this be resolved? Is there some flag that tells the consumer if the content is pitch or linear size?

onepiecefreak3 commented 3 years ago

It seems that dwFlags contains a flag denoting linear size or pitch. I investigate that further.

ptasev commented 3 years ago

An alternative could be to not rely on that field, and instead replace the code with something like this (rely on format to get byte size):

var sizeInBytes = GetByteSize(dxt10Format ? dxt10Header.dxgiFormat : header.ddsPixelFormat.DxgiFormat, width, height);
private static uint GetByteSize(DxgiFormat format, uint width, uint height)
{
    uint sizeInBytes;
    if (format.IsCompressedFormat())
    {
        sizeInBytes = Math.Max(1, (width + 3) / 4) * Math.Max(1, (height + 3) / 4);
        if (format == DxgiFormat.DxgiFormatBc1Unorm ||
            format == DxgiFormat.DxgiFormatBc1UnormSrgb ||
            format == DxgiFormat.DxgiFormatBc1Typeless)
        {
            sizeInBytes *= 8;
        }
        else
        {
            sizeInBytes *= 16;
        }
    }
    else
    {
        sizeInBytes = width * height;
        sizeInBytes = (uint)(sizeInBytes * format.GetByteSize());
    }

    return sizeInBytes;
}
Nominom commented 3 years ago

This is good to know. I'll add a test case for this.

onepiecefreak3 commented 3 years ago

I see. To get this code a bit more performant I replace the division by 4 to pad width and height by & ~3. Resulting in (n + 3) & ~3 to pad on 4.

ptasev commented 3 years ago

It probably doesn't matter too much in this code, but it may be beneficial here:

https://github.com/Nominom/BCnEncoder.NET/blob/master/BCnEnc.Net/Decoder/BcBlockDecoder.cs#L21

Just leave a comment to help anyone reading the code understand what's happening.

onepiecefreak3 commented 3 years ago

True. That would probably help there as well.

onepiecefreak3 commented 3 years ago

As I could test, your alternative code is ok as is and was now added to my pending PR.

onepiecefreak3 commented 3 years ago

@ptasev As this was solved with the last PR and your review of it, can this issue be closed?

ptasev commented 3 years ago

Seems good to me.