godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.98k stars 21.16k forks source link

Non-matching texture filtering mode for cursor preview in new tilemap editor #52332

Open HybridEidolon opened 3 years ago

HybridEidolon commented 3 years ago

Godot version

4.0.dev.custom_build [a160a95ea]

System information

W10, Vulkan, NVIDIA GeForce GTX 1080

Issue description

When drawing on tilemaps using the new tilemap feature, the transparent preview of the tile under the drawing cursor always uses linear filtering.

It looks like this when the TileMap's filter mode is set to Nearest:

image

It should probably match whichever filter mode the TileMap is assigned to use, rather than assuming Linear.

Steps to reproduce

  1. Create a tileset with a tile.
  2. Create a tilemap node, set its filtering mode to Nearest, and use the tileset to paint that tile.
  3. Compare the tile preview to the painted tiles.

Minimal reproduction project

No response

groud commented 3 years ago

I investigated the issue, but this will most likely be a wontfix for now.

The TileMapEditor node makes use of the forward_canvas_draw_over_viewport, which, as its name suggests, draw things on a CanvasItem node provided by the CanvasItemEditor. However, the filtering status is for the whole node, so changing the filter mode on this control would lead to filtering changed for everything that is drawn on this CanvasItem (by other plugins, or the CanvasItemEditorPlugin too).

Solutions might be to either allow changing the filtering on the fly (similar to what we do with drawing transforms for example), or to change the plugin API to be able to add a new Control on top of the CanvasItemEditor node (instead of using callbacks like forward_canvas_draw_over_viewport).

KoBeWi commented 3 years ago

be able to add a new Control on top of the CanvasItemEditor node

What if, instead of adding the Control to editor, you add it to the TileMap and set as toplevel? Then move the control inside draw callback, so it acts as pseudo-cursor.

groud commented 3 years ago

What if, instead of adding the Control to editor, you add it to the TileMap and set as toplevel? Then move the control inside draw callback, so it acts as pseudo-cursor.

It makes things likely more complex and it bloats the tree for an editor-only feature. So I would avoid this.

KoBeWi commented 3 years ago

If you don't set the owner of the "cursor" node, it will be hidden and effectively editor-only (users won't even be aware of its existence if that's your concern).

groud commented 3 years ago

If you don't set the owner of the "cursor" node, it will be hidden and effectively editor-only (users won't even be aware of its existence if that's your concern).

That would be the case if they could not could be accessed using tool scripts. Implementing such workarounds modifying the tree structure may have significant impacts on a lot of things, so I don't want a workaround that could trigger quite tricky situations.

And in any case, this is definitely not a clean way to solve the issue, and, considering how the issue is not so bad, I won't go for such a workaround.

KoBeWi commented 3 years ago

That would be the case if they could not could be accessed using tool scripts.

Which is possible after #30391 btw

IsaacMarovitz commented 2 months ago

Still present in Godot 4.3

demolen commented 1 week ago

and still present in 4.4 dev.