Improve Region Blend, gizmos, shaders#899
Conversation
|
Note, the color map is getting injected in #842. |
|
We could optionally generate new regions at ground level; and maybe the world noise offset level. Though I'm not terribly concerned about leaving it at 0. |
8816be7 to
fcda93b
Compare
Do we need this? I'd like to drop the classes. The gizmos are ugly. As long as we can render everywhere we need, is there any reason to keep them? |
a08d5db to
b8d3312
Compare
b8d3312 to
f8ff8ca
Compare
|
The speed boost is great. I don't see any, but are there any artifacts due to vertical projection? What happens with faces exactly at 45? Or when a grass slope starts off <45 and curves up >45? Is there a seam? |
The condition is checked against each index normal, (not the smooth interpolated terrain normal) and then the biliniear blend prevents seams. So there are no severe stretching artifacts like before, if the threshold was too high/low. |
a20f57c to
73641d9
Compare
491e304 to
4f04ede
Compare
TokisanGames
left a comment
There was a problem hiding this comment.
Wow, really nice work on the region blend, matching collision and the control we have over the edge of regions now. This makes sculpting down under the ocean a breeze.
I'm also very glad to be rid of the region gizmo. We can finally keep view gizmos turned on without seeing all of those annoying lines.
Some notes for improvement:
One of these is a region, the rest are not. What about making the grid near non-regions a dark/middle grey say around .333-.4, and white around regions?

This addition to the material is great:
I don't think these debug views are very useful. I was thinking these checkboxes would turn on and off the above material settings, and what they currently do goes away. They could be renamed, height dropped, etc.
|
Yeah, i can look at making noise blend regions similar to flat. None is problematic, but now that its a seperate insert, I could investigate it a bit more. I suspect having tesselation enabled doesnt work well with the current fix. |
4f04ede to
57d13d7
Compare
|
NOISE background is now purely background (fixed blend, noise always has ZERO effect on in-region heights), and has the same features as FLAT mode. The PBR debug views, I have found useful on occasion, for checking normals, heights etc are packed correctly. They are tucked away at the bottom of the debug views so I think its fine if they stay. None mode I think could be a seperate issue, as it might need some investigating again. |
8a8a4a7 to
ee3a03f
Compare
5efdd4f to
2ddf03a
Compare
TokisanGames
left a comment
There was a problem hiding this comment.
Really great work, as always. I fixed some broken things in the minimum shader, and adjusted categories in the shader uniforms and inspector parameters.
One pending issue is if the Region tool is selected, it shows the grid. If Terrain3D is deselected, the grid goes away. If reselected, it does not show the grid.
src/terrain_3d_material.h
Outdated
| UNIFORMS = 0u, | ||
| TEXTURE_ARRAYS = 1u, | ||
| REGION_ARRAYS = 2u, | ||
| FULL_REBUILD = TEXTURE_ARRAYS | REGION_ARRAYS, |
There was a problem hiding this comment.
This structure is a little confusing.
- UNIFORMS implies that it's optional, meaning update(TEXTURE_ARRAYS) won't include UNIFORMS, but it does in the activity.
- 0 isn't a bit and should mean no flags are set. If UNIFORMS was optional it should be a bit. NONE or UNIFORMS_ONLY might be more clear. WorldBackground::NONE already exists and we might have an issue with the same name, especially in C#. Maybe I'll just do UNIFORMS_ONLY.
- The values should look like bits, eg, 1<<1 or 0x1, 0x2, 0x4
- Full Rebuild updates both the arrays, AND rebuilds the shader. There's no option to just do the arrays. I also added a new bit for full_rebuild, allowing doing the arrays only.
src/terrain_3d_material.h
Outdated
| void _update_uniforms(const RID &p_material, const uint32_t p_update = UpdateFlags::UNIFORMS); | ||
| void _set_shader_parameters(const Dictionary &p_dict); | ||
| Dictionary _get_shader_parameters() const { return _shader_params; } | ||
| static inline bool _has_update_flag(uint32_t value, UpdateFlags flag) { return (value & static_cast<uint32_t>(flag)) != 0u; } |
There was a problem hiding this comment.
I don't think we need this. if (value & flag) is fine. If we need more type safety we could look at moving to C++20 and enum class and these helper functions. But this is only the first flags enum in the class.
| map_data.resize(hshape_size * hshape_size); | ||
| real_t min_height = FLT_MAX; | ||
| real_t max_height = FLT_MIN; | ||
| real_t max_height = -FLT_MAX; |
There was a problem hiding this comment.
FLT_MIN is not negative!? OMG






Uh oh!
There was an error while loading. Please reload this page.