DaemonEngine / Daemon

The Dæmon game engine. With some bits of ioq3 and XreaL.
https://unvanquished.net
BSD 3-Clause "New" or "Revised" License
299 stars 61 forks source link

Dæmon fails to load some .crn (cCRNFmtDXT5_AGBR format) files #220

Open illwieckz opened 5 years ago

illwieckz commented 5 years ago

Dæmon is known to fail on some .crn files. Those files follow a common pattern: a few colors (sometime only one flat color), and a small definition (small amount of pixels).

It may be a crunch bug. I mean, a crnlib bug. Crunch is able to convert those files back to tga properly, it may be a bug in the crnlib standalone file, or a bug in the engine, one of them is probably lacking support for some edgy use case.

illwieckz commented 5 years ago

Those are known to fail:

m77-blanc
m77-blanc.jpg
m77-blanc.crn

rust06e
rust06e.png
rust06e.crn

When they are loaded, Dæmon prints this:

Warn: Invalid CRN image: models/mapobjects/kosad/conteneur/rust06e.crn 
Warn: Couldn't find image file for shader models/mapobjects/kosad/conteneur/rust06e
Warn: Invalid CRN image: textures/metro/m77-blanc.crn 
Warn: Couldn't find image file for shader textures/metro/m77-blanc 

Those textures comes from the metro map by KOsAD¹, you can find a prebuild (reproducing the issue) there: map-metro_0+20190804-154825+39a8bc3.dpk

¹ beware: broken google site that downloads .html pages instead of displaying them.

dimhotepus commented 4 years ago

@illwieckz Tried to reproduce, but map pack was not loaded.

Error is smth about missed tremulous map dependency. May be you can create loadable map with such textures to reproduce?

illwieckz commented 4 years ago

Sorry, dependency is there: http://gg.illwieckz.net/dl/unvanquished/pkg/res-tremulous_1.2.1.dpk

illwieckz commented 4 years ago

Can it be related to this? https://github.com/Unity-Technologies/crunch/pull/11

dimhotepus commented 4 years ago

Failed CRN files are in derivative of DXT5 compression format - cCRNFmtDXT5_AGBR, which we doesn't support in the engine (supported ones are cCRNFmtDXT1 | cCRNFmtDXT3 | cCRNFmtDXT5 | cCRNFmtDXT5A | cCRNFmtDXN_XY - https://github.com/DaemonEngine/Daemon/blob/master/src/engine/renderer/tr_image_crn.cpp#L54).

Looks like we need converting such files into supported formats either internally or externally.

dimhotepus commented 4 years ago

Now reasoning about exact problem with CRN image will be easier:

Warn: CRN image 'models/mapobjects/kosad/conteneur/rust06e.crn' has unsupported format 'cCRNFmtDXT5_AGBR'
Warn: Couldn't find image file 'models/mapobjects/kosad/conteneur/rust06e'
Warn: CRN image 'textures/metro/m77-blanc.crn' has unsupported format 'cCRNFmtDXT5_AGBR'
Warn: Couldn't find image file 'textures/metro/m77-blanc'
illwieckz commented 4 years ago

Failed CRN files are in derivative of DXT5 compression format - cCRNFmtDXT5_AGBR, which we doesn't support in the engine (supported ones are cCRNFmtDXT1 | cCRNFmtDXT3 | cCRNFmtDXT5 | cCRNFmtDXT5A | cCRNFmtDXN_XY - https://github.com/DaemonEngine/Daemon/blob/master/src/engine/renderer/tr_image_crn.cpp#L54).

It would be good to support those formats. It's not good if the image converter can select compression preset the engine does not support. The purpose of that image converter is also to select the best compression preset for a given texture, so, the fix would not be to prevent the image compression optimizer to not optimize, but to get the engine to support such formats.

illwieckz commented 4 years ago

@gimhael, would you know how to add support for DXT5/AGBR like you did with DXT3/BC2 before (#86)?

It looks like crunch produces files using this format with small images with low if no details. So we may do CPU-side conversion if it's too complicated to do it the right way.

The purpose of this effort would not be to support the format natively, but to make sure people can use the image conversion tool without having to test produced files one by one after the compression.

Currently we are forced to workaround the issue by hardcoding compression rules in asset repositories, the knowledge of which texture needs such workaround is acquired through trial and error, and I would like to remove such trial and error from the mapping workflow.

Of course if the engine can support the format natively, it would be better. But the priority is to grok the format whatever the way it is implemented.

ghost commented 2 years ago

I admit I've not read the discussion: I'm trying to see if stuff in more than 100+ issues are still actual after all. Could it be possible to update original comment to allow for reproduction of the bug, so that future reviewers would be able to quickly check this is fixed, or still a problem?

illwieckz commented 2 years ago

@bmorel the second comment provides a map that reproduced the issue at the time the issue was created.

So, one can download this map in pkg/, make sure he has no higher version of “metro” map and load the map with /devmap metro and look at console log.

ghost commented 2 years ago

I'll do a deeper read of that soon, then. I'm still browsing all the issues :)

illwieckz commented 1 year ago

See this crunch PR to add an option to crunch to prevent producing such incompatible files:

Anyway, we may still want to get Dæmon being able to read those files if they happen to be produced anyway.