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

Some drawtypes cause z-fighting underwater since 5.8.0-dev #13569

Closed
Desour opened this issue Jun 7, 2023 · 14 comments · Fixed by #13594
Closed

Some drawtypes cause z-fighting underwater since 5.8.0-dev #13569

Desour opened this issue Jun 7, 2023 · 14 comments · Fixed by #13594
Labels
Bug Issues that were confirmed to be a bug @ Client rendering Regression Something that used to work no longer does
Milestone

Comments

@Desour
Copy link
Member

Desour commented Jun 7, 2023

Minetest version
Minetest 5.8.0-dev-1b51ff333 (Linux)
Using Irrlicht 1.9.0mt11

Does not happen with:

Minetest 5.7.0 (Linux)
Using Irrlicht 1.9.0mt10

Bisected, issue exists since: 35929d2
cc @numberZero

OS / Hardware

GPU model: AMD vega 8
OpenGL version: OpenGL 4.6

Summary

Nodes with drawtype glasslike, glasslike_framed, allfaces, and possibly more z-fight when placed underwater (in a pool of liquid nodes (tested with liquid source nodes)).
This does not happen in 5.7.0.

(This was originally reported in #13499 by @runsy. Reopened because issue template was not used.)

Steps to reproduce
  • Start devtest.
  • Get a glasslike node (or whatever you want to test) from the chest of everything.
  • Place it underwater.
  • Look at it.
  • Do the same in 5.7.0.
@Desour Desour added Bug Issues that were confirmed to be a bug @ Client rendering Regression Something that used to work no longer does labels Jun 7, 2023
@ghost
Copy link

ghost commented Jun 7, 2023

Not only flowing liquids.

imagen

imagen

This only-one-face nodebox shows the same issue.

It is not a trivial issue.

@ghost
Copy link

ghost commented Jun 7, 2023

The vine is defined here:

minetest.register_node(creeper_name, {
description = S(def.desc),
drawtype = "nodebox",
walkable = false,
paramtype = "light",
paramtype2 = "facedir",
tiles = {tile},
inventory_image = inventory_image,
wield_image = def.wield_image or def.inventory_image or def.tile,
use_texture_alpha = "clip",
node_box = node_box,
groups = {
snappy = 2, flammable = 3, oddly_breakable_by_hand = 3, choppy = 2, carpet = 1, leafdecay = 3, leaves = 1,
},
sounds = sound.leaves()
})

@NathanSalapat
Copy link
Contributor

Your definition doesn't help us runsy, as you haven't included what the tiles or node_box tables are.

@Desour Desour added this to the 5.8.0 milestone Jun 8, 2023
@ghost
Copy link

ghost commented Jun 8, 2023

Your definition doesn't help us runsy, as you haven't included what the tiles or node_box tables are.

It is a flat pane:

helper.nodebox.flat_b = {
type = "fixed",
fixed = {
{-0.5, -0.5, 0.5, 0.5, 0.5, 0.5}, -- Flat Plane Back
},
}

@lhofhansl
Copy link
Contributor

lhofhansl commented Jun 9, 2023

@numberZero Any ideas?

Interestingly drawGlasslikeNode and drawGlasslikeFramedNode have not changed.

Maybe this: 35929d2#diff-4806d13ea7cfd098b8fc25a44c74c48d0367563df45bdee925277960f11c15cbL813
And/or this: 35929d2#diff-b76715a8a8224ffb4118b092d53887bec3e236111ef85d15ad151dbc7984b39cR473
?

@SmallJoker
Copy link
Member

SmallJoker commented Jun 9, 2023

The provided example by runsy uses a nodebox which cannot be solved easily because Minetest currently has to draw all faces in case the nodebox has an opening (see-though). Hence that particular issue cannot be fixed yet.

The z-fighting of framed glass or similar transparent nodes can be fixed by enabling backface culling which still causes incorrectly rendered glass back sides due to cumulated semi-transparent textures:
glass vs glass under water

However, flowing liquid does yet not support backface culling. Either you get z-fighting (left) or incorrect colorizing (right), see variable draw_liquid_bottom

z fightning vs seethough

I did yet not check how 5.7.0 looked like but similar issues might occur there as well.

Diff used for testing:

diff --git a/src/client/content_mapblock.cpp b/src/client/content_mapblock.cpp
index 0274c767b..a9afb76f6 100644
--- a/src/client/content_mapblock.cpp
+++ b/src/client/content_mapblock.cpp
@@ -461,7 +461,7 @@ void MapblockMeshGenerator::drawSolidNode()
                                        continue;
                                if (n2 == nodedef->getId(f->liquid_alternative_source))
                                        continue;
-                               backface_culling = f2.solidness >= 1;
+                               backface_culling = true;
                        }
                }
                faces |= 1 << face;
diff --git a/src/nodedef.cpp b/src/nodedef.cpp
index 383edae36..55e154596 100644
--- a/src/nodedef.cpp
+++ b/src/nodedef.cpp
@@ -858,21 +858,21 @@ void ContentFeatures::updateTextures(ITextureSource *tsrc, IShaderSource *shdsrc
                is_liquid = true;
                break;
        case NDT_FLOWINGLIQUID:
-               solidness = 0;
+               solidness = 1;
                if (tsettings.opaque_water)
                        alpha = ALPHAMODE_OPAQUE;
                is_liquid = true;
                break;
        case NDT_GLASSLIKE:
-               solidness = 0;
+               solidness = 1;
                visual_solidness = 1;
                break;
        case NDT_GLASSLIKE_FRAMED:
-               solidness = 0;
+               solidness = 1;
                visual_solidness = 1;
                break;
        case NDT_GLASSLIKE_FRAMED_OPTIONAL:
-               solidness = 0;
+               solidness = 1;
                visual_solidness = 1;
                drawtype = tsettings.connected_glass ? NDT_GLASSLIKE_FRAMED : NDT_GLASSLIKE;
                break;

On a side note, nodedef->getId(f->liquid_alternative_flowing) should probably be cached.

@Desour
Copy link
Member Author

Desour commented Jun 9, 2023

For comparison:
5.7.0:
screenshot_20230610_001939
master:
screenshot_20230610_002017

And here another screenshot (150% nodebox from devtest, a flat nodebox as described by runsy, allfaces but without backface culling):
screenshot_20230610_003554
It looks the same in 5.7.0 and master. I can not reproduce a regression with the flat nodebox.


Could we maybe move all the faces of full nodes an eps inwards?

@ghost
Copy link

ghost commented Jun 10, 2023

To check the flat nodebox dissapearing under certain angles:

fixed = {
	{-0.5, -0.5, 0.5, 0.5, 0.5, 0.5}, -- Flat Plane Back
},

use_texture_alpha = "clip",

Texture:

floraz_jungle_creeper

@ghost
Copy link

ghost commented Jun 10, 2023

Well, i have solved the flat pane use changing from "clip" to "blend":

use_texture_alpha = "blend",

@numberZero
Copy link
Contributor

The underlying issue is well-known, it is #9276. Sorry that my PR exaggerated it, need to force some backface culling probably, like it was in FastFaces.

Could we maybe move all the faces of full nodes an eps inwards?

No, that would cause other problems. What we could is to use glPolygonOffset as was suggested in #9276.

@Desour
Copy link
Member Author

Desour commented Jun 13, 2023

The underlying issue is well-known, it is #9276.

Just to make clear: This issue also happens without waving liquids.

Could we maybe move all the faces of full nodes an eps inwards?

No, that would cause other problems.

Just out of interest, what are these problems? 👀

@lhofhansl
Copy link
Contributor

So fast-faces had some logic where within a scan column it found adjacent "similarly transparent" nodes and would cull those backfaces. That would not work between these columns, and hence just eliminate some of the z-fighting only... Assuming I understand the old code correctly.

Getting rid of fast-faces is still the right thing, IMHO. Not sure how to fix it if we do not want to search the space of adjacent transparent nodes to find backfaces that could be culled.

Or ... maybe I misunderstand :)

@numberZero
Copy link
Contributor

This issue also happens without waving liquids.

Like #6903 (comment)? That should’ve been fixed... but isn’t.
One problem is that overlaid quads are split across different diagonals. Fixing that fixes most z-fighting, but not all. I suspect the vertex coordinates aren’t exact integers like they should be.

Could we maybe move all the faces of full nodes an eps inwards?

No, that would cause other problems.

Just out of interest, what are these problems? 👀

I should’ve written “may”. It may cause gaps at edges due to rounding errors. Not a major problem, FastFaces had it all along. But there may be something else I simply don’t remember right now.

Not sure how to fix it if we do not want to search the space of adjacent transparent nodes to find backfaces that could be culled.

To my understanding, search space remains the same: it has to check for adjacent solid nodes already. Only checks will change a bit.

@Desour
Copy link
Member Author

Desour commented Jun 13, 2023

This issue also happens without waving liquids.

Like #6903 (comment)? That should’ve been fixed... but isn’t. One problem is that overlaid quads are split across different diagonals. Fixing that fixes most z-fighting, but not all. I suspect the vertex coordinates aren’t exact integers like they should be.

Umm, I don't usually see the glitches shown in those screenshots.

It may cause gaps at edges due to rounding errors.

Oh right, I forgot that inner edges exist. x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug @ Client rendering Regression Something that used to work no longer does
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants