Implement loading DDS textures at run-time (reverted)

[marcinn]

Closes godotengine/godot-proposals#5748

Adds:

• Image::load_dds_from_buffer()
• ability to load DDS textures at runtime via Image::load_from_file

The DDS loader implementation is moved from texture loader to image loader.

Backport for 3.x is in progress and will be available soon.

[fire]

I think this is good to review for merge. Hopefully someone can help me out to review. 👍

[Calinou]

For future reference: I've built a release export template with and without this PR, and this appears to be make no difference in binary size at all (not even 1 byte).

[marcinn]

I've compiled template_release and exported my test project. Seems to work fine (dds texture is loaded at runtime).
Could you compare sha of the binaries?

[Calinou]

I've compiled template_release and exported my test project. Seems to work fine (dds texture is loaded at runtime). Could you compare sha of the binaries?

33ca6fbaaed1b38af0395ccd3c28d983e558e48acb2c81875e7d1b8ce1303b98  bin/godot.linuxbsd.template_release.x86_64 (this PR)
c3852aa853d7540ec0b56a325ca7ba729f82dec8efe2e437bf2a3f737dd0c4f8  bin/godot.linuxbsd.template_release.x86_64.vanilla

.rwxr-xr-x   65,502,000 hugo 24 Nov  2:03  godot.linuxbsd.template_release.x86_64 (this PR)
.rwxr-xr-x   65,502,000 hugo 24 Nov  2:03  godot.linuxbsd.template_release.x86_64.vanilla

[marcinn]

Thanks.

65,502,000

Nicely rounded.
Size of my binary is 87530672. We probably used different build parameters.
I've compiled binary with scons platform=linuxbsd tools=no target=template_release bits=64

You can check if it works if you have some time. I'm attaching an example project. The icon path is set by default to /tmp/icon.dds. Without the icon file, or when dds at runtime does not work, you will see a white box. If dds at runtime works as expected, you will see a textured box.

TestRuntimeDDS.zip
icon.zip

[akien-mga]

Amended to fix copyright headers for 2023+.

[Calinou]

Should be good to merge after rebasing.

[marcinn]

Rebased

[marcinn]

Thank you. Could this pr be merged now, please?
If so, please also merge a backport for 3.6 #69101 (it is accepted too)

[clayjohn]

This looks correct to me. I have reviewed the code, but I haven't tested locally.

[YuriSizov]

@marcinn Could you please rebase? This looks well approved now, we can merge it if it's rebased.

[marcinn1]

Rebased

[YuriSizov]

I'm not entirely sure why enums and structs are not class-scoped and are not in the header. That's where they should be. But this was already the case before you moved around relevant code, so I'll merge this as is. Still, some codestyle clean-up as a follow-up PR would be welcome.

[YuriSizov]

Thanks!

[TokisanGames]

Since Godot 3, DDS files have been used natively, without conversion or import files. This is why we use DDS files and have over 800 in our game, because they do not get converted and are used exactly as we made them. Godot 4's DXT conversion from non-DDS files has bugs that we wish to avoid, and we want control over the exact format and algorithm used in DXT conversion.

This PR broke this (confirmed by bisecting). When I loaded up my game in 4.2-dev, it started converting all of my DDS files from a working DXT format to a broken DXT format. Importing didn't complete and hung Godot, requiring a forcequit. Files that have mipmaps now report there are none. Textures are grainy. I have a duplicate of all textures on my drive (as DDS and ctex), and I likely have artifacts and other issues with various textures as they have been converted to a different format I didn't ask for.

It causes issues such as this:
TokisanGames/Terrain3D#190

I also got these error messages loading up the Terrain3d demo.

ERROR: Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
ERROR:ERROR:ERROR:   at: detect_used_channels (core/io/image.cpp:3325)
 Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
ERROR: Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
 Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
   at:  Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
   at:    at: detect_used_channels (core/io/image.cpp:3325)
   at: detect_used_channels (core/io/image.cpp:3325)
detect_used_channels (core/io/image.cpp:3325)
detect_used_channels (core/io/image.cpp:3325)
res://demo/textures/rock028_nrm_rgh.dds: Texture detected as used as a normal map in 3D. Enabling red-green texture com.
res://demo/textures/rock028_nrm_rgh.dds: Texture detected as used as a roughness map in 3D. Enabling roughness limiter .
res://demo/textures/rock028_nrm_rgh.dds: Texture detected as used in 3D. Enabling mipmap generation and setting the tex.
ERROR: Cannot generate mipmaps in compressed or custom image formats.
   at: (core/io/image.cpp:1820)
ERROR: Condition "is_compressed()" is true. Returning: USED_CHANNELS_RGBA
   at: detect_used_channels (core/io/image.cpp:3325)

Please revert this change so DDS files are not converted or modified unless specifically requested.

[clayjohn]

@TokisanGames That sounds very unexpected. This PR didn't touch the importer. So if your files were not importing before, they should not start being imported after this change

That being said. I can confirm that Godot is defaulting to importing dds textures now all of a sudden

Dev3

4.1.1

[akien-mga]

@clayjohn This PR does add ImageLoaderDDS, which handles all files with .dds extension.

[TokisanGames]

Unexpected or not, it was confirmed by bisecting from 4.1 to master. DDS files have never been imported or converted since I started using Godot in 2018. And nor have import files been generated. Now both are happening. Not only are import files generated, the contents of the import files show that my DDS files have been converted to a different format, which is not desired.

