[4.3] RenderingDevice split caused TextureArray Mipmap rendering artifact

[TokisanGames]

Tested versions

Bisecting reveals:

• First caused by Split RenderingDevice into API-agnostic and RenderingDeviceDriver parts #83452
• Improved by Fix determination of copyable layout for compressed textures in Vulkan #86855
• Still present in Godot 4.3-dev5
• Not present in Godot 4.2.2

System information

Windows 11/64, RTX 3070, Vulkan

Issue description

4.3 introduced a rendering artifact in Terrain3D (the blue). sampler2DArray + Mipmaps causes the problem. Regular sampler, or disabling mipmaps (in import or sampler texture filtering) removes the artifact. Problem PRs found by bisecting.

Terrain3D uses a sampler2DArray with mipmap filtering to render the terrain. Here it is displaying in 4.2.2 with a flat terrain and a minimal shader.

PR #83452 caused two rendering artifacts, the green up close and dark far away. Changing UV scale moves the artifacts closer or farther from the camera.

PR #86855 improved the artifact by fixing the foreground.

• The texture used here is stored in a TextureArray.

• It is a PNG imported with VRAM Compressed HQ as BPTC. DXT5 RGB also tested. No difference between them.

• With and without mipmaps have been tested. Disabling mipmaps removes the artifact.

• The shader in the three pictures above has been simplified down to this minimal example.

shader_type spatial;
render_mode blend_mix,depth_draw_opaque,cull_back,diffuse_burley,specular_schlick_ggx;

uniform sampler2DArray _texture_array_albedo : source_color, filter_linear_mipmap_anisotropic, repeat_enable;

void vertex() {
	vec3 v_vertex = (MODEL_MATRIX * vec4(VERTEX, 1.0)).xyz;
	UV = round(v_vertex.xz);
}

void fragment() {
	ALBEDO = texture(_texture_array_albedo, vec3(UV*.05, 0.)).rgb;
}

• Using this shader, but changing the sampler filtering to linear or nearest without mipmaps removes the artifact, though it's noisy.
• Using this shader, but changing the texture import settings to not generate mipmaps removes the artifact, though it's noisy.

• Creating a large meshinstance plane 20k x 20k and the same albedo texture file as on the terrain, using the Standard Material and linear filtering with mipmaps does not produce the artifact.
• Using a shadermaterial on the plane with the above shader, with linear filtering w/ mipmaps, but making it a regular Sampler2D instead of a texture array does not produce the artifact.
• Using this sampler2D version of the shader on our terrain meshes does not produce the artifact.

Conclusion, it shows the artifact when using sampler2DArray and textures with mipmaps.

Steps to reproduce

• Download the release of Terrain3D https://github.com/TokisanGames/Terrain3D/releases/tag/v0.9.1-beta
• Open the demo project in Godot 4.2.2 to see it normally, or 4.3-dev to see the artifact.
• Restart when prompted, then again after Godot imports to get setup.
• Under Terrain3D/Material, enable Shader Override and overwrite it with this minimum shader to see the artifact.

shader_type spatial;
render_mode blend_mix,depth_draw_opaque,cull_back,diffuse_burley,specular_schlick_ggx;

uniform sampler2DArray _texture_array_albedo : source_color, filter_linear_mipmap_anisotropic, repeat_enable;

void vertex() {
	vec3 v_vertex = (MODEL_MATRIX * vec4(VERTEX, 1.0)).xyz;
	UV = round(v_vertex.xz);
}

void fragment() {
	ALBEDO = texture(_texture_array_albedo, vec3(UV*.05, 0.)).rgb;
}

• This variation uses sampler2D and does not show the artifact. Attach one of the demo textures to the texture_albedo slot.

shader_type spatial;
render_mode blend_mix,depth_draw_opaque,cull_back,diffuse_burley,specular_schlick_ggx;

uniform sampler2D texture_albedo : source_color, filter_linear_mipmap_anisotropic, repeat_enable;

void vertex() {
	vec3 v_vertex = (MODEL_MATRIX * vec4(VERTEX, 1.0)).xyz;
	UV = round(v_vertex.xz);
}

void fragment() {
	ALBEDO = texture(texture_albedo, UV*.05).rgb;
}

Minimal reproduction project (MRP)

See above.

cc: @RandomShaper, @Calinou, @clayjohn

[clayjohn]

Taking a look in Renderdoc, I can see that the final mipmap levels of the ArrayTexture are not being filled (2x2 and 1x1) so garbage data is being used. The solution is going to be ensuring that the full mipmap data is transferred to the GPU

