godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 78 forks source link

Add `RGB10_A2` pixel format to Image #7914

Open armsnyder opened 1 year ago

armsnyder commented 1 year ago

Describe the project you are working on

Procedural terrain. I am generating a heightmap and normalmap using a compute shader, and then passing these as textures to a spatial shader for displaying the terrain.

Describe the problem or limitation you are having in your project

I want to pass a normalmap texture to a spatial shader using higher than 8-bit precision, without exceeding 4 bytes per texel. My normalmap includes steep gradients, so I need all 3 axes. There is a GL_RGB10_A2 OpenGL texture format for this purpose.

GL_RGB10_A2: 10 bits each for RGB, 2 for Alpha. This can be a useful format for framebuffers, if you do not need a high-precision destination alpha value. It carries more color depth, thus preserving subtle gradations. They can also be used for normals, though there is no signed-normalized version, so you have to do the conversion manually. It is also a required format (see below), so you can count on it being present.

However, it is not possible to set a spatial shader uniform sampler2D using the GL_RGB10_A2 texture format, because sampler2D uniforms are set using Texture2D, and Image does not have a GL_RGB10_A2 Format.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I propose expanding the existing Format enum to include a new FORMAT_RGB10_A2 value.

There is already a DATA_FORMAT_A2B10G10R10_UNORM_PACK32 data format in the lower level RenderingDevice code, which this new image format would map to.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Here is the example code which I would like to work with the FORMAT_RGB10_A2 image format:

# Set up the compute shader uniform. This part already works.
var normalmap_format := RDTextureFormat.new()
normalmap_format.format = RenderingDevice.DATA_FORMAT_A2B10G10R10_UNORM_PACK32

# <snip> The rest of the compute shader code is boilerplate.

# Get the normalmap output from the compute shader.
var normalmap: PackedByteArray = rd.texture_get_data(normalmap_rid, 0)

# Pass the normalmap to the spatial shader. Note the FORMAT_RGB10_A2 format is not implemented.
var image := Image.create_from_data(size, size, false, Image.FORMAT_RGB10_A2, normalmap)
material.set_shader_parameter("u_normalmap", ImageTexture.create_from_image(image))

If this enhancement will not be used often, can it be worked around with a few lines of script?

I have not found a workaround without compromise. I can choose the FORMAT_RGBH or FORMAT_RGB8 format, but I would be compromising either efficiency or precision.

Is there a reason why this should be core and not an add-on in the asset library?

This is an enhancement to an existing Godot type.

armsnyder commented 1 year ago

Related to:

armsnyder commented 1 year ago

I'm going to take a shot at implementing this. I'll base my code on the existing FORMAT_RGBE9995 format, which is another OpenGL special format. The naming convention would dictate that the new enum be named FORMAT_RGB10A2.

It looks like some special handling will be needed for mipmap generation in the Image::shrink_x2 and Image::generate_mipmaps methods. I'm actually not sure if this is necessary, since not all image formats are implemented there, but I can do it for completeness.

All non-compressed image formats have mipmap generation except FORMAT_RGBA4444 and FORMAT_RGB565. FORMAT_RGBA4444 has a special "Cannot generate mipmaps from RGBA4444 format." error message, but FORMAT_RGB565 will fail silently. This raises 2 questions:

  1. Why can't mipmaps be generated for FORMAT_RGBA4444? Is this a design choice, or is it just not-yet-implemented?
  2. Isn't it a bug that FORMAT_RGB565 will pass the error checking but does not have a switch case to generate the mipmaps?
armsnyder commented 1 year ago

I'm considering changing my request to be for the R16G16_UINT data format instead of A2B10G10R10_UNORM_PACK32, although at this point it is a small optimization over the already-supported FORMAT_RGH image format. 😄

Why?

  1. More bits for the same word size. For the use case of terrain normals, I will never have a negative Y (vertical) component. Therefore, my texture can include only the X and Z components, and I can reconstruct the Y component in my spatial shader with the knowledge that it is a unit vector. This increases the bits per component from 10 to 16.
  2. Integer over float. I'm only representing values between -1 and 1, which means I should get more accuracy with an integer format than the already-supported FORMAT_RGH, which maps to DATA_FORMAT_R16_SFLOAT according to texture_storage.cpp.
  3. Vulkan compatibility. My ideal choice would be a normalized version like R16G16_SNORM, however they are not a required format for Vulkan implementations. R16G16_UINT and R16G16_SINT are both required, but I would lean toward R16G16_UINT as it is slightly easier to renormalize by hand.

That being said, the original request for A2B10G10R10_UNORM_PACK32 has its merits for representing vectors that are either unnormalized or do not have a predictable sign for one of the components, or if reconstructing the vector is expensive. But I no longer have this use case personally.