[3.x] Fix rendering tiles using nested AtlasTextures

[kleonc]

In #68960 rendering TileMap tiles was changed from:

godot/scene/2d/tile_map.cpp

Line 571 in 1426cd3

tex->draw_rect_region(canvas_item, rect, r, modulate, c.transpose, normal_map, clip_uv);

to:

godot/scene/2d/tile_map.cpp

Line 578 in 910ddd1

VisualServerCanvasHelper::tilemap_add_rect(canvas_item, rect, tex->get_rid(), r, modulate, c.transpose, normal_map.is_valid() ? normal_map->get_rid() : RID(), clip_uv);

AtlasTexture's override of the Texture::draw_rect_region was properly handling nested AtlasTextures, but VisualServerCanvasHelper::tilemap_add_rect is not, it uses the passed rects directly within the VisualServer.

This PR adds Texture::get_combined_rect_region virtual method which returns the proper final dst/src rects in case of nested AtlasTextures. Such rects can be passed to VisualServerCanvasHelper::tilemap_add_rect and alike.
I've changed only the TileMap to use this new method, not sure if anything else would need to be changed too (like fonts or anything else changed in #68960? 🤔).
Not sure if that's the best solution but it works. 🙃

Fixes #76686.

Also removed MeshTexture::get_rect_region override as it was doing the same thing as Texture::get_rect_region anyway.

[clayjohn]

Looks good to me. Once @lawnjelly takes a look this should be good to merge

[lawnjelly]

Sorry been away for week, great work on finding this! 👍
I'll go through the PR asap.

[lawnjelly]

Looks like I didn't notice this function was virtual 😊 when making the PR and had some different implementations...

The general approach here looks good to me too, replacing the call to draw_rect_region() with some logic in a new function to translate the rects.

I wonder if for the complex texture types LargeTexture, MeshTexture we should just fallback to the old draw_rect_region(). MeshTexture uses a different API call, and LargeTexture probably isn't used enough to worry about accelerating. It would be nice to fix these too, I can do a follow up PR though, as it seems like AtlasTexture is the most pressing. It's a little pity because it makes things less elegant, but is more important it works to start with.

not sure if anything else would need to be changed too (like fonts or anything else changed

From a quick search in the original PR #68960 it seems like only the tilemap is calling draw_rect_region(), the fonts seem to be calling VisualServer directly, so shouldn't be affected unless I'm missing something (I'll look over in more detail this week).

The only thing I wasn't totally sure about was the name of the new function get_combined_rect_region(). I'm not sure it makes sense for e.g. an atlas texture, but I'm struggling to think of a better name. refine_rect_region()? Not sure. 🤔

[lawnjelly]

Am now working on a slightly modified version of this PR (will add coauthor with @kleonc as he found the problem / fix) that should fix all the cases, so maybe we can hold off on merging this until we have the other PR for comparison / discussion.

[akien-mga]

Thanks!

[akien-mga]

Am now working on a slightly modified version of this PR (will add coauthor with @kleonc as he found the problem / fix) that should fix all the cases, so maybe we can hold off on merging this until we have the other PR for comparison / discussion.

Oops, I hadn't seen this comment and this PR had two approvals so I went ahead and merged it.

#76775 can probably be updated to include a revert of the changes that would not be necessary with that other approach, so it can still be evaluated as a potentially better option to merge later on.

[lawnjelly]

Oops! 😄 Is okay, I'll rebase. This PR does fix the major bug, but we want to catch the other cases as well.