Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DDS: Add support for loading layered textures #98322

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

BlueCube3310
Copy link
Contributor

@BlueCube3310 BlueCube3310 commented Oct 18, 2024

Adds support for loading layered DDS texture files.

Progress:

  • 2D Arrays,
  • Cubemaps,
  • Cubemap arrays,
  • 3D textures,
  • Testing project.

MRP: ddslayered.zip

@BlueCube3310 BlueCube3310 requested a review from a team as a code owner October 18, 2024 21:27
@fire
Copy link
Member

fire commented Oct 19, 2024

There are some unused variables.


modules/dds/texture_loader_dds.cpp:648:14: error: unused variable 'dimension' [-Werror,-Wunused-variable]
                                uint32_t dimension = f->get_32();
                                         ^
modules/dds/texture_loader_dds.cpp:649:14: error: unused variable 'misc_flags_1' [-Werror,-Wunused-variable]
                                uint32_t misc_flags_1 = f->get_32();
                                         ^
modules/dds/texture_loader_dds.cpp:651:14: error: unused variable 'misc_flags_2' [-Werror,-Wunused-variable]
                                uint32_t misc_flags_2 = f->get_32();
                                         ^
Retrieved `modules/gdscript/gdscript_lambda_callable.android.editor.arm64.o' from cache
modules/dds/texture_loader_dds.cpp:579:11: error: unused variable 'caps_1' [-Werror,-Wunused-variable]
        uint32_t caps_1 = f->get_32();

@BlueCube3310
Copy link
Contributor Author

There's an issue with the quick load menu where the dds files don't show up, since it narrows down the search range to a specific resource class (like Texture2D), and doesn't recognize them by their parent type (Texture in this case)

@BlueCube3310 BlueCube3310 marked this pull request as draft October 28, 2024 09:15
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Testing project: test_dds_cubemap.zip

Before

Notice the resource type icon at the top. Here, it's loaded as a Texture2D. Only one face is imported.

Uncompressed DXT5
image image

After

The DDS file is now loaded as a Cubemap automatically. Memory usage is multiplied by 6 because it's now importing all faces, not just one.

Uncompressed DXT5
image image

PS: We should improve the Cubemap preview to allow choosing a face to view, or even have an interactive view like the material preview (except you'd be inside the box and the box would be unshaded, and with flipped faces).

@BlueCube3310
Copy link
Contributor Author

PS: We should improve the Cubemap preview to allow choosing a face to view, or even have an interactive view like the material preview (except you'd be inside the box and the box would be unshaded, and with flipped faces).

This system is already in place, though it's not well-documented: left-click the cubemap preview and drag the cursor :)

@BlueCube3310 BlueCube3310 marked this pull request as ready for review October 29, 2024 19:13
@BlueCube3310
Copy link
Contributor Author

I've added a testing project for every texture type. It seems to work correctly, though now thumbnails are not generated and the aforementioned issue with the quick search menu persists

@clayjohn clayjohn modified the milestones: 4.x, 4.4 Oct 29, 2024
@clayjohn clayjohn merged commit 0debc73 into godotengine:master Oct 29, 2024
20 checks passed
@clayjohn
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants