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

Image.FORMAT_R8 uses Alpha in GLES2 instead of Red channel, this is not reflected in docs #38974

Open
Birdulon opened this issue May 23, 2020 · 4 comments

Comments

@Birdulon
Copy link
Contributor

Godot version: v3.2.1.stable.official, though relevant code appears to be the same in master

OS/device including version:

Issue description: Using Image.FORMAT_R8 in GLES3 correctly produces ImageTextures that have your given data in the Red channel. In GLES2, the data is not in the Red channel but the Alpha channel, breaking compatibility between the two backends and also breaking expectations.
The documentation does not specify this behavior which can cause confusion.
Relevant GLES3 code:

case Image::FORMAT_R8: {
r_format.format = RD::DATA_FORMAT_R8_UNORM;
r_format.swizzle_r = RD::TEXTURE_SWIZZLE_R;
r_format.swizzle_g = RD::TEXTURE_SWIZZLE_ZERO;
r_format.swizzle_b = RD::TEXTURE_SWIZZLE_ZERO;
r_format.swizzle_a = RD::TEXTURE_SWIZZLE_ONE;

Relevant GLES2 code:
case Image::FORMAT_R8: {
r_gl_internal_format = GL_ALPHA;
r_gl_format = GL_ALPHA;

Relevant documentation: https://github.com/godotengine/godot/blob/master/doc/classes/Image.xml#L506-L507

Steps to reproduce: Create an Image with format Image.FORMAT_R8, add data to it either by drawing pixels or supplying it with an existing PoolByteArray, create an ImageTexture from that Image and draw it.

Minimal reproduction project:
Make a scene based off a Control, add a TextureRect as a child with the following script:

extends TextureRect

func _ready() -> void:
	rect_scale = Vector2(100, 100)
	var image = Image.new()
	image.create_from_data(3, 3, false, Image.FORMAT_R8, PoolByteArray([0, 20, 40, 60, 80, 100, 128, 180, 255]))
	var image_texture = ImageTexture.new()
	image_texture.create_from_image(image, 0)
	texture = image_texture

Proposed fixes:

  1. Least disruptive to existing projects - add a note to the documentation specifying that Image.FORMAT_R8 will store data in the Alpha channel in GLES2.
  2. Make the GLES2 backend use GL_LUMINANCE instead of GL_APLHA for the format and internal format. The OpenGL ES2.0 reference card states that the red channel is used to access the data in this format, which will ensure compatibility with godot shaders written for the GLES3 backend, and not break expectations for users who aren't already working around the current behavior. It won't draw the same pure-red gradient that the GLES3 backend might, but it will give more expected behavior for shader code.
    https://www.khronos.org/opengles/sdk/docs/reference_cards/OpenGL-ES-2_0-Reference-card.pdf
@clayjohn
Copy link
Member

I would go with option 2. From what I can tell there is no difference in support between GL_LUMINANCE and GL_ALPHA. It looks like when porting over to GLES2 karroffel just went with GL_ALPHA because it seemed to be the closest to GL_RED

Note that GLES3 switches between GL_LUMINANCE and GL_RED for L8 depending on if mobile or desktop is used:

case Image::FORMAT_L8: {
#ifdef GLES_OVER_GL
r_gl_internal_format = GL_R8;
r_gl_format = GL_RED;
r_gl_type = GL_UNSIGNED_BYTE;
#else
r_gl_internal_format = GL_LUMINANCE;
r_gl_format = GL_LUMINANCE;
r_gl_type = GL_UNSIGNED_BYTE;
#endif
} break;

@Calinou
Copy link
Member

Calinou commented Oct 25, 2021

We can consider fixing this behavior in 3.5. Since 3.4 is nearing release and is in feature freeze, I think it's better to document the current status for now.

@clayjohn
Copy link
Member

I agree with @Calinou

Also, one catch I missed before but won't really change things, GL_LUMINANCE is automatically swizzled to return vec4(L, L, L, 1.0) whereas the swizzling we apply to GL_RED is vec4(R, 0.0, 0.0, 1.0). Therefore, if we switch to using GL_LUMINANCE the behaviour will slightly differ if users are relying on the blue and green channels being 0. That being said, mobile in GLES3 is already using GL_LUMINANCE so it already has that problem and no one has complained.

@jacobo-mc
Copy link

Maybe you should do what the GLES3 backend does, as it is known to work.

jacobo-mc added a commit to jacobo-mc/godot that referenced this issue Mar 25, 2022
@Mickeon Mickeon modified the milestones: 3.5, 3.6 Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants