Skip to content

Add 16-bits TGA support#65717

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
Game3DEE:feat-tga-16bits
Sep 26, 2022
Merged

Add 16-bits TGA support#65717
akien-mga merged 1 commit into
godotengine:masterfrom
Game3DEE:feat-tga-16bits

Conversation

@Ithamar

@Ithamar Ithamar commented Sep 12, 2022

Copy link
Copy Markdown

This adds support for TGA files with 16 bits per pixel (basically, RGBA5551, where the alpha bit is optional).
It should be possible to cherry-pick this into 3.x branch with no conflicts too.

Closes #65715

@Ithamar Ithamar requested a review from a team as a code owner September 12, 2022 19:14
@Calinou Calinou added this to the 4.0 milestone Sep 12, 2022
@fire

fire commented Sep 12, 2022

Copy link
Copy Markdown
Member

We made a decision not to support dithered artwork, but not sure the current state of this decision.

@Ithamar

Ithamar commented Sep 12, 2022

Copy link
Copy Markdown
Author

We made a decision not to support dithered artwork, but not sure the current state of this decision.

Does that also mean that the RGB565 / RGB5551 Image formats would be dropped in 4.x?

@Calinou

Calinou commented Sep 12, 2022

Copy link
Copy Markdown
Member

We made a decision not to support dithered artwork, but not sure the current state of this decision.

This is only for import – the resulting image format remains whatever Godot supports.

Does that also mean that the RGB565 / RGB5551 Image formats would be dropped in 4.x?

No, they won't be dropped. I think @fire is referring to paletted (256-color) textures, which Godot can import (e.g. from PNGs) but GPUs haven't supported for decades. These are converted to RGB8 or RGBA8 on import depending on whether the PNG contains an alpha channel.

@LauPhi

LauPhi commented Sep 16, 2022

Copy link
Copy Markdown

I see that the RGBA5551 TGA is still being saved as an Image::FORMAT_RGBA8 (32-bit).
Would it be possible to save it as a proper Image::FORMAT_RGBA5551 (16-bit) so that it would take two times less VRAM while loaded in the GPU?

@Ithamar

Ithamar commented Sep 16, 2022

Copy link
Copy Markdown
Author

I see that the RGBA5551 TGA is still being saved as an Image::FORMAT_RGBA8 (32-bit). Would it be possible to save it as a proper Image::FORMAT_RGBA5551 (16-bit) so that it would take two times less VRAM while loaded in the GPU?

It seems the TGA loader simply converts anything it reads to FORMAT_RGBA8, including monochrome images. This could indeed be improved, but I think it is something for another PR (I would be happy to look into that, but I prefer this PR getting resolved first).

@Ithamar

Ithamar commented Sep 16, 2022

Copy link
Copy Markdown
Author

On another note, while I recently used Image::load_bmp_from_buffer, I notice the docs actually state that is does not support 16-bit, though it does not give a reason. @Calinou are you aware of why that is?

@Calinou

Calinou commented Sep 17, 2022

Copy link
Copy Markdown
Member

On another note, while I recently used Image::load_bmp_from_buffer, I notice the docs actually state that is does not support 16-bit, though it does not give a reason. @Calinou are you aware of why that is?

The only mention of 16-bit BMP I can see is in #30634. I don't think there's a specific reason for 16-bit BMP not being supported, so feel free to add support for it as well 🙂

Please do so in a separate pull request, so they can be reviewed and merged independently.

@SaracenOne SaracenOne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems fine to me!

@akien-mga akien-mga merged commit 985fc6c into godotengine:master Sep 26, 2022
@akien-mga

Copy link
Copy Markdown
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 26, 2022
@timothyqiu

Copy link
Copy Markdown
Member

Cherry-picked for 3.6

@timothyqiu timothyqiu removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Dec 4, 2022
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.

TGA import/loading does not support 16-bits-per-pixel files

7 participants