giordi91 commented 5 years ago

Good evening, I am trying to generate a normal map using Compressonator but finding some issues, the normal maps show some artifacts, although they work just fine from the GUI here a comparison:

From SDK normalWrong From GUI normalCorrect

The options seem to be the same and both are recognized as BC3_TYPELSS if I open them with render doc here the Compressonator gui config: normalsConfig

Here the code I use to generate the normals:

  int w;
  int h;
  unsigned char *data = getTextureDataFromFile(path, &w, &h);

  CMP_Texture srcTex;
  srcTex.dwSize = sizeof(CMP_Texture);
  srcTex.dwWidth = w;
  srcTex.dwHeight = h;
  srcTex.dwPitch = w * 4;
  srcTex.dwDataSize = w * h * sizeof(unsigned char) * 4;
  srcTex.format = CMP_FORMAT_RGBA_8888;
  srcTex.pData = data;

  CMP_FORMAT format = CMP_FORMAT_DXT1;
  getFormatFromString(formatString, format);

  //===================================
  // Initialize Compressed Destination
  //===================================
  CMP_Texture destTexture;
  destTexture.dwSize = sizeof(destTexture);
  destTexture.dwWidth = srcTex.dwWidth;
  destTexture.dwHeight = srcTex.dwHeight;
  destTexture.dwPitch = 0;
  destTexture.format = format;
  destTexture.dwDataSize = CMP_CalculateBufferSize(&destTexture);
  destTexture.pData = (CMP_BYTE *)malloc(destTexture.dwDataSize);

  //==========================
  // Set Compression Options
  //==========================
  CMP_CompressOptions options = {0};
  options.dwSize = sizeof(options);
  options.fquality = 0.05f;
  options.dwnumThreads = 8;

  CMP_ERROR cmp_status =
      CMP_ConvertTexture(&srcTex, &destTexture, &options, nullptr, NULL, NULL);
  if (cmp_status != CMP_OK) {
    free(srcTex.pData);
    free(destTexture.pData);
    SE_CORE_ERROR("Compression returned an error {0}", cmp_status);
    return false;
  }

I can confirm that the format gets correctly set to CMP_FORMAT_DXT5.

Any idea on why that might be the case? Please find here the two textures: https://1drv.ms/f/s!Ai0n7iKmKMz0gbUQlxM-LeP_FMHCjA

NPCompress commented 5 years ago

@giordi91 Thanks for the detailed information. The issue is currently under investigation.

NPCompress commented 5 years ago

@giordi91 The sample leftShoulderNormalWrong.dds is been compressed using BC1 instead of BC3 (DXT5) You need to check that destTexture.format = format; is correct and not defaulting to DXT1 Try using CMP_FORMAT_BC3.

We will continue to review this issue as your image shown for "From SDK" is different from the sample provided, which shows a scaled up image with artifacts.

NPCompress commented 5 years ago

@giordi91

The following test was performed and it cannot reproduce the issue you have!,

SDK Example code supplied in Compressonator 3.1.4064\examples\example1\SDK_Example1.sln was built and a DDS BGRA sample source was used with the following command line: (please note that in future releases v3.2 and above, Codecs DXT5 and less will expect image sources as RGBA file formats)

Example1.exe leftShoulderNormal_BGRA.dds leftShoulderNormal_dxt5.dds DXT5 0.05

It produced the attached results

leftShoulderNormal.zip

giordi91 commented 5 years ago

I am 100% sure I am using DXT5, I checked in the debugger right before compressing: visualStudio

I am also sure the data I am loading is RGBA since I am specifying that to stbi_image:

 unsigned char *cpu_data =
      stbi_load(path, outWidth, outHeight, 0, STBI_rgb_alpha);

Interesting enough I don't seem to see a difference if in the resulting image if I set "CMP_FORMAT_RGBA_8888" or "CMP_FORMAT_ARGB_8888" I get the same result. Weirdly enough.

To be sure I have hardcoded the out format to DXT5. I also tried with BC3 and got the result you can find attached. DXT5: blockyDXT5

BC3: blockyBC3

