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

DDSLoader: Remove FourCC header check. #24124

Merged
merged 1 commit into from
May 25, 2022
Merged

DDSLoader: Remove FourCC header check. #24124

merged 1 commit into from
May 25, 2022

Conversation

LeviPesin
Copy link
Contributor

@LeviPesin LeviPesin commented May 25, 2022

Related issue: #24121

Description

FourCC code is missing iff the file contains uncompressed texture and the DDSLoader seems to support uncompressed textures.

FourCC code is missing iff the file contains uncompressed texture and the DDSLoader seems to support uncompressed textures.
@Mugen87
Copy link
Collaborator

Mugen87 commented May 25, 2022

I think reporting a controlled error is better than producing a real exception. Hence, I would keep the check. It's like when GLTFLoader checks for an unsupported version.

@LeviPesin
Copy link
Contributor Author

But it does not produce an exception.

@LeviPesin
Copy link
Contributor Author

LeviPesin commented May 25, 2022

DDSLoader supports uncompressed textures:

https://github.com/mrdoob/three.js/blob/dev/examples/jsm/loaders/DDSLoader.js#L190-L198

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Ah... So the problem was that the check was actually not working as intended, neither it was needed?

@LeviPesin
Copy link
Contributor Author

Yes. Maybe the support for uncompressed textures was added after the check?

@LeviPesin
Copy link
Contributor Author

LeviPesin commented May 25, 2022

It seems that it was already like this in the first implementation of DDSLoader but it was not in the webgl-texture-utils implementation (it did not support uncompressed textures and had broken check).

@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Sounds good 👍

@mrdoob mrdoob added this to the r141 milestone May 25, 2022
@mrdoob mrdoob merged commit 5a4e5e4 into mrdoob:dev May 25, 2022
@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Thanks!

@LeviPesin LeviPesin deleted the patch-1 branch May 25, 2022 08:18
@Mugen87 Mugen87 changed the title DDSLoader: Fix DDSLoader: Remove FourCC header check. May 25, 2022
abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
FourCC code is missing iff the file contains uncompressed texture and the DDSLoader seems to support uncompressed textures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants