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

TileSet Vertex Shader unexpected behaviour- rendering_quadrant_size affects VERTEX value #86812

Closed
SebasZwolf opened this issue Jan 5, 2024 · 6 comments · Fixed by #86847
Closed

Comments

@SebasZwolf
Copy link

SebasZwolf commented Jan 5, 2024

Tested versions

  • Found in: v4.2.1.stable.official [b09f793]

System information

Godot v4.2.1.stable - Windows 10.0.22621 - Vulkan (Forward+) - dedicated GeForce GT 730 () - Intel(R) Core(TM) i3-10100F CPU @ 3.60GHz (8 Threads)

Issue description

When the TileMap is drawn, at VERTEX position x:256 or y:256 (or both), it causes the vertex shadow to produce unexpected behaviour.

imagen_2024-01-04_193831252

Steps to reproduce

My tile size is 16*16.

the code required for this shader is the following at it must be used in a TileSet Tile using the TileSet editor.

// the UV values are not used here because they are relative to the atlas texture. therefore the UV.x and UV.y do not represent values from 0.0 to 1.0

varying vec2 pos;
void vertex() {
	pos = VERTEX / 32.0; //32.0 is later referred as "division value"
}
void fragment() {
	vec2 lpos = mod(pos,1.0);
	COLOR.rgb = vec3(lpos.x, lpos.y, 0.5);
}

The code allows to pass to fragment a value that represent the coordinates of the tile (vec2 pos).
Then it converts it to a value between 0.0; 0.5 and 0.5; 1.0 (since the position is divided by 32 instead of 16).

this allows to create gradients conntecting 4 tiles

The error appears in every tile that is in x:256 or y:256, in order to provide example, take tile at (x: 256, y: 0): the VERTEX value will have the values:

(240.0, 00.0) (256.0, 00.0)
(240.0, 16.0) (256.0, 16.0)

Since is not posible to debug the shader, this values were obtain comparing the VERTEX value of the affected tiles with their neighbors using conditionals inside the fragment shader. This revealed that:

these are the same values of the tile located at (x:240, y:0)

However, this error will not occur if rendering_quadrant_size is set to 32 or the division value is set to 16.

Minimal reproduction project (MRP)

bugreport.zip

edit: i corrected the table values (the VERTEX values for each vertex for tile (256, 0)), values were misplaced

@clayjohn
Copy link
Member

clayjohn commented Jan 5, 2024

I'm not sure there is an actual bug here. The tilemap doesn't render tiles individually, it renders them as quadrants. So if the quadrant size doesn't align with the size you are using in the fragment shader, then they just won't line up. You need to use a quadrant size that aligns with the data you expect in your shader

@SebasZwolf
Copy link
Author

SebasZwolf commented Jan 5, 2024

I'm aware that tiles are rendered by quadrants. However, the shader is executed for each tile drawn, the issue is that from the first column (and row) of the new quadrant being drawn, the VERTEX value for all tiles is moved -16.0 (E.g. tile at (256,0) will have as VERTEX value for the top left corner (240,0)), which causes the issue of the gradient shown in the image.

My apologies if i cause confusion.

I hope this images help explain the issue

image

the quadrant 2, the VERTEX values of all tiles are moved 16.0 to the left so at tile (256,0) the VERTEX value is the equal to the VERTEX value at tile (240,0).

while this can be "solved" by making the "division value" in the shader and the rendering_quadrant_size the same value, it would make quadrant size too big it may have impact on performance.

E.g. suppose a gradient composed by 8x8 tiles,it would require the "division value" to be 128 and the rendering_quadrant_size too. It would mean that 16384 tiles will have to be drawn in the screen per quadrant and most of them would not be inside the visible window.

@kleonc
Copy link
Member

kleonc commented Jan 5, 2024

The issue is that TileMap's each quadrant's canvas item position (to which vertices of the in-quadrant-tiles are relative to) is set to its first cell's map-coordinates:

int quad_size = tile_map_node->get_rendering_quadrant_size();
const Vector2i &coords = r_cell_data.coords;
// Rounding down, instead of simply rounding towards zero (truncating).
quadrant_coords = Vector2i(
coords.x > 0 ? coords.x / quad_size : (coords.x - (quad_size - 1)) / quad_size,
coords.y > 0 ? coords.y / quad_size : (coords.y - (quad_size - 1)) / quad_size);
canvas_items_position = quad_size * quadrant_coords;

It should be set to the local position of such first cell in the quadrant instead.
Note that what's considered cell's position is its center so the question would be whether the quadrant canvas item position should be its first cell's position/center or maybe such cell's top left corner instead. I think top left corner makes more sense.

It's not obviously visible issue as draw_tile calls take into account quadrant's canvas item position so the tiles end up where they need to:

tile_map_node->draw_tile(ci, local_tile_pos - rendering_quadrant->canvas_items_position, tile_set, cell_data.cell.source_id, cell_data.cell.get_atlas_coords(), cell_data.cell.alternative_tile, -1, tile_map_node->get_self_modulate(), tile_data, random_animation_offset);


To visualize given the provided example:

imagen_2024-01-04_193831252

varying vec2 pos;
void vertex() {
	pos = VERTEX / 32.0; //32.0 is later referred as "division value"
}
void fragment() {
	vec2 lpos = mod(pos,1.0);
	COLOR.rgb = vec3(lpos.x, lpos.y, 0.5);
}

tile_size = 16
quadrant size = 16

The white square below shows the first cell of the quadrant with quadrant-coords (1, 0). Currently this quadrant's canvas_items_position is set to the map-coords of such cell which is 16 * (1, 0) = (16, 0). Since quadrant canvas item is a child of the TileMap's canvas item this is the point marked with the red cross below. That's what VERTEX in the shader is relative to, hence for this white-square cell VERTEX is from (240, 0) to (256, 16).
294331568-97042f9f-c6b6-4e70-97ac-0b63fe63ce60

canvas_items_position of such quadrant should be instead set to the white-square cell's center (blue cross, (264, 8)) or topleft corner (green cross, (256, 0)).
Assuming it would be set to the green point the VERTEX in the shader for the white-square cell would be from (0, 0) to (16, 16). And I'd say that's expectable, AFAIK that's exactly how UVs work in the 3.x version of the TileMap.
PR for this incoming.

@kleonc kleonc added bug and removed needs testing labels Jan 5, 2024
@clayjohn
Copy link
Member

clayjohn commented Jan 5, 2024

@SebasZwolf I think a valid workaround for you use-case might be to use render_mode world_vertex_coords which presents VERTEX in world space instead of local space. That way you won't have the discontinuity at the border of the quadrant. Here is your MRP with render_mode world_vertex_coords added:
image

@SebasZwolf
Copy link
Author

SebasZwolf commented Jan 5, 2024

This workaround works fine, thanks a lot :0

@clayjohn
Copy link
Member

clayjohn commented Jan 6, 2024

This workaround works fine, thanks a lot :0

Your welcome. It seems in the time I took to figure out a workaround kleonc figured out a solution 😆

@akien-mga akien-mga added this to the 4.3 milestone Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants