Skip to content

Support all formats, cubemaps and arrays in DdsImporter, make it more robust#67

Merged
mosra merged 15 commits into
masterfrom
dds-dxgi-compressed
May 31, 2022
Merged

Support all formats, cubemaps and arrays in DdsImporter, make it more robust#67
mosra merged 15 commits into
masterfrom
dds-dxgi-compressed

Conversation

@mosra
Copy link
Copy Markdown
Owner

@mosra mosra commented Sep 22, 2019

This was all fun and games until I tried to open real-world files and realized it doesn't work at all. What needs to be done:

  • Turn the switch into a table -- 401ea77
  • GLI supports ASTC formats in DXGI since 2015, how standard is that?! g-truc/gli@0ef1b27 ... apparently it was "added by microsoft and then removed again"? haha https://forums.developer.nvidia.com/t/nv-tt-exporter-astc-compression/122477
    • There's an ASTC DDS file at https://zenhax.com/viewtopic.php?t=8684 but it spectacularly fails to load with current code. Try to get NVidia Texture Tools Exporter somehow (needs an account) and make my own files for testing.
  • Reduce existing excessive tests a bit (gives a false impression that it's tested A LOT, while not really) -- a64351a
    • Test also files with incomplete compressed blocks -- 3a9f67b
    • Test cube maps and arrays! and cube map arrays!
  • Better handling of short files -- d961772
  • Actually cover RGBA, BGRA and greyscale handling with tests -- bfb8930, 28a2fa3
  • Support DXT10 swizzled formats -- 0be45f3
  • Support depth/stencil formats as we have them in PixelFormat now -- 2e71548
  • Support remaining compressed formats -- a477af5
    • crafting proper test files in random compression types (especially varying compressed block size / block data size), because that's currently totally untested -- using an external tool such as AMD Compressonator
  • Y-flip for uncompressed data, warning for compressed. All other plugins do this, this one somehow never did. The horror!
    • Does DDS have any way to describe origin/coordinate system? I don't think so. No.
    • Provide an assumeYUp setting to not flip / hide the warning
  • building up a "knowledge base" table with compressed block sizes and data sizes for each generic format we have directly in magnum -- mosra/magnum@75bfd41
  • reworking CompressedImage(View|Data) to check against those, similarly to what Image does for pixels already not critically needed for this PR

@mosra mosra changed the title Support for all DXGI formats in DdsImporter [WIP] Support for all DXGI formats in DdsImporter Sep 22, 2019
@mosra mosra added this to the 2019.0c milestone Sep 22, 2019
@mosra mosra force-pushed the dds-dxgi-compressed branch from d087733 to bceb318 Compare September 23, 2019 07:54
@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 23, 2019

Codecov Report

Merging #67 (6a5d565) into master (65381f8) will increase coverage by 0.35%.
The diff coverage is 100.00%.

❗ Current head 6a5d565 differs from pull request most recent head 24e3e1d. Consider uploading reports for the commit 24e3e1d to get more accurate results

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   95.73%   96.08%   +0.35%     
==========================================
  Files         121      121              
  Lines       11955    11917      -38     
==========================================
+ Hits        11445    11451       +6     
+ Misses        510      466      -44     
Impacted Files Coverage Δ
src/MagnumPlugins/DdsImporter/DdsImporter.h 100.00% <ø> (ø)
src/MagnumPlugins/DdsImporter/DdsImporter.cpp 99.41% <100.00%> (+20.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65381f8...24e3e1d. Read the comment docs.

@mosra mosra mentioned this pull request Oct 2, 2019
30 tasks
@mosra mosra mentioned this pull request Jan 1, 2020
87 tasks
@mosra mosra modified the milestones: 2020.0a, 2020.0b Jun 24, 2020
@mosra mosra changed the title [WIP] Support for all DXGI formats in DdsImporter Support for all DXGI formats in DdsImporter May 27, 2022
@mosra mosra marked this pull request as draft May 27, 2022 11:56
@mosra mosra force-pushed the dds-dxgi-compressed branch from bceb318 to 61cd82e Compare May 27, 2022 22:20
@mosra mosra changed the title Support for all DXGI formats in DdsImporter Support all DXGI formats in DdsImporter, make it more robust May 28, 2022
@mosra mosra force-pushed the dds-dxgi-compressed branch 3 times, most recently from 6a5d565 to 24e3e1d Compare May 29, 2022 19:04
@mosra
Copy link
Copy Markdown
Owner Author

mosra commented May 29, 2022

Major part of the changes is merged in 65381f8...ee51ae4. This again exploded way more than expected and motivation dried out.

What remains is looking closer at ASTC and verifying with a second independent source, and cubemaps/arrays. Support for cubemaps, arrays, mips and especially combinations of those is either nonexistent or broken in majority of tools I tried, so it's not that important to be able to read them. When such a self-contained file is needed, it can be always easily extracted out of separately compressed DDS files and then combined into a KTX using magnum-imageconverter --layers.

@mosra mosra changed the title Support all DXGI formats in DdsImporter, make it more robust Support all formats, cubemaps and arrays in DdsImporter, make it more robust May 29, 2022
@mosra mosra force-pushed the dds-dxgi-compressed branch 3 times, most recently from 96734f1 to e12d402 Compare May 31, 2022 08:40
mosra added 7 commits May 31, 2022 10:50
Being more strict when checking legacy pixel formats (expecting also
RGB(A) bits being set), clearly preferring the DXT10 header over legacy
information, and making sure we handle cases of zero mip levels and zero
depth robustly.
Otherwise it looked we're testing swizzle twice for no reason.
@mosra mosra force-pushed the dds-dxgi-compressed branch from e12d402 to e7a68a4 Compare May 31, 2022 08:51
What was there was ... not working at all, mixing array slices and mip
levels together, not importing cubemaps as 3D, .... I suppose that was
because the code originated at a time where the importer interfaces had
no support for mip levels, or the GL library no support for uploading
cubemaps as a single 3D image, and so there wasn't really any other way
to implement this.

Also further tightening up various checks, like disallowing depths for
non-3D textures, or failing if both 3D and cubemap flag is set.
mosra added 7 commits May 31, 2022 18:48
KtxImporter does it like this, and it makes sense. Much less code that
way, and it's not endlessly repeated for every mip level.
Instead of during every image access, which can be quite excessive when
importing many mip levels. This is consistent with how KtxImporter does it.
Consistently with basically all other image importers. This was a
serious omission, however given the file format is most often used for
block-compressed data (where we can't flip as easily yet), it was not
*that* critical.

The behavior is mostly consistent with what got invented by pezcode for
KtxImporter. I just shamelessly compied most of the logic from there.
Basically mirroring the DdsImporter behavior. Once we have ASTC flipping
implemented and/or an ability to control the imported orientation, the
warnings will go away. Until then it's shitty like this.
@mosra mosra force-pushed the dds-dxgi-compressed branch from e7a68a4 to 192d26e Compare May 31, 2022 17:11
@mosra mosra merged commit 192d26e into master May 31, 2022
@mosra mosra deleted the dds-dxgi-compressed branch May 31, 2022 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants