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

feat/mod/fix: buffers: Texture/Image Format Rewrite #331

Merged
merged 10 commits into from
Nov 1, 2024

Conversation

Luracasmus
Copy link
Contributor

@Luracasmus Luracasmus commented Sep 30, 2024

  • Clarify which pixel formats and types are associated with different image formats
  • Clarify which formats are supported in custom images
  • Add missing documentation for R3_G3_B2
  • Add notes about minimum OpenGL versions or required extensions for some formats and pixel types
  • Add info about the new formats
  • Rename the page to Image Format for consistency with the OpenGL Wiki

@Luracasmus
Copy link
Contributor Author

Should the page be renamed to Image Format since that's what OpenGL calls it?

Comment on lines 130 to 142

The shader loader supports some additional pixel types not directly associated with supported image formats. See the [OpenGL Wiki](https://www.khronos.org/opengl/wiki/Pixel_Transfer#Pixel_type) for more information about these.

| Pixel Type |
| ---------------------------- |
| `UNSIGNED_BYTE_3_3_2` |
| `UNSIGNED_SHORT_5_6_5` |
| `UNSIGNED_SHORT_4_4_4_4` |
| `UNSIGNED_SHORT_5_5_5_1` |
| `UNSIGNED_INT_8_8_8_8` |
| `UNSIGNED_INT_8_8_8_8_REV` |
| `UNSIGNED_INT_10_10_10_2` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's probably no point in including these if they aren't supported/used by Iris

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's probably no point in including these if they aren't supported/used by Iris

The thing is that they are supported, which not all pixel types are. They can still be used when loading raw custom textures if the raw data is stored with that pixel type afaik

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahh okay, I would specifically mention they are usable for raw custom textures, otherwise it seems like they don't have a purpose


Unsigned normalized buffers support values from 0.0 to 1.0 (inclusive).

| Image Format | Bit Depth | Pixel Format | Pixel Type | Image | Requirements |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be "Texture Format" instead of "Image Format"? Or is Image Format the correct term?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be "Texture Format" instead of "Image Format"? Or is Image Format the correct term?

#331 (comment)

After quickly reading the OpenGL specification and wiki i think image format is the more correct term

The wiki page most of what i wrote is based on is also called "Image Format"

| RG8 | RG16 | RGB10_A2 |
| RGB8 | RGB16 | |
| RGBA8 | RGBA16 | |
> Cells marked with ~strikethrough~ are currently unsupported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any point in including formats which aren't supported? Or is this because of the discussion of adding support for more formats? Either way, I feel that for now there's not much of a point in documented unsupported formats, they can always be added later if support is increased

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any point in including formats which aren't supported? Or is this because of the discussion of adding support for more formats? Either way, I feel that for now there's not much of a point in documented unsupported formats, they can always be added later if support is increased

I added them both for consistency and clarity (to show which image formats, bit depths, pixel formats and pixel types are associated, even one of them is unsupported).

You can still load custom raw textures with pixel formats and types associated with unsupported image formats, and image formats with unsupported pixel formats can still be used as colortex formats.

It also clearly shows which formats exist in OpenGL but aren't supported by Iris and makes it very easy to mark them as supported if they ever get added. I feel it would be less clear if we were to write items associated with unsupported items in a separate table or leave cells of unsupported items empty

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay that makes sense, I would make sure to describe at least some of that then, because it wasn't clear to me at first glance.

@Luracasmus Luracasmus changed the title feat/mod/fix: buffers: Texture Format Rewrite feat/mod/fix: buffers: Texture/Image Format Rewrite Oct 30, 2024
@Luracasmus Luracasmus marked this pull request as ready for review October 30, 2024 21:16
@ninjamike1211 ninjamike1211 merged commit a30d6f3 into IrisShaders:main Nov 1, 2024
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.

2 participants