Add manual slope painting tool #456
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you. Please see issue #282. This should be a tool setting, similar to paintable scale and rotation, not a tool. That will allow it to provide a slider to adjust slope amount, or invert it, and affect all relevant tools such as overlay, base, color, wetness, holes, navigation, etc. |
toolsetting.mp4 |
|
This looks really really helpful, thank you so so much for putting in the time to make this! |
|
Thank you for the changes. I tested it today. It's not very intuitive so I need to think about it and how to improve it or document it. Some initial notes:
|
Yes |
|
It now uses degrees as an input when it's set to 0, slope painting is disabled. |
|
Oh, you definitely do not want to ever merge. That makes the commit history messy and breaks this PR. The proper operation is to rebase using the Godot PR Workflow linked in our contributing document. However that merge commit you just made needs to be removed first before we can rebase or merge this PR. |
Ryan-000
force-pushed
the
paint_slope
branch
2 times, most recently
from
e4a8303
to
52e47c9
Compare
TokisanGames
changed the title
Xtarsia
reviewed
Sep 16, 2024
|
Hi - I just wanted to check in, is there any way I can help out with this? I'm super excited to see this as a feature, it already seems incredibly helpful. |
|
It needs to be finished to work with the instancer. Now that I'm done with regions, I can work on this sometime before the next release. If you want to help, you could do the legwork on the needed code. Should just be a matter of checking the slope and not calling the instancer, done all within Terrain3DEditor. It's probably 2-4 lines of code. |
|
Great! If I get the chance (I have some IRL things tomorrow) I'll take a shot at it. Thanks for the update. If I get it working, should I comment with a patch or request changes? |
|
Comment here. Only Op and I can make changes to the PR. |
|
I did not get the chance to work on this tonight as I got super swamped today, but I will do my best to get things working tomorrow. |
|
Here's what I've gotten working: I put the diff within this fold-out to keep things less cluttered.diff --git a/project/addons/terrain_3d/src/ui.gd b/project/addons/terrain_3d/src/ui.gd
index f8d170a..de97f32 100644
--- a/project/addons/terrain_3d/src/ui.gd
+++ b/project/addons/terrain_3d/src/ui.gd
@@ -220,10 +220,9 @@ func _on_tool_changed(p_tool: Terrain3DEditor.Tool, p_operation: Terrain3DEditor
to_show.push_back("remove")
Terrain3DEditor.INSTANCER:
- # if you want, you can add this, but you'll have to modify terrain_3d_instances.cpp to support it
- # eg, it would be useful to avoid placing instances on steep hills
- #to_show.push_back("invert_slope_paint")
- #to_show.push_back("minimum_slope_paint_angle_degrees")
+ to_show.push_back("invert_slope_paint")
+ to_show.push_back("minimum_slope_paint_angle_degrees")
+ set_menu_visibility(tool_settings.slope_list, true)
to_show.push_back("size")
to_show.push_back("strength")
set_menu_visibility(tool_settings.height_list, true)
diff --git a/src/terrain_3d_editor.cpp b/src/terrain_3d_editor.cpp
index 2e61e81..db003bc 100644
--- a/src/terrain_3d_editor.cpp
+++ b/src/terrain_3d_editor.cpp
@@ -297,7 +297,7 @@ void Terrain3DEditor::_operate_map(const Vector3 &p_global_position, const real_
// Shader scale array is aligned to match this.
std::array<uint32_t, 8> scale_align = { 5, 6, 7, 0, 1, 2, 3, 4 };
- if (!_can_operate_on_slope(brush_global_position)) {
+ if (!can_operate_on_slope(brush_global_position)) {
continue;
}
@@ -409,7 +409,7 @@ void Terrain3DEditor::_operate_map(const Vector3 &p_global_position, const real_
dest = Color(as_float(bits), 0.f, 0.f, 1.f);
} else if (map_type == Terrain3DStorage::TYPE_COLOR) {
- if (!_can_operate_on_slope(brush_global_position)) {
+ if (!can_operate_on_slope(brush_global_position)) {
continue;
}
@@ -445,7 +445,7 @@ void Terrain3DEditor::_operate_map(const Vector3 &p_global_position, const real_
storage->add_edited_area(edited_area);
}
-bool Terrain3DEditor::_can_operate_on_slope(const Vector3 &p_brush_global_position) {
+bool Terrain3DEditor::can_operate_on_slope(const Vector3 &p_brush_global_position) {
const real_t minimum_slope_paint_angle_degrees = _brush_data["minimum_slope_paint_angle_degrees"];
const bool invert_slope_paint = _brush_data["invert_slope_paint"];
diff --git a/src/terrain_3d_editor.h b/src/terrain_3d_editor.h
index f09caa3..438d0d1 100644
--- a/src/terrain_3d_editor.h
+++ b/src/terrain_3d_editor.h
@@ -82,7 +82,6 @@ private:
void _region_modified(const Vector3 &p_global_position, const Vector2 &p_height_range = Vector2());
void _operate_region(const Vector3 &p_global_position);
void _operate_map(const Vector3 &p_global_position, const real_t p_camera_direction);
- bool _can_operate_on_slope(const Vector3 &p_brush_global_position);
bool _is_in_bounds(const Vector2i &p_position, const Vector2i &p_max_position) const;
real_t _get_brush_alpha(const Vector2i &p_position) const;
Vector2 _get_uv_position(const Vector3 &p_global_position, const int p_region_size) const;
@@ -109,6 +108,7 @@ public:
void operate(const Vector3 &p_global_position, const real_t p_camera_direction);
void stop_operation();
bool is_operating() const { return _pending_undo; }
+ bool can_operate_on_slope(const Vector3 &p_brush_global_position);
protected:
static void _bind_methods();
diff --git a/src/terrain_3d_instancer.cpp b/src/terrain_3d_instancer.cpp
index 8dbcac0..6b5248a 100644
--- a/src/terrain_3d_instancer.cpp
+++ b/src/terrain_3d_instancer.cpp
@@ -222,6 +222,8 @@ void Terrain3DInstancer::add_instances(const Vector3 &p_global_position, const D
real_t random_hue = CLAMP(real_t(p_params.get("random_hue", 0.f)) / 360.f, 0.f, 1.f); // degrees -> 0-1
real_t random_darken = CLAMP(real_t(p_params.get("random_darken", 0.f)) * .01f, 0.f, 1.f); // 0-100%
+ Terrain3DEditor* editor = _terrain->get_editor();
+
TypedArray<Transform3D> xforms;
TypedArray<Color> colors;
for (int i = 0; i < count; i++) {
@@ -240,6 +242,10 @@ void Terrain3DInstancer::add_instances(const Vector3 &p_global_position, const D
position.y = height;
}
+ if (!editor->can_operate_on_slope(position)) {
+ continue;
+ }
+
// Orientation
Vector3 normal = Vector3(0.f, 1.f, 0.f);
if (align_to_normal) {
I made Super happy to help out! |
|
Ahh I'm so sorry. I had forgotten about that use-case. I'll do my best to revise it to work properly. |
|
I may be misunderstanding something here, but it seems as though putting the call there would not work as intended. Since that section of terrain_3d_editor works based off the brush position (rather than the per-instance position) it would just filter the entire brush based off its center rather than each instance based off its position. Was that the intended behavior, or am I misinterpreting your advice? |
|
That's probably true. Alright then I'd say @az-raven What would help us move closer to releasing 0.9.3 is helping to wrap up the things in the left column on the project board. Any of the bottom 5 issues would be great to tackle. |
|
Great! I'll do my best to help out with those as soon as I handle my homework for the week. |
TokisanGames
force-pushed
the
paint_slope
branch
from
52e47c9
to
1e565a1
Compare
|
@Ryan-000 Thanks for taking it on. The improvements are much better and simpler. The priority for this has finally come up. I don't think there's any need for slope on auto/navigation/holes. It will just lead to missing vertices that don't get filled in. I limited it to paint, spray, color, wetness, foliage. This update:
Todo:
|
Hi - I don't mean to intrude, but I feel as though slope filtering on navigation would be extremely useful for a variety of use cases. I believe it would particularly simplify the workflow for large terrains or when trying to be consistent with what areas are navigable. That said, I would still love to hear your thoughts on the matter. |
|
Godot navigation is not robust. One shouldn't just draw a massive area covering all paths and pass that to Godot and expect it will generate a good result. It needs to be refined a bit, and segmented into areas. Second, slope occurs per pixel, so will produce non-contiguous application. This is fine for texturing but terrible for holes and navigation generation. Third, when users remove slope enabled navigation, they're going to leave pixels behind. We don't need to create more problems and complaints due to the navigation server doing weird things because we're feeding it garbage data. |
|
That makes sense. Thank you for the clarification! |
TokisanGames
force-pushed
the
paint_slope
branch
from
1e565a1
to
bdb16a3
Compare
|
Updated the slope method based on discord discussions. This has resulted in a double slider that eliminates the need for invert. Set the range you want and paint. It's the most intuitive option I've seen so far. Please try it out in the push build (edit: Oh, actually Ryan has his builds disabled and I haven't enabled pull request builds yet, so you'll have to build from source or just take my word for it).
I'll tackle foliage tomorrow probably. |
|
Sadly, I can't compile it and test it out right now. But I’ll mention that it’s very important to have a feature to quickly invert/uninvert the slope range. For example, if it was set to 0-45°, inverting it (maybe by pressing "slope" or using the current angle range) would switch it to 45-90°. This would make it much easier to paint as users can quickly switch between painting the steep slope, and the flat top surface, without having to fiddle with the slider every time. It would be even nicer to have the option to set different slope ranges for each material, so instead of needing two clicks (selecting the texture and inverting the slope), it could be done in one. |
|
I have reinstated the std:: namespace on isnan for linux. You should be able to build now. I have enabled the invert key CTRL to inverse the operation, so now if the range is 37-60 the inverted range will be "not 37-60" or 0-37 and 60-90. Pressing "Slope" resets the values to default (0-90/disabled) as it does for all non-checkbox options. |
TokisanGames
force-pushed
the
paint_slope
branch
from
f617038
to
ed1bb30
Compare
|
Instancer has been implemented. Thanks @az-raven. I instead used Alt to reverse the slope. Paint color + CTRL, Instancer + CTRL already removes, so conflicted. I need to document all of the keyboard operations, so drafted them below. This is almost ready to merge. The last thing is that the embedded editor picker icon doesn't work in compatibility mode. I might have to revert the file. General Keys
Sculpting Specific
Slope Operations SpecificThese operations support slope: Paint, Spray, Color, Wetness, Instancer.
Instancer Specific
|
TokisanGames
changed the title
… no popup for advanced
…-> ALT, remove all instances -> CTRL+SHIFT
TokisanGames
force-pushed
the
paint_slope
branch
from
ed1bb30
to
4ba3368
Compare
This one is finished. Thanks for the help guys. Any more bug fixes or improvements can go in another PR. Good job everyone. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Admin edit:
Fixes #282
It's useful when you need to paint slopes with different types of materials (because autoshader only supports 1 material).
exampleusage.webm