[remap]

importer="texture"
type="CompressedTexture2D"
uid="uid://p60g73cyfwcj"
path="res://.godot/imported/ground037_alb_ht.dds-465fdc1deba1c0c06d4b6c6d8226eacf.ctex"
metadata={
"vram_texture": false
}

[deps]

source_file="res://demo/textures/ground037_alb_ht.dds"
dest_files=["res://.godot/imported/ground037_alb_ht.dds-465fdc1deba1c0c06d4b6c6d8226eacf.ctex"]

As for an MRP @marcinn, all you need is any DDS. Load it up in 4.1 and it won't import. Load it up in any version after this commit and it will import/convert. If you want a more practical project, download Terrain3D and look at the demo and it will make all the terrain textures grainy.

[marcinn1]

This PR does add ImageLoaderDDS, which handles all files with .dds extension.
I can confirm that Godot is defaulting to importing dds textures now all of a sudden

That's look like implicite behaviour of Godot (unexpected in context of this PR).
Is possible to force default import settings per file type?

[clayjohn]

Is possible to force default import settings per file type?

@marcinn1 It doesn't look like it. There is the added problem that DDS needs to be imported as a plain resource so that it is available in the export pck. So we can't just filter it out of import completely.

[marcinn1]

So this is the importer which not work as other importers, and it is required for export. Quite unclear / inconsistent.
The goal of this PR was to able load DDS at runtime, but not change importer settings.

As far I understand:

1. The editor changed import behaviour because of registration of ".dds" extension for ImageLoader (!)
2. There is no possibility to force the editor to use "Keep file / No import" by default for particular resource type
3. DDS import was always buggy and wasn't used before ("Keep file / No import" was used by default)
4. I reused this buggy code in runtime DDS loader

Possible solutions:

• revert the PR, drop dds loading, and runtime and leave possibly broken code as it was before
• tell the Editor to use "Keep file / No import" by default for DDS
• or as a workaround: change DDS importer to pass-through/dummy/do-nothing implementation (if possible)

[TokisanGames]

The original proposal was for loading DDS files on the CPU as Images. In Terrain3D we operate predominantly with Images and we recommend all of our users use DDS files only, due to Godot's bugs in DXT conversion. We pull images from the textures, then convert them back into TextureArrays. It does waste cycles, but it is functional and only requires a quick, one-time operation on load. Also export works fine. So while I'd be happy to reduce wasted cycles and load DDS files as Images directly, sorry to say that this PR is not required even for an editor plugin that relies completely on DDS files.

As a result of this PR, Godot currently does not load DDS files. On import, Godot converts the DDS files to CTEX files, and those are loaded. Since DXT is a highly compressed format, it's like opening a jpg and saving it again, going through two lossy conversions. The lower quality derivative is what is loaded, which is a far worse outcome than loading files as Textures and copying the memory to an Image, IMO.

It can be fixed on the user side by manually setting each DDS file to Keep and pressing Reimport on the Import tab, one at a time. Or creating a script that overwrites import files with the following contents. Though I haven't tested export.

[remap]

importer="keep"

We have 800 files in our game, and hundreds of Terrain3D users who have been recommended to use DDS files. But it could be done.

Setting Keep by default for DDS would be great.

[clayjohn]

• revert the PR, drop dds loading, and runtime and leave possibly broken code as it was before
• tell the Editor to use "Keep file / No import" by default for DDS
• or as a workaround: change DDS importer to pass-through/dummy/do-nothing implementation (if possible)

I suggest we do the first option, then explore something like the second option. Ultimately, it makes no sense for the importer to expose any option other than "Keep File". Exposing anything else is just creating additional complexity that only serves to hurt users.

[clayjohn]

@TokisanGames Could you highlight what exactly the bugs you are facing with the current DXT compression? I'm not aware of any bugs currently. Ideally we would fix the compression bugs your team is facing so that you and others can use other image types other than DDS

[TokisanGames]

It's been a while since I looked, and it looks like some are fixed. The ones I had in mind were:

Always using DXT5, fixed #58012

Issues I've had with Image::compress() #79932
The key issue isn't the lack of the compression library on export, I've worked around that. The issue is the wrong auto selection of format and no control over the format I want. I have it working now, without issue, but had to go through a lot of discovery to get there. It's not a robust API.

Artifacts in reflections #57981
Maybe addressed with settings?

I typically do DDS conversion in Gimp and what I've found is even there the default algorithm doesn't look good on reflective surfaces. But they give a wide variety of options and better control over the conversion so I can find a compression algorithm that works well. So I'll maintain the practice of external conversion for that control.

I also need the external tool for channel packing. We're going to build a channel packing tool into Terrain3D, and will probably have to write out PNG, which will be read in as DXT :/ so hopefully that will go smoothly.

[clayjohn]

Thanks for filling me in! I am very interested in improving our image compression workflow. While I can't promise that every barrier your team runs into will be removed, it really helps me prioritize stuff to know what is blocking you.

[TokisanGames]

Thanks for your efforts and interest. We're currently not blocked on image compression at all. We have workarounds or documented processes for everything. The dds change was an unpleasant surprise, but I'm glad @Calinou caught it before 4.2 was released. We are blocked on cowdata limitations though.