Skip to content

Fix texture coords off by one error #16055

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

Closed
wants to merge 3 commits into from

Conversation

cx384
Copy link
Contributor

@cx384 cx384 commented Apr 21, 2025

While investigating #12266 I noticed that align_style does not align properly, even in the most basic case. How did nobody complain about this so far. To be fair it took quite some time to find the cause.
Anyway, if I'm not wrong, texture coordinates of all node tiles (except the bottom one) have an off by one error which this PR tries to fix.

(I hope I didn't misunderstand anything about the code, since this part worked fine for the past 8 years.)

To do

Ready for Review.

How to test

  • Start devtest
  • Press the key to toggle block bounds
  • Place a lot of tiled:tiled nodes and see that the world alignment is correct.
  • See that all other nodes look the same, including rotations

@Desour
Copy link
Member

Desour commented Apr 21, 2025

This whole stuff is a mess. We're using world (or modulo 16?) coordinates and use them for texture coordinates. This completely breaks tilable_vertical=false (already in master) (but for some reason not for solid nodes (why? am I blind)). And then there's the mesh collector that applies a scale to texture coordinates, which is also a quite weird implementation.
I tried to fix and implement it properly (https://github.com/Desour/minetest/tree/fix_tileable_vertical), but gave up.

For testing:

diff --git a/games/devtest/mods/stairs/init.lua b/games/devtest/mods/stairs/init.lua
index 267540e8d..59f64dec7 100644
--- a/games/devtest/mods/stairs/init.lua
+++ b/games/devtest/mods/stairs/init.lua
@@ -61,6 +61,6 @@ stairs.register_stair_and_slab("desert_stone", "basenodes:desert_stone",
 
 stairs.register_stair_and_slab("cobble", "basenodes:cobble",
 		{cracky=3},
-		{"default_cobble.png"},
+		{{name = "default_cobble.png", tileable_vertical = false}},
 		"Cobblestone Stair",
 		"Cobblestone Slab")

Now this PR breaks for example allfaces nodes with tilable_vertical=false and visual_scale=1.1.

@cx384
Copy link
Contributor Author

cx384 commented Apr 21, 2025

I just tested this, for me, tileable_vertical=false breaks textures both in this PR and in master (independent of visual_scale=1.1). In master all faces except the top one are broken and with this PR it's the other way around.
tileable_vertical is currently not documented anyway, so mods are not supposed to change it. Is there even an usecase?

If I'm not wrong texture coordinates are mainly used for two things,
tile rotation see here:

v2f &tcoords = vertex.TCoords;
switch (tile.rotation) {
case TileRotation::None:
break;
case TileRotation::R90:
tcoords.set(-tcoords.Y, tcoords.X);
break;
case TileRotation::R180:
tcoords.set(-tcoords.X, -tcoords.Y);
break;
case TileRotation::R270:
tcoords.set(tcoords.Y, -tcoords.X);
break;
}

and for initial mirroring for each of the 6 faces.
(As I wrote, the bottom one is the only correct one in master, all others get mirrored and increased by +1 for whatever reason.)

For reference, this is where the alligne_style gets applied to the texture coordinates

f32 scale = 1.0f;
if (use_scale)
scale = 1.0f / layer.scale;

@Desour
Copy link
Member

Desour commented Apr 22, 2025

I just tested this, for me, tileable_vertical=false breaks textures both in this PR and in master (independent of visual_scale=1.1).

Hm, tested again, and now it's broken iff visual_scale is not set.
I guess I did a mistake in testing, sorry!

tileable_vertical is currently not documented anyway, so mods are not supposed to change it. Is there even an usecase?

It controls what happens when the texture is sampled out-of bounds, if true, the texture is repeated, if false, it's clipped to edge.
You need this for dirt with grass for example, because you'd otherwise sometimes be able to see a brown strip at the top.
(Can be noticed with anisotropic filtering and looking from an angle, for example. (But it somehow is now much less visible than I have it in memory.) Undersampling can also make it easier to see. (In devtest, use dirt with snow, not with grass, to reproduce.))

(As I wrote, the bottom one is the only correct one in master, all others get mirrored and increased by +1 for whatever reason.)

If I understand it correctly, the mirroring is to get the right texture orientation, i.e. on the side you want to have upright textures (y up in 2d should be y up in 3d).
The 1 offset would make sense if the box's coords were in [0,1] bounds, to get the texture coords in [0,1] range. (And with visual_scale set, this should be the case.)

@cx384
Copy link
Contributor Author

cx384 commented Apr 22, 2025

Fair enough, so our initial mirroring preserves texture coord bounds, but later rotations don't, interesting. Is this another bug?

From your response, I infer that we want to preserve the texture coord bounds after mirroring, so I adjusted it accordingly.

@sfan5
Copy link
Collaborator

sfan5 commented Apr 24, 2025

I think someone needs to sit down and re-do the texture coordinate stuff.
I noticed during #15968 that our texture coords are all over the place:
grafik
The normal range is [0,1] and yet we have all this?! (normal stone surface, no world_align, no scale)

@cx384
Copy link
Contributor Author

cx384 commented Apr 27, 2025

Well, our align style implementation needs texture coords at least in [0,16].
(I have no idea why it has been implemented this way)

There are so many weird things going on.
Even the initial adjustments in generateCuboidTextureCoords should not be necessary.
Maybe I will look into this.

(This PR is only meant to fix the align style texture coords problem.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combining animated & world-aligned textures has alignment issues.
3 participants