GPUOpen-Tools / compressonator

Tool suite for Texture and 3D Model Compression, Optimization and Analysis using CPUs, GPUs and APUs
1.29k stars 193 forks source link

output file does not properly detect file extension #300

Open milesmj opened 6 months ago

milesmj commented 6 months ago

I just got this to compile on my apple M1 (arm) laptop, but I had to fix this bug first. I was compressing with bc7 to a ktx or dds texture, and it would always end up stored as uncompressed. I dug into the source and found a bug when detecting if the destination file is compressed or not:

cmp_fileio.cpp

std::string CMP_GetJustFileExt(const std::string& sourceFile)
{
    std::string fileExt = "";

#if defined _CMP_CPP17_ || defined _CMP_CPP14_
    sfs::path fp(sourceFile);
    fileExt = fp.extension().string();
#else
    // Alternate Code
    fileExt = sourceFile.substr(sourceFile.find_last_of('.') + 1); // ***** ERROR HERE ***** should not add one. period should be included in result
#endif

    return fileExt;
}

The return from this function expects the '.' to be included, but the alternate code here will strip the '.'. You can see where this function expects the '.' here:

textureio.cpp

bool IsDestinationUnCompressed(const char* fname)
{
    bool        isuncompressed = true;
    std::string file_extension = CMP_GetJustFileExt(fname);

    // tolower
    for (char& c : file_extension)
        c = tolower(c);

    if (file_extension.compare(".dds") == 0)
    {
        isuncompressed = false;
    }
    else if (file_extension.compare(".astc") == 0)
    {
        isuncompressed = false;
    }
<more code>
}
denislevesqueAMD commented 6 months ago

@milesmj Good catch!

That function is used in a few places in less than ideal ways. I think the best solution would be to replace all uses of that function with CMP_GetFileExtension since it allows for more customization of the result, would reduce a lot duplicated code (like always converting the result of CMP_GetJustFileExt to upper or lower case), and fix the bug.