Missing support for DDS RGB formats

[lmichaelis]

Currently, the DDS codec only supports the DXT compression format:

https://github.com/Try/Tempest/blob/88865780a3798912b1b79294c12bd75413f58b8f/Engine/formats/image/pixmapcodecdds.cpp#L35-L57

This becomes an issue when trying to load RGB from DDS files since processing just fails. In Try/OpenGothic#271 this leads to the wrong image being loaded.

A solution specifically for the OpenGothic issue (apart from implementing support for this format) would be to allow the creation of a Pixmap from raw bytes. This would require adding more formats (like R5G6B5 and BGR8) to Pixmap::Format.

[Try]

Hi, @lmichaelis!

Is it related ZTEXFMT_R5G6B5 and such, right? Probably it would be better to upcast those formats to RGBA8, since pretty much no Vulkan drivers do support for BGR8.

[lmichaelis]

It is. I can upcast it but there is no way of providing raw bytes to Pixmap, right? That would be the most efficient way of doing things.

[Try]

providing raw bytes to Pixmap, right?

Something like that, should do:

pm = Pixmap(width,height, Pixmap::Format::DXT1);
std::memcpy(pm.data(), rawBytes, numBytes);

[lmichaelis]

Hm I guess that works. Seems kinda unsafe though. If the data member changes its size, my code will break. It would probably better to have a proper constructor.

[Try]

It wont - Pixmap allocates it's storage only once, at creation time.

From constructor based interface:

• in theory taking storage as movable, like Pixmap(w,h,frm,std::move(data)) can be nice, but then Pixmap will have to track allocation source. (Now it assumes std::malloc)
• Pixmap(w,h,frm,const void* data,size_t dataSz) - will work, but only as shortcut for memcpy

[lmichaelis]

Well, yes, that is the case right now. But if you ever decide to change when or how data is allocated, doing it like this will cause a bug (probably a buffer overflow) which can not be detected by the compiler.

Ultimately it's your choice but that's my 2 cents ;)

[Try]

@lmichaelis do you remember how relevant this issue still is?

My current problem is: squish CMake file is broken for arm64, and ideally we need to remove it from dependency chain. (or fix CMakeLists.txt)

After looking into formats:

• tex_dxt* formats are handled fine with engine tools
• tex_B8G8R8, tex_p8 are problematic, yet conversion code can be moved to OpenGothic side

[lmichaelis]

Hm I'm not sure. I think there was an issue at some point bit since the textures are loaded correctly now in OpenGothic, I don't think is particularly important any more.

How are you handling DXT formats, especially on Vulkan? Are you just using an extension? Otherwise you'd still need to decode that, no?

Phoenix depends on Squish too so if that is a problem here, phoenix would be the same. I already have a fork of libsquish here so you could use that and we can update the CMakeLists there if you'd like :)

[Try]

Are you just using an extension?

Extension by default; squish-tempest otherwise(it's same squish, but with simplifyed build options)

I already have a fork of libsquish

Oh, that's great, can you please fix this part in cmake:

IF (CMAKE_GENERATOR STREQUAL "Xcode")
    SET(CMAKE_OSX_ARCHITECTURES "i386;ppc")
ELSE (CMAKE_GENERATOR STREQUAL "Xcode")
    IF (BUILD_SQUISH_WITH_SSE2 AND NOT WIN32)
        ADD_DEFINITIONS(-DSQUISH_USE_SSE=2 -msse2)
    ENDIF (BUILD_SQUISH_WITH_SSE2 AND NOT WIN32)
    IF (BUILD_SQUISH_WITH_ALTIVEC AND NOT WIN32)
        ADD_DEFINITIONS(-DSQUISH_USE_ALTIVEC=1 -maltivec)
    ENDIF (BUILD_SQUISH_WITH_ALTIVEC AND NOT WIN32)
ENDIF (CMAKE_GENERATOR STREQUAL "Xcode")

to

IF (NOT CMAKE_GENERATOR STREQUAL "Xcode")
    IF (BUILD_SQUISH_WITH_SSE2 AND NOT WIN32)
        ADD_DEFINITIONS(-DSQUISH_USE_SSE=2 -msse2)
    ENDIF (BUILD_SQUISH_WITH_SSE2 AND NOT WIN32)
    IF (BUILD_SQUISH_WITH_ALTIVEC AND NOT WIN32)
        ADD_DEFINITIONS(-DSQUISH_USE_ALTIVEC=1 -maltivec)
    ENDIF (BUILD_SQUISH_WITH_ALTIVEC AND NOT WIN32)
ENDIF (CMAKE_GENERATOR STREQUAL "Xcode")

[Try]

Fixed by phoenix update, thanks @lmichaelis :)