Zylann / godot_voxel

Voxel module for Godot Engine
MIT License
2.69k stars 251 forks source link

TransvoxelMesher seems to generate invalid weight data on mesh #687

Closed NoTalkOnlyProx closed 2 months ago

NoTalkOnlyProx commented 2 months ago

Split from https://github.com/Zylann/godot_voxel/issues/685

Issue

Basic Setup

First of all, I am using VoxelGeneratorGraphs/OutputWeight. I am not sure whether that is part of the problem -- I went with this just as an early prototyping solution, since this is my first week using Godot (I'm a former Unity user from a decade ago :P) and I am deeply unfamiliar with this all.

I put together a very simple 2D noise + Heightmap SDF graph, and then mapped the height value (clamped to between 0 and 1) to the weight for layer 0 (L0) and the weight for layer 1 (L1) so that L1 = 1 - L0.

I then used a simple test shader to display the weight values on the mesh.

Expected Result

The expected result is that the red channel of the shader should be 0 near the bottom, and 1 near the top of the mesh. And the green channel should be 1 near the bottom, and 0 near the top.

Actual Result

Instead, I see something very different:

A screenshot of the terrain with the test shader modified to show only the x/r/L0 channel: image

void fragment() {
    ALBEDO = vec3(v_weights.x, 0, 0);
}

A screenshot of the terrain with the test shader modified to show only the y/g/L1 channel: image

void fragment() {
    ALBEDO = vec3(0, v_weights.y, 0);
}

So L0 is, correctly, 0 near the bottom. But on the way to the top, it shows a strange striation pattern?

Meanwhile, L1 is, correctly, 1 near the bottom. But then it shows a very similar striation pattern on the way up. But the dark regions are the same as for L0?

How to reproduce

  1. Create a new project
  2. Add a VoxelNode, set the generator to VoxelGeneratorGraphs and the mesher to VoxelMesherTransvoxel. Leave the streamer unset. image
  3. The VoxelMesherTransvoxel must have the texturing mode set to TEXTURES_BLEND_4_OVER_16.
  4. Add a material, and for that material, use the provided test shader.
  5. Build the generator graph shown in the test graph section.
  6. After you create each example, make certain to not only regenerate the terrain, but also remesh it afterwards. On rare occasions, regenerating is not enough.
  7. Please let me know if there is any additional required information for easy reproduction. I think I hopefully covered it all, though!

Test Shader

Here is the aforementioned test shader. It just outputs the first three encoded texture weights to RGB channels of albedo.

shader_type spatial;

varying vec4 v_indices;
varying vec4 v_weights;

vec4 decode_8bit_vec4(float v) {
    uint i = floatBitsToUint(v);
    return vec4(
        float(i & uint(0xff)),
        float((i >> uint(8)) & uint(0xff)),
        float((i >> uint(16)) & uint(0xff)),
        float((i >> uint(24)) & uint(0xff)));
}

void vertex() {
    v_indices = decode_8bit_vec4(CUSTOM1.x);
    v_weights = decode_8bit_vec4(CUSTOM1.y) / 16.0;
}

void fragment() {
    ALBEDO = v_weights.xyz;
}

This is adapted from the example here: https://voxel-tools.readthedocs.io/en/latest/smooth_terrain/#shader

But I changed the divisor from 255 to 16, since encode_weights_to_packed_u16_lossy already divides by 16 inherently.

I have verified manually that that v_indices.x == 0 and v_indices.y == 1 always, so none of what follows is due to index swapping.

Test Graph

Here is the test graph I was using. Some important notes for reproducing it:

image

Notes for recreating these:

So, am I just misusing this feature? I've read somewhere around 60% of the source code related to the texture data passing from generator to mesher to shader, and I'm pretty sure I grasp the overall concept, and that everything I have done is correct. But the code looks correct to me too, at least to the extent that I understand it. So it's really unclear to me right now where this problem is occuring.

686 probably isn't helping, though -- maybe fixing that first will make the problem here more obvious?

Environment

Monkeygogobeans commented 2 months ago

hi NTOP's friend here, I was able to recreate the issue so I can send it here while he is away. there are some differences as I don't have the exact clamp and curve values but the striations are indeed present.

strange_shader_reproduction.zip

MGilleronFJ commented 2 months ago

I haven't looked into it yet, but I remember something the mesher does:

https://github.com/Zylann/godot_voxel/blob/0417f38bf7519f9d4da358fabd974f81b8c81cb1/meshers/transvoxel/transvoxel.cpp#L259

Weights from voxels with signed distance > 0 (aka "air" voxels) are forced down to zero in order to prevent materials "in the air" from affecting ground (one of the side-effects of voxels that required some kind of decision). A side-effect of that is, if a vertex gets close, but doesn't touch the "marching cube corner" in which an air voxel is, its weight will interpolate towards 0 (yet not reach 0). This may be the cause of the "striation" you are seeing when looking at the raw weights.

But, you aren't actually supposed to use these weights as-is in your fragment shader. In fact, they won't even sum to 1 if you were to add them up, due to interpolation and heavy compression of the data. So you have to normalize them: https://github.com/Zylann/voxelgame/blob/9c0f1d805c4061dd60560b4cf57565fd0ba5d066/project/smooth_materials/smooth_texture_array_shader.gdshader#L49

I haven't seen that striation before and others have tried the texturing demo without seeing any, so that difference in your shader may be the reason you have this issue.

Zylann commented 2 months ago

hi NTOP's friend here, I was able to recreate the issue so I can send it here while he is away. there are some differences as I don't have the exact clamp and curve values but the striations are indeed present. strange_shader_reproduction.zip

I had a look at this test project.

The shader is wrong:

    v_weights = decode_8bit_vec4(CUSTOM1.y) / 16.0; // You should divide by 255.0, not 16.0

Also you should normalize weights like in the doc and demo, as mentionned earlier:

    float weights_sum = v_weights.x + v_weights.y + v_weights.z + v_weights.w + 0.001;
    vec4 weights = v_weights / weights_sum;

image

I suspect your graph is not doing what you think it does. Here it is, with preview nodes showing XY slices of the values you send to weight outputs: image Notice how they don't change vertically. You wanted to link height to weight. But that's not exactly what's happening. If you trace back all dependencies of your weight nodes, the Y coordinate never comes up. So it's always the same values at every height. Sure, noise gives the desired height from X and Z, but remember this is a voxel graph. It runs for every voxel along X, Z, and Y. So what this does essentially, is making all voxels "red" no matter which Y coordinate they are at, the closer they are from the peak of hills.

Now, it looks like it uses height from the result in the scene, because hills become redder the higher they are? Well, that's because you are seeing the noise itself, not the height of hills it produces. Noise already varies between -1 to 1, but much slower than you'd expect, and doesn't depend on Y, so it produces gradients that are stretched out thin and distorted. Then you get weight 0 very slowly going from 0 to maybe 0.5, and weight 1 going from 1 to 0.5. They end up showing quite mixed up all around. But also, because you used a Curve that maps 0..1 to 0..1, you're actually never getting values below 0, only in 0..1. ClampC does nothing. Maybe you did this intentionally, but just making sure.

With the previous fixes and changes to the graph, I get this: image This is just making all voxels "red" above Y=10, and green under Y=10, with a transition of 1 voxel in the middle (it could spread longer by dividing or multiplying values by a factor less than 1).

To which some noise can be added to vary the height where it transitions, but it is preferable to use a noise with a different seed (for reasons I'm not sure how to explain): image

With longer spread: image

NoTalkOnlyProx commented 2 months ago

I suspect your graph is not doing what you think it does. Notice how they don't change vertically. You wanted to link height to weight. But that's not exactly what's happening. Well, that's because you are seeing the noise itself, not the height of hills it produces. Maybe you did this intentionally, but just making sure.

Yes! It was my intent to directly view the noise image as weight data

Also you should normalize weights like in the doc and demo, as mentioned earlier:

I got rid of normalization on purpose so that I could test/validate the range of weight data being transferred from graph to shader -- ideally, if I pass the weight 1.0 to the node, I should see 1.0 output in the shader (well, 1.0 would technically not be encodeable, hypothetically the maximum would be 0.9375, but you catch my drift)

You should divide by 255.0, not 16.0

Ah, see, I initially did this due to a misunderstanding of your code. let me re-read and report back to you!

NoTalkOnlyProx commented 2 months ago

Okay, I see now that I just read your code wrong. I will delete my previous comments, and come back here when I am certain I actually read and understood what it is you have done. I apologize for all the pings.

Zylann commented 2 months ago

I got rid of normalization on purpose so that I could test/validate the range of weight data being transferred from graph to shader -- ideally, if I pass the weight 1.0 to the node, I should see 1.0 output in the shader (well, 1.0 would technically not be encodeable, hypothetically the maximum would be 0.9375, but you catch my drift)

Well, they dont, that's what I'm raising here. That discrepancy is deliberate, in part because the shader should normalize anyways.

NoTalkOnlyProx commented 2 months ago

Well, they dont, that's what I'm raising here. That discrepancy is deliberate, in part because the shader should normalize anyways.

You're quite right, yep. That appears to be the source of the rings, my bad! I also missed that you had left another comment basically explicitly laying this out. My bad on that as well.

Unfortunately in the process of figuring that out, I've spotted another discrepancy, but I'll close this ticket and file another one, after I play with it some more