[clayjohn]

Okay, after some investigation, I have created an MRP for this issue.

terrain3dmrp.zip

Edit: Actually this MRP seems to be inconsistent, it only fails 50% of the time. Will need to investigate more

Back to the original MRP, I have sped up testing using the following script (same as in the new MRP)

extends Terrain3D

var ridd
var imgtex
func _ready() -> void:
	var tex:CompressedTexture2D = load("res://demo/assets/textures/ground037_alb_ht.png")
	var img: Image = tex.get_image()
	img.decompress()
	ridd = RenderingServer.texture_2d_layered_create([img, img], RenderingServer.TEXTURE_LAYERED_2D_ARRAY)
	var mat = $Camera3D/MeshInstance3D.material_override.get_rid()
	RenderingServer.material_set_param(mat, "texture_albedo", ridd)
	
	imgtex = ImageTexture.create_from_image(img)
	mat = $Camera3D/MeshInstance3D.material_overlay.get_rid()
	RenderingServer.material_set_param(mat, "texture_albedo", imgtex.get_rid())

When the second texture is created from the image, it displays the same issue as the layered texture. So this issue likely isn't specific to TextureArrays after all.

Okay, I can confirm that the image has the issue, even before the ImageTexture of TextureArray are created. So this is most likely a texture readback issue.

[clayjohn]

Okay, here is a diff that appears to work. Please give this a try. I'm going to double check it in the morning, but I think this should be good:

diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp
index 1906d168fe..aeeacafc6e 100644
--- a/drivers/vulkan/rendering_device_driver_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp
@@ -1703,7 +1703,7 @@ void RenderingDeviceDriverVulkan::texture_get_copyable_layout(TextureID p_textur
                uint32_t bw = 0, bh = 0;
                get_compressed_image_format_block_dimensions(tex_info->rd_format, bw, bh);
                uint32_t sbw = 0, sbh = 0;
-               r_layout->size = get_image_format_required_size(tex_info->rd_format, w, h, d, 1, &sbw, &sbh);
+               r_layout->size = get_image_format_required_size(tex_info->rd_format, MAX(w, bw), MAX(h, bh), d, 1, &sbw, &sbh);
                r_layout->row_pitch = r_layout->size / ((sbh / bh) * d);
                r_layout->depth_pitch = r_layout->size / d;
                r_layout->layer_pitch = r_layout->size / tex_info->vk_create_info.arrayLayers;
diff --git a/servers/rendering/rendering_device.cpp b/servers/rendering/rendering_device.cpp
:...skipping...
diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp
index 1906d168fe..aeeacafc6e 100644
--- a/drivers/vulkan/rendering_device_driver_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp
diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp
index 1906d168fe..aeeacafc6e 100644
--- a/drivers/vulkan/rendering_device_driver_vulkan.cpp
diff --git a/drivers/vulkan/rendering_device_driver_vulkan.cpp b/drivers/vulkan/rendering_device_driver_vulkan.cpp
index 1906d168fe..aeeacafc6e 100644
--- a/drivers/vulkan/rendering_device_driver_vulkan.cpp
+++ b/drivers/vulkan/rendering_device_driver_vulkan.cpp
@@ -1703,7 +1703,7 @@ void RenderingDeviceDriverVulkan::texture_get_copyable_layout(TextureID p_textur
                uint32_t bw = 0, bh = 0;
                get_compressed_image_format_block_dimensions(tex_info->rd_format, bw, bh);
                uint32_t sbw = 0, sbh = 0;
-               r_layout->size = get_image_format_required_size(tex_info->rd_format, w, h, d, 1, &sbw, &sbh);
+               r_layout->size = get_image_format_required_size(tex_info->rd_format, MAX(w, bw), MAX(h, bh), d, 1, &sbw, &sbh);
                r_layout->row_pitch = r_layout->size / ((sbh / bh) * d);
                r_layout->depth_pitch = r_layout->size / d;
                r_layout->layer_pitch = r_layout->size / tex_info->vk_create_info.arrayLayers;
diff --git a/servers/rendering/rendering_device.cpp b/servers/rendering/rendering_device.cpp
index 2b6644e893..855eeabfd5 100644
--- a/servers/rendering/rendering_device.cpp
+++ b/servers/rendering/rendering_device.cpp
@@ -1350,6 +1350,9 @@ Vector<uint8_t> RenderingDevice::texture_get_data(RID p_texture, uint32_t p_laye
                thread_local LocalVector<RDD::BufferTextureCopyRegion> command_buffer_texture_copy_regions_vector;
                command_buffer_texture_copy_regions_vector.clear();
 
+               uint32_t block_w = 0, block_h = 0;
+               get_compressed_image_format_block_dimensions(tex->format, block_w, block_h);
+
                uint32_t w = tex->width;
                uint32_t h = tex->height;
                uint32_t d = tex->depth;
@@ -1365,8 +1368,8 @@ Vector<uint8_t> RenderingDevice::texture_get_data(RID p_texture, uint32_t p_laye
                        copy_region.texture_region_size.z = d;
                        command_buffer_texture_copy_regions_vector.push_back(copy_region);
 
-                       w = MAX(1u, w >> 1);
-                       h = MAX(1u, h >> 1);
+                       w = MAX(block_w, w >> 1);
+                       h = MAX(block_h, h >> 1);
                        d = MAX(1u, d >> 1);
                }
 
@@ -1395,8 +1398,6 @@ Vector<uint8_t> RenderingDevice::texture_get_data(RID p_texture, uint32_t p_laye
                for (uint32_t i = 0; i < tex->mipmaps; i++) {
                        uint32_t width = 0, height = 0, depth = 0;
                        uint32_t tight_mip_size = get_image_format_required_size(tex->format, w, h, d, 1, &width, &height, &depth);
-                       uint32_t block_w = 0, block_h = 0;
-                       get_compressed_image_format_block_dimensions(tex->format, block_w, block_h);
                        uint32_t tight_row_pitch = tight_mip_size / ((height / block_h) * depth);
 
                        // Copy row-by-row to erase padding due to alignments.
@@ -1408,8 +1409,8 @@ Vector<uint8_t> RenderingDevice::texture_get_data(RID p_texture, uint32_t p_laye
                                wp += tight_row_pitch;

Basically the problem is that we forgot about block size when actually copying the texture from the GPU. With compressed textures the 2x2 and 1x1 mipmaps still contain 4x4 pixels but we were only copying 5 pixels total from the GPU.

Also, for further investigation. I found out while working on this that our BPTC decompression is broken. Likely with a similar issue

The BPTC decompression issue was totally different:

diff --git a/modules/cvtt/image_compress_cvtt.cpp b/modules/cvtt/image_compress_cvtt.cpp
index ad70772270..e9a7009d7c 100644
--- a/modules/cvtt/image_compress_cvtt.cpp
+++ b/modules/cvtt/image_compress_cvtt.cpp
@@ -302,8 +302,6 @@ void image_decompress_cvtt(Image *p_image) {
                        int y_end = y_start + 4;
 
                        for (int x_start = 0; x_start < w; x_start += 4 * cvtt::NumParallelBlocks) {
-                               int x_end = x_start + 4 * cvtt::NumParallelBlocks;
-
                                uint8_t input_blocks[16 * cvtt::NumParallelBlocks];
                                memset(input_blocks, 0, sizeof(input_blocks));
 
@@ -315,6 +313,8 @@ void image_decompress_cvtt(Image *p_image) {
                                memcpy(input_blocks, in_bytes, 16 * num_real_blocks);
                                in_bytes += 16 * num_real_blocks;
 
+                               int x_end = x_start + 4 * num_real_blocks;
+
                                if (is_hdr) {
                                        if (is_signed) {
                                                cvtt::Kernels::DecodeBC6HS(output_blocks_hdr, input_blocks);

I'll make PRs tomorrow

[TokisanGames]

I checked out master and applied these. Though the first one is a really odd diff. I couldn't apply it automatically with patch nor git apply. It includes changes across multiple commits, even a duplicate one. I applied it manually.

I see no change to the far dark artifact. I also regenerated the ctex files going to DXT and a new BPTC generation and see no difference.

[clayjohn]

Hmmm, maybe something went wrong applying the diffs. Here is a proper patch file with all the changes after rebasing:
terrain3d.patch.txt

edit: looking at the diff above I can see that it got messed up somehow when copying. I'll update the diff in that post too

[TokisanGames]

This patch applied fine and resolved the artifact, thank you!

I also regenerated the BPTC textures, but noticed no difference or problem.

btw unrelated but I noticed that on the current master, loading Godot (win/3070) the editor viewport won't render any models until the camera is moved. Wasn't a problem on 4.3dev-5.