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

texture_get_data doesn't return mipmaps for layered textures #49442

Open
Xyotic opened this issue Jun 8, 2021 · 5 comments
Open

texture_get_data doesn't return mipmaps for layered textures #49442

Xyotic opened this issue Jun 8, 2021 · 5 comments

Comments

@Xyotic
Copy link

Xyotic commented Jun 8, 2021

Godot version

3.3.2

System information

Windows 10 (ver. 2004), GLES3, NVIDIA GeForce RTX 2080

Issue description

When creating a Texture array with Flag 1 (FlagsEnum.Mipmaps), no mipmaps are created for the layers, nor will existing mipmaps be transfered over.

Since it is possible to set this flag the expected behaviour would be that mipmaps will be created for all layers.

Steps to reproduce

Using C#:

  1. Create a TextureArray
TextureArray textureArray = new TextureArray();
textureArray.Create(2048, 2048, 1, Image.Format.Rgba8, (int)TextureLayered.FlagsEnum.Mipmaps);
  1. Add Image
textureArray.SetLayerData(myImage, 0);

If in a prior step mipmaps were created they wont get transfered over. ( myImage.GenerateMipmaps() )

  1. Check if the Image inside the Texture got any mipmaps
bool hasMipmaps = textureArray.GetLayerData(0).HasMipmaps(); //will be false

Note:
You can also check for mipmaps looking at the size of the Data array of the Image using myImage.GetData().Count() or by checking with .GetMipmapOffset

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Jun 8, 2021

Related to/duplicate of #34312?

@Xyotic
Copy link
Author

Xyotic commented Jun 8, 2021

Seems related...

Also wanted to add:
The documentation states quote: "FLAG_MIPMAPS = 1 --- Texture will generate mipmaps on creation." link
Sounds like the mipmap generation might be already in place, Just happens at the wrong time since you have to call Create() first before adding Images...

@clayjohn clayjohn changed the title TextureArray/TextureLayered Mipmaps not working texture_get_data doesn't return mipmaps for layered textures Jun 9, 2021
@clayjohn
Copy link
Member

clayjohn commented Jun 9, 2021

This was a lot of fun to explore.

The problem in your MRP lies in get_layer_data(). get_layer_data()` calls texture_get_data()`` which is defined here:

Ref<Image> RasterizerStorageGLES3::texture_get_data(RID p_texture, int p_layer) const {

texture_get_data() has a different pathway when dealing with layered textures. Unfortunately, it seems that whoever implemented that pathway forget about mipmaps. So texture_get_data() always returns an image without mipmaps when called on a Texture3D or TextureArray. This doesn't mean your image doesn't have mipmaps. The fact that mipmaps are missing from texture_get_data is a bug

You are right to point out that mipmaps are created on texture creation. In your MRP mipmaps are created. However if myImage doesn't have mipmaps, then the mipmaps in textureArray will be ignored when you call setLayerData(), which makes sense, you are only giving Godot a single mip level so it only sets a single mip level. Accordingly, even though you have mipmaps, you are likely never seeing them. This should be better documented.

The combination of these two issues made it look like your TextureArray was never creating mipmaps.

Below is an MRP that shows mipmaps working correctly with TextureArray:

var tex = load("res://icon.png")
var image = tex.get_data() # need to load as a texture otherwise mipmaps are not generated
var textureArray = TextureArray.new()
textureArray.create(64, 64, 1, Image.FORMAT_RGBA8, 1) # allocates space for mipmaps
textureArray.set_layer_data(image, 0)
print(textureArray.data["flags"]) # prints "1" because the mipmap flag is enabled
print(textureArray.get_layer_data(0).has_mipmaps()) # prints "false" because
$ColorRect.material.set_shader_param("tex_array", textureArray)

Shader code for the ColorRect:

shader_type canvas_item;

uniform sampler2DArray tex_array;

void fragment() {
	
	COLOR = textureLod(tex_array, vec3(UV, 0), 3.0*3.0*sin(TIME));
}

@Xyotic
Copy link
Author

Xyotic commented Jun 9, 2021

Thanks for looking into this!
This would have been a mayor performance issue.
I will test your MRP later today.

@Xyotic
Copy link
Author

Xyotic commented Jun 11, 2021

Can confirm this works.

I want to add that:

var image = tex.get_data() # need to load as a texture otherwise mipmaps are not generated

is not 100% correct as you dont need to load it as a texture.
Just call .GenerateMipmaps() on your image before adding it to the array.

This is useful when you dont import your image as a texture

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

3 participants