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

Implements vertex spacing (mesh density) #296

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

lfxu
Copy link
Contributor

@lfxu lfxu commented Jan 6, 2024

Admin edit:
Fixes #131
Fixes #137


Implements #131.

Opted for naming "vertex spacing" because it has the least ambiguity.

@TokisanGames
Copy link
Owner

TokisanGames commented Jan 6, 2024

If this doesn't scale the overall terrain size, then it means 1px != 1 vertex, and we're losing detail. If mesh resolution increases to 2, then a 1024px means 2048m. If this only changes the mesh but not the overall size then we're paying for 1024 memory, but only using 25% of it. 75% waste is not good.

I think it should scale the mesh in the shader to match.

Also I don't see it changing the collision shape at all. Turn on debug collision so you can see it. It needs to be pixel perfect.

@Saul2022
Copy link

Saul2022 commented Jan 6, 2024

Where is the option at? i built it from source, with your branch and it doesn´t seem to appear neither in the mesh or the renderer section.

@lfxu
Copy link
Contributor Author

lfxu commented Jan 7, 2024

Rendering and editor are now scaled. Navigation and collision todo.

@lfxu
Copy link
Contributor Author

lfxu commented Jan 7, 2024

Navigation and collision should now work too. Let me know if anything is still missing.

Note: for larger vertex spacing the user may need to increase NavMesh resource cell size.

@lfxu
Copy link
Contributor Author

lfxu commented Jan 7, 2024

Where is the option at? i built it from source, with your branch and it doesn´t seem to appear neither in the mesh or the renderer section.

It's in the mesh section, under LOD and size. Make sure you replace the right files in your project and restart Godot.

@Saul2022
Copy link

Tested ( now after finally being able to make it build after fail and error) and it works as expected, the terrain m scales by it double when changing from for instance, 1 to 2, even with multiple regions it seem´s to work well.

Copy link
Owner

@TokisanGames TokisanGames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this. I tested it and it works well with collision, navigation, and occlusion. Good job so far! This is a highly requested feature. Here are some issues I found.

  • The cursor decal disappears at some angles and heights, and the current tool doesn't work or is offset from the decal at different angles and locations. I tested and found these issues with both 0.5 and 2.0. Use the height brush on the demo and you'll see it disappear above a certain height. Rotate the camera around and add new regions to see it.

  • Please explain in detail the changes you made to the shader.

  • The normals are off. The first image here has a directional light facing directly down. Here the mesh has a scale of 1.

image

Here is a different section where the mesh is scaled at 10. I sculpted it to have a similar height and slope as the other. With the light straight down it should look like the above.

image

src/terrain_3d.cpp Outdated Show resolved Hide resolved
src/terrain_3d.h Outdated Show resolved Hide resolved
src/terrain_3d.cpp Outdated Show resolved Hide resolved
Copy link
Contributor Author

@lfxu lfxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issues should be fixed now. Please take a look.

src/shaders/main.glsl Show resolved Hide resolved
src/shaders/main.glsl Show resolved Hide resolved
src/shaders/main.glsl Show resolved Hide resolved
src/shaders/main.glsl Show resolved Hide resolved
src/terrain_3d.cpp Outdated Show resolved Hide resolved
src/terrain_3d.cpp Outdated Show resolved Hide resolved
src/terrain_3d.h Outdated Show resolved Hide resolved
@lfxu lfxu closed this Jan 22, 2024
@lfxu lfxu deleted the vertex-spacing branch January 22, 2024 19:59
@lfxu lfxu restored the vertex-spacing branch January 22, 2024 20:00
@lfxu lfxu reopened this Jan 22, 2024
src/shaders/main.glsl Outdated Show resolved Hide resolved
@@ -61,9 +63,11 @@ struct Material {
};

varying vec3 v_vertex; // World coordinate vertex location
varying vec3 v_camera_pos;
varying flat vec3 v_camera_pos;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this flat?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not? Does view space to world space transform matrix change per vertex?

Copy link
Owner

@TokisanGames TokisanGames Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't really familiar with it. I see it disables interpolation, not truncates to integer, which is what it sounds like. So why not also make v_vertex and v_vertex_dist flat also? All of the varyings we're using currently are there to provide fixed vertex data. I see no visual or performance difference when I try it, but it's good to be explicit.

@TokisanGames
Copy link
Owner

Thanks for fixing the highlighted issues. Please address the individual inline notes I mentioned, as well the following and I'll merge it in:

  • Fix region grid and vertex grid in debug_views.glsl, and any other usages of UV/UV2.
  • Restore v_vertex height
  • Rebase and resolve conflicts

@TokisanGames
Copy link
Owner

TokisanGames commented Jan 29, 2024

Ok, found a few more errors.

  • Decal wasn't scaling. I fixed it.
  • New slope sculpting doesn't pick the correct heights. At vertex spacing 1, use the slope sculpting brush, pick a peak, pick a plane, it makes a ramp. Repeat and the ramp gets stronger.
    • At larger spacing, the higher peak decreases in height each iteration. It should stay the same.
    • At smaller spacing, the higher end increases in height.
    • The flatten brush height picker seems fine.
  • Detecting the mouse cursor has an odd hiccup at some camera angles, when scaling is above 1. Set to 2 or more, use the color map to paint red, pick a flat area and start making vertical streaks and you'll see it jump. The mouse is already clunky enough. I have some ideas to improve it in Cursor flickers, is unstable and inaccurate up close #121. But the oddity in this PR needs to be addressed.
paintbrush.gap.mp4
  • Also, I've gotten this a few times when painting on the colormap and it scaled up. However I'm not able to consistently produce it.
ERROR: Index p_y = 1024 is out of bounds (height = 1024).
   at: get_pixel (core/io/image.cpp:3280)

@lfxu
Copy link
Contributor Author

lfxu commented Jan 30, 2024

These don't seem introduced by this PR.

  1. The slope tool always decreases a little bit. This is on main.
  2. Drawing has a gap at distance, in a round shape, due to a branch in editor.gd:
    if intersection_point.x < 3.4e38:

See the reproductions on main.
draw.webm
slope.webm

These issues should be addressed elsewhere.

A real issue though is the slope decals are at wrong locations. I fixed them.

@TokisanGames
Copy link
Owner

The slope sculpting was still broken due to mouse_global_position.y also being scaled when it shouldn't have been. That's now fixed.

I also made all of the varyings flat.

The mouse cursor jump can be addressed in #121.

This is ready to go. Great work. Thank you!

@lfxu
Copy link
Contributor Author

lfxu commented Jan 31, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

Android Texture Artifacts Adjustable mesh density
3 participants