I will have a look again at the sample, but I am 90% sure that is where I got the code from. I might also try to modify the sample to read from STBI_IMage from a bmp, that is what I am doing.

Here the link of the images, including a zoom out on the blocky: https://1drv.ms/f/s!Ai0n7iKmKMz0gbd1v_bo6DKmV0PExw

Here below the full code if might be more of use:

#include "processTexture.h"

#define STB_IMAGE_IMPLEMENTATION
#include "stb/stb_image.h"

#include "DirectXTex/DirectXTex.h"

#include "Compressonator/Header/Compressonator.h"
#include "DDS_Helpers.h"
#include "SirEngine/log.h"

#include <unordered_map>

const std::unordered_map<std::string, CMP_FORMAT> STRING_TO_FORMAT{
    {"DXT1", CMP_FORMAT_DXT1},
    {"DXT3", CMP_FORMAT_DXT3},
    {"DXT5", CMP_FORMAT_DXT5},
    {"DXT7", CMP_FORMAT_BC7}};

const std::unordered_map<std::string, DXGI_FORMAT> STRING_TO_DXGI_FORMAT{
    {"DXT1", DXGI_FORMAT_BC1_UNORM},
    {"DXT3", DXGI_FORMAT_BC3_UNORM},
    {"DXT5", DXGI_FORMAT_BC3_UNORM},
    {"DXT7", DXGI_FORMAT_BC7_UNORM}};
const std::unordered_map<std::string, DXGI_FORMAT> STRING_TO_DXGI_FORMAT_GAMMA{
    {"DXT1", DXGI_FORMAT_BC1_UNORM_SRGB},
    {"DXT3", DXGI_FORMAT_BC3_UNORM_SRGB},
    {"DXT7", DXGI_FORMAT_BC7_UNORM_SRGB}};

inline unsigned char *getTextureDataFromFile(const char *path, int *outWidth,
                                             int *outHeight) {
  unsigned char *cpu_data =
      stbi_load(path, outWidth, outHeight, 0, STBI_rgb_alpha);
  if (cpu_data == nullptr) {
    SE_CORE_ERROR("Could not read image: {0},path");
    return nullptr;
  }
  return cpu_data;
}

void getFormatFromString(const std::string &formatString,
                         CMP_FORMAT &outFormat) {
  auto found = STRING_TO_FORMAT.find(formatString);
  if (found != STRING_TO_FORMAT.end()) {
    outFormat = found->second;
  }
}

bool processTextureFile(const char *path, const char *outPath,
                         const std::string &formatString, bool gamma) {
  int w;
  int h;
  unsigned char *data = getTextureDataFromFile(path, &w, &h);

  CMP_Texture srcTex;
  srcTex.dwSize = sizeof(CMP_Texture);
  srcTex.dwWidth = w;
  srcTex.dwHeight = h;
  srcTex.dwPitch = w * 4;
  srcTex.dwDataSize = w * h * sizeof(unsigned char) * 4;
  srcTex.format = CMP_FORMAT_RGBA_8888;
  srcTex.pData = data;

  CMP_FORMAT format = CMP_FORMAT_BC3;
  getFormatFromString(formatString, format);

  //===================================
  // Initialize Compressed Destination
  //===================================
  CMP_Texture destTexture{};
  destTexture.dwSize = sizeof(destTexture);
  destTexture.dwWidth = srcTex.dwWidth;
  destTexture.dwHeight = srcTex.dwHeight;
  destTexture.dwPitch = 0;
  // destTexture.format     = destFormat;
  destTexture.format = format;
  destTexture.dwDataSize = CMP_CalculateBufferSize(&destTexture);
  destTexture.pData = (CMP_BYTE *)malloc(destTexture.dwDataSize);

  //==========================
  // Set Compression Options
  //==========================
  CMP_CompressOptions options = {0};
  options.dwSize = sizeof(options);
  options.fquality = 0.05f;
  options.dwnumThreads = 8;

  CMP_ERROR cmp_status =
      CMP_ConvertTexture(&srcTex, &destTexture, &options, nullptr, NULL, NULL);
  if (cmp_status != CMP_OK) {
    free(srcTex.pData);
    free(destTexture.pData);
    SE_CORE_ERROR("Compression returned an error {0}", cmp_status);
    return false;
  }

  //==========================
  // Save Compressed Texture
  //==========================

  if (cmp_status == CMP_OK) {
    // SaveDDSFile(outPath, destTexture);

    // lets find the right format to use
    if (gamma && formatString == "DXT5") {
      free(srcTex.pData);
      free(destTexture.pData);
      SE_CORE_ERROR("Error in saving texture, cannot use DXT5 and gamma: {0}",
                    path);
      return false;
    }

    DXGI_FORMAT outFormat;

    if (gamma) {
      auto found = STRING_TO_DXGI_FORMAT_GAMMA.find(formatString);
      if (found == STRING_TO_DXGI_FORMAT_GAMMA.end()) {
        free(srcTex.pData);
        free(destTexture.pData);
        SE_CORE_ERROR(
            "Error in saving texture, format not supported with gamma {0}",
            formatString);
        return false;
      }
      outFormat = found->second;

    } else {
      auto found = STRING_TO_DXGI_FORMAT.find(formatString);
      if (found == STRING_TO_DXGI_FORMAT.end()) {
        free(srcTex.pData);
        free(destTexture.pData);
        SE_CORE_ERROR("Error in saving texture, format not supported {0}",
                      formatString);
        return false;
      }
      outFormat = found->second;
    }

    DirectX::Image img;
    img.width = destTexture.dwWidth;
    img.height = destTexture.dwHeight;

    size_t rowPitch;
    size_t slicePitch;
    DirectX::ComputePitch(outFormat, img.width, img.height,
                          rowPitch, slicePitch);
    img.rowPitch = rowPitch;
    img.slicePitch = slicePitch;
    img.pixels = destTexture.pData;
    img.format = outFormat;
    DWORD flags = 0;
    std::string outPathS{outPath};
    std::wstring outPathW{outPathS.begin(), outPathS.end()};
    HRESULT res = DirectX::SaveToDDSFile(img, flags, outPathW.c_str());

    free(srcTex.pData);
    free(destTexture.pData);
  }
  return true;
}

giordi91 commented 5 years ago

On the other end, color images seem perfectly fine, so I am inclined to exclude issues in saving the image.

NPCompress commented 5 years ago

@giordi91

A running application using your code sample has been setup.

Note on your sample: unsigned char *data = getTextureDataFromFile() loads RGBA data. when using v3.1 SDK release binaries, srcTex.pData should be BGRA , it has been corrected in the master branch when using DXTn <= 5, but the CMP_FORMAT_RGBA_8888; vs CMP_FORMAT_BGRA_8888 in CMP_Texture is still been ignored and it will be fixed for v3.2 release.

There seems to be no issues with your code sample. The original source samples are needed to further the analysis.

giordi91 commented 5 years ago

Thank you for getting back to me, so you are saying, for now, BGRA will work on DXTn <=5 and RGBA won't work until 3.2 hits release right? What about DXT6-7? Will it still require BGRA? I will write a quick swizzle from RGBA -> BGRA and see if that fixes it.

Do you need anything else from me to help in your analysis? I am not sure if I have to send something over from reading your last sentence.

Thanks a lot for the help!

EDIT: I tried with all sort of swizzles that does not change the end result, it is still blocky although the dominant channel changes. going from RGBA to BGRA the dominant color becomes red, but still blocky.

NPCompress commented 5 years ago

@giordi91 BGRA will work on DXTn <=5 and RGBA won't work until 3.2 hits release right? Yes, if you can't wait for the release just build the current master source it has the swizzle fix.

What about DXT6-7? Will it still require BGRA? No, those work with RGBA

Do you need anything else from me to help in your analysis? Yes, please post the original left hand normal image for us to debug with.

giordi91 commented 5 years ago

Here it is the original texture: https://1drv.ms/u/s!Ai0n7iKmKMz0gbgnvq6rMt-CbtFmLg

Thank you

NPCompress commented 5 years ago

@giordi91 Thanks for the sample image. Attached is the example code (modified source) that generated a dxt5.dds file in an attached zip file. (issue67.zip with exe debug_md x64 build and image sample), this code does not show any major artifacts for the sample leftShoulderNormal.bmp

define STB_IMAGE_IMPLEMENTATION

include "stb/stb_image.h"

include "DirectXTex/DirectXTex.h"

include "Compressonator.h"

include "DDS_Helpers.h"

include

const std::unordered_map<std::string, CMP_FORMAT> STRING_TO_FORMAT{ {"DXT1", CMP_FORMAT_DXT1}, {"DXT3", CMP_FORMAT_DXT3}, {"DXT5", CMP_FORMAT_DXT5}, {"DXT7", CMP_FORMAT_BC7}};

const std::unordered_map<std::string, DXGI_FORMAT> STRING_TO_DXGI_FORMAT{ {"DXT1", DXGI_FORMAT_BC1_UNORM}, {"DXT3", DXGI_FORMAT_BC3_UNORM}, {"DXT5", DXGI_FORMAT_BC3_UNORM}, {"DXT7", DXGI_FORMAT_BC7_UNORM}}; const std::unordered_map<std::string, DXGI_FORMAT> STRING_TO_DXGI_FORMAT_GAMMA{ {"DXT1", DXGI_FORMAT_BC1_UNORM_SRGB}, {"DXT3", DXGI_FORMAT_BC3_UNORM_SRGB}, {"DXT7", DXGI_FORMAT_BC7_UNORM_SRGB}};

inline unsigned char getTextureDataFromFile(const char path, int outWidth, int outHeight) { unsigned char *cpu_data = stbi_load(path, outWidth, outHeight, 0, STBI_rgb_alpha); if (cpu_data == nullptr) { printf("Could not read image: %s",path); return nullptr; } return cpu_data; }

void getFormatFromString(const std::string &formatString, CMP_FORMAT &outFormat) { auto found = STRING_TO_FORMAT.find(formatString); if (found != STRING_TO_FORMAT.end()) { outFormat = found->second; } }

void RGBA_to_BGRA( CMP_Texture texture) { unsigned char pixels = texture.pData; unsigned char pix; for (int i=0; i < texture.dwDataSize; i+=4) { pix = pixels; pixels = (pixels+2); *(pixels+2) = pix; pixels += 4; } }

bool processTextureFile(const char path, const char outPath, const std::string &formatString, bool gamma) { int w; int h;

// loads RGBA data unsigned char *data = getTextureDataFromFile(path, &w, &h);

CMP_Texture srcTex = {0}; srcTex.dwSize = sizeof(CMP_Texture); srcTex.dwWidth = w; srcTex.dwHeight = h; srcTex.dwPitch = w 4; srcTex.dwDataSize = w h sizeof(unsigned char) 4; srcTex.format = CMP_FORMAT_RGBA_8888; srcTex.pData = data;

RGBA_to_BGRA(srcTex); // CMP_v3.1 and lower requires BGRA regardless of srcTex.format setting for DXT5 and lower!

CMP_FORMAT format = CMP_FORMAT_BC3; getFormatFromString(formatString, format);

//=================================== // Initialize Compressed Destination //=================================== CMP_Texture destTexture{}; destTexture.dwSize = sizeof(destTexture); destTexture.dwWidth = srcTex.dwWidth; destTexture.dwHeight = srcTex.dwHeight; destTexture.dwPitch = 0; // destTexture.format = destFormat; destTexture.format = format; destTexture.dwDataSize = CMP_CalculateBufferSize(&destTexture); destTexture.pData = (CMP_BYTE *)malloc(destTexture.dwDataSize);

//========================== // Set Compression Options //========================== CMP_CompressOptions options = {0}; options.dwSize = sizeof(options); options.fquality = 0.05f; options.dwnumThreads = 8;

CMP_ERROR cmp_status = CMP_ConvertTexture(&srcTex, &destTexture, &options, nullptr, NULL, NULL); if (cmp_status != CMP_OK) { free(srcTex.pData); free(destTexture.pData); printf("Compression returned an error %d", cmp_status); return false; }

//========================== // Save Compressed Texture //==========================

if (cmp_status == CMP_OK) {

// lets find the right format to use
if (gamma && formatString == "DXT5") {
  free(srcTex.pData);
  free(destTexture.pData);
  printf("Error in saving texture, cannot use DXT5 and gamma: %s",
                path);
  return false;
}

DXGI_FORMAT outFormat;

if (gamma) {
  auto found = STRING_TO_DXGI_FORMAT_GAMMA.find(formatString);
  if (found == STRING_TO_DXGI_FORMAT_GAMMA.end()) {
    free(srcTex.pData);
    free(destTexture.pData);
    printf("Error in saving texture, format not supported with gamma %s",formatString.c_str());
    return false;
  }
  outFormat = found->second;

} else {
  auto found = STRING_TO_DXGI_FORMAT.find(formatString);
  if (found == STRING_TO_DXGI_FORMAT.end()) {
    free(srcTex.pData);
    free(destTexture.pData);
    printf("Error in saving texture, format not supported %s",formatString.c_str());
    return false;
  }
  outFormat = found->second;
}

DirectX::Image img;
img.width = destTexture.dwWidth;
img.height = destTexture.dwHeight;

size_t rowPitch;
size_t slicePitch;
DirectX::ComputePitch(outFormat, img.width, img.height,rowPitch, slicePitch);
img.rowPitch = rowPitch;
img.slicePitch = slicePitch;
img.pixels = destTexture.pData;
img.format = outFormat;
DWORD flags = 0;
std::string outPathS{outPath};
std::wstring outPathW{outPathS.begin(), outPathS.end()};
HRESULT res = DirectX::SaveToDDSFile(img, flags, outPathW.c_str());

free(srcTex.pData);
free(destTexture.pData);

} return true; }

int main(int argc, const char* argv[]) { processTextureFile("leftShoulderNormal.bmp","dxt5.dds","DXT5",false); return(0); }

Please note (RGBA_to_BGRA(srcTex);) is not checking DXTC format and should not be applied on BC7

issue67.zip

giordi91 commented 5 years ago

Thank you so the only issue ends up to be the BGRA vs RGBA? I am going to test this asap since I am out of town at the moment.

giordi91 commented 5 years ago

@NPCompress I have tried both manually to modify my code and to copy paste your entire code (except main) and this is the result I got: updateDXT5

Although I am able to run your executable and get the correct result. I am building the library manually using cmake, the only thing I can think of is if I am somehow building it wrong? Can you think on top of your head anything that might affect the quality of the texture?

Left is the result from the SDK, right is a result from the GUI (same config as earlier, DXT5 format everything else default). The color appears to be swapped, as it still expects/uses RGBA, not BGRA, and you can still see the blocks artifacts in the

NPCompress commented 5 years ago

@giordi91 Since your building the code from source, you do not need to swizzle the channels as the fix is in the GitHub master source.

It's still puzzling why you see quality differances from building the library and running the GUI. The GUI essentially uses the library to compress the textures.

giordi91 commented 5 years ago

@NPCompress I am not building from this repo at this point in time, I made a fork in January to which I committed my CMake build file. I just made a dummy application with your code, if I link to the installed SDK version I get your result if I link to my built version of the SDK I get the red/blocky result.

In the end, the blockiness seems to be a build issue on my side of compressonator.lib. I will look into that to figure out what is going on there. I can close this issue or keep it open if you want to know what the build issue is.

giordi91 commented 5 years ago

@NPCompress in order to debug this, I compiled the lib using the provided visual studio solution and I am still seeing the same problem, two things I had to change, first I changed MTd to MDd second I am using Visual Studio 2017 so I had to update the project (v141).

I will see if I can install VS2015 and re-try everything to see if it is an issue with 2015 vs 2017.

giordi91 commented 5 years ago

I installed vs 2015 on another machine I have, compiled the vs solution provided and linked against the test application you have posted. Still get the blocky issue, please find attached the lib: https://1drv.ms/f/s!Ai0n7iKmKMz0gbopR3tFzZLwUil7Xw

This lib has been built with the provided visual studio solution, not my custom CMake build, literally:

clone
open in vs 2015
build
link to test app
run
blocky result

If I link instead against the pre-compiled lib coming from the SDK installer all works fine. I have almost run out of ideas, the last thing I will do will be to also compile the test application with vs 2015 and see if anything changes.

I suppose the visual studio solution is the same you use for your internal development?

NPCompress commented 5 years ago

@giordi91 Puzzling that you still have an issue with this. !!

For the posted sample source, VS2015 was used for the test along with the release version v3.1.4064 SDK libs. When you cloned did you use the master source or the source.zip of the release.?

"I suppose the visual studio solution is the same you use for your internal development?" Yes: for internal development we use VS2015.

giordi91 commented 5 years ago

The one I linked above was from my forked copy of Compressonator, which was from January or so. I am now trying to do a clean clone from master build using VS solution provided and link to the test application. I will get back to you in a little

giordi91 commented 5 years ago

Ok,I have run all the possible tests I could think of... I still get the blocky issue. Pleaes find here attached the test solution I have used: https://1drv.ms/f/s!Ai0n7iKmKMz0gb5MZ3-yMg4s7mnm-Q

you just need to copy the image in the same path of the executable to run it. The solution is setup in debug only and should compile out of the box. I have provided two libraries :

Compressonator_MDdFromMaster.lib : which is just a pure clone, build from provided solution on VS 2015
Compressonator_MDFromSDKInstall.lib is the one copied from compressonator SKD install more specifically Compressonator 3.1.4064.

If you link the same test application against the lib provided by the SDK it works. If you recompile against the one compiled from master it gets blocky.

I have tried to compile both lib and test app with vs 2015, compile only the lib with 2015 and test app with 2017 and finally full 2017, I still get the same issue. The visual studio 2015 version I am using is configured like this:

This is my vs 2015 version:

The reason I am writing all of this is that my idea was a possible disparity between vs2015 and vs2017. It seems not to be the case, as of now I have run out of ideas. I am not sure how to reproduce this any further. Seems to happen no matter what machine I build on. Can you build on any other machine or different version of visual studio? I really wonder what is going on there.

NPCompress commented 5 years ago

@giordi91 Thanks for the sample project files for debugging.

The following tests will be performed on my dev system using VS2015.

Baseline the issue using your sample pre-built libs: Compressonator_MDFromSDKInstall.lib: Your test shows no issues
Compressonator_MDdFromMaster.lib: Your test shows issues

Next step: (1) Rebuild the SDK v3.1.4064 lib using the published release sources: https://github.com/GPUOpen-Tools/Compressonator/archive/v3.1.4064.zip (2) Rebuild the libs using the master branch source

A. Validate that the issues still remain between the two sources. B. if issues are repoduced, then determine the root cause of issue C. if no issues then cause might be an external lib issue and or build setups

Will get back to you soon on the results

NPCompress commented 5 years ago

@giordi91

The baseline test is now completed.

The issue was reproduced from the supplied SDK binaries using your project file.

The issue was not reproduced when building from source: https://github.com/GPUOpen-Tools/Compressonator/archive/v3.1.4064.zip (works)

The issue is reproduced using the Master source (block artifacts)

The root cause is been investigated and the fix will be posted in master source. In the meantime, I would suggest that you build your projects using the v3.1.4064 lib sources link above, and enable the RGBA_to_BGRA() in your sample.

DXT5 No issues when libs built from v3.1.4064 lib source

DXT5 Blocking artifact seen from the master source build

giordi91 commented 5 years ago

Amazing!! I am glad we finally managed to reproduce on both sides! If there is anything else I can do to help please let me know.

M

GPUOpen-Tools / compressonator

Issue in generating normal maps from SDK #67

define STB_IMAGE_IMPLEMENTATION

include "stb/stb_image.h"

include "DirectXTex/DirectXTex.h"

include "Compressonator.h"

include "DDS_Helpers.h"

include