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

Detect cubemap for dds textures #10222

Merged
merged 1 commit into from
Oct 21, 2023
Merged

Conversation

Kanabenki
Copy link
Contributor

@Kanabenki Kanabenki commented Oct 21, 2023

Objective

Solution

  • When loading a dds texture, the header capabilities are checked for the cubemap flag. An error is returned if not all faces are provided.

Changelog

Added

  • Added a new texture error TextureError::IncompleteCubemap, used for dds cubemap textures containing less than 6 faces, as that is not supported on modern graphics APIs.

Fixed

  • DDS cubemaps are now loaded as cubemaps instead of 2D textures.

Migration Guide

If you are matching on a TextureError, you will need to add a new branch to handle TextureError::IncompleteCubemap.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 21, 2023
@github-actions
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@alice-i-cecile
Copy link
Member

Technically breaking as a variant was added to a public enum.

};
if is_cubemap {
if !dds.header.caps2.contains(
Caps2::CUBEMAP_NEGATIVEX
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right logic? It looks like this will return ture if any of these faces are found, but we want to return true only if all of the faces are found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's correct, this create a Caps2 bitflag containing all directions (with each being a distinct bit), so if one is missing it will return false. Just checked to be sure:

    let all = Caps2::CUBEMAP_NEGATIVEX
        | Caps2::CUBEMAP_NEGATIVEY
        | Caps2::CUBEMAP_NEGATIVEZ
        | Caps2::CUBEMAP_POSITIVEX
        | Caps2::CUBEMAP_POSITIVEY
        | Caps2::CUBEMAP_POSITIVEZ;
    dbg!(Caps2::CUBEMAP_NEGATIVEX.contains(all));
    dbg!(all.contains(all));

returns false then true, as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, there should be a CUBEMAP_ALLFACES flag that OR all directions according to this DDS doc, I'll see if I can add it to ddsfile.

@superdump superdump added this pull request to the merge queue Oct 21, 2023
Merged via the queue into bevyengine:main with commit 9cfada3 Oct 21, 2023
robtfm pushed a commit to robtfm/bevy that referenced this pull request Oct 23, 2023
# Objective

- Closes bevyengine#10049.
- Detect DDS texture containing a cubemap or a cubemap array.

## Solution

- When loading a dds texture, the header capabilities are checked for
the cubemap flag. An error is returned if not all faces are provided.

---

## Changelog

### Added

- Added a new texture error `TextureError::IncompleteCubemap`, used for
dds cubemap textures containing less than 6 faces, as that is not
supported on modern graphics APIs.

### Fixed

- DDS cubemaps are now loaded as cubemaps instead of 2D textures.

## Migration Guide

If you are matching on a `TextureError`, you will need to add a new
branch to handle `TextureError::IncompleteCubemap`.
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
# Objective

- Closes bevyengine#10049.
- Detect DDS texture containing a cubemap or a cubemap array.

## Solution

- When loading a dds texture, the header capabilities are checked for
the cubemap flag. An error is returned if not all faces are provided.

---

## Changelog

### Added

- Added a new texture error `TextureError::IncompleteCubemap`, used for
dds cubemap textures containing less than 6 faces, as that is not
supported on modern graphics APIs.

### Fixed

- DDS cubemaps are now loaded as cubemaps instead of 2D textures.

## Migration Guide

If you are matching on a `TextureError`, you will need to add a new
branch to handle `TextureError::IncompleteCubemap`.
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Closes bevyengine#10049.
- Detect DDS texture containing a cubemap or a cubemap array.

## Solution

- When loading a dds texture, the header capabilities are checked for
the cubemap flag. An error is returned if not all faces are provided.

---

## Changelog

### Added

- Added a new texture error `TextureError::IncompleteCubemap`, used for
dds cubemap textures containing less than 6 faces, as that is not
supported on modern graphics APIs.

### Fixed

- DDS cubemaps are now loaded as cubemaps instead of 2D textures.

## Migration Guide

If you are matching on a `TextureError`, you will need to add a new
branch to handle `TextureError::IncompleteCubemap`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DDS loading does not recognize cubemap imports
3 participants