From a40a4ccb2ff64fac2e4e64392dc7d4a6b0946993 Mon Sep 17 00:00:00 2001 From: Falk David Date: Wed, 5 Jun 2024 15:06:49 +0200 Subject: [PATCH] GPv3: Draw Tool: Make "Simplify" option a screen space threshold Previously, the "Simplify" option was a world space distance threshold. This meant that zooming in and out of the view changed the way this option behaved. There were complaints from artists about this. This change improves two things: * The simplify algorithm uses the screen space coordinates rather than the 3D positions. * The UI setting is in pixels making it much easier to tweak (no need for values in the 1e-4 range). Pull Request: https://projects.blender.org/blender/blender/pulls/122719 --- scripts/startup/bl_ui/space_view3d_toolbar.py | 2 +- source/blender/blenkernel/intern/brush.cc | 21 +++++++ .../sculpt_paint/grease_pencil_paint.cc | 59 +++++++++---------- .../blender/geometry/GEO_simplify_curves.hh | 9 +++ .../geometry/intern/simplify_curves.cc | 17 ++++++ source/blender/makesdna/DNA_brush_types.h | 3 +- source/blender/makesrna/intern/rna_brush.cc | 11 ++++ 7 files changed, 90 insertions(+), 32 deletions(-) diff --git a/scripts/startup/bl_ui/space_view3d_toolbar.py b/scripts/startup/bl_ui/space_view3d_toolbar.py index 72a464833a5..f8276d363b2 100644 --- a/scripts/startup/bl_ui/space_view3d_toolbar.py +++ b/scripts/startup/bl_ui/space_view3d_toolbar.py @@ -2735,7 +2735,7 @@ class VIEW3D_PT_tools_grease_pencil_v3_brush_post_processing(View3DPanel, Panel) col1.prop(gp_settings, "pen_subdivision_steps", text="Subdivisions") col1 = col.column(align=True) - col1.prop(gp_settings, "simplify_factor") + col1.prop(gp_settings, "simplify_pixel_threshold", slider=True) col1 = col.column(align=True) col1.prop(gp_settings, "use_trim") diff --git a/source/blender/blenkernel/intern/brush.cc b/source/blender/blenkernel/intern/brush.cc index 8ea7c56cc0a..ac0299a41d9 100644 --- a/source/blender/blenkernel/intern/brush.cc +++ b/source/blender/blenkernel/intern/brush.cc @@ -799,6 +799,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.002f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.4f; + } brush->gpencil_settings->draw_random_press = 0.0f; brush->gpencil_settings->draw_jitter = 0.0f; @@ -844,6 +847,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 2; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.000f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.0f; + } brush->gpencil_settings->flag |= GP_BRUSH_GROUP_RANDOM; brush->gpencil_settings->draw_random_press = 0.6f; @@ -891,6 +897,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.002f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.4f; + } brush->gpencil_settings->flag &= ~GP_BRUSH_GROUP_RANDOM; brush->gpencil_settings->draw_random_press = 0.0f; @@ -938,6 +947,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.002f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.4f; + } brush->gpencil_settings->flag &= ~GP_BRUSH_GROUP_RANDOM; brush->gpencil_settings->draw_random_press = 0.0f; @@ -990,6 +1002,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 1; brush->gpencil_settings->simplify_f = 0.002f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.4f; + } brush->gpencil_settings->draw_random_press = 0.0f; brush->gpencil_settings->draw_random_strength = 0.0f; @@ -1031,6 +1046,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.000f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.0f; + } brush->gpencil_settings->draw_random_press = 0.0f; brush->gpencil_settings->draw_random_strength = 0.0f; @@ -1075,6 +1093,9 @@ void BKE_gpencil_brush_preset_set(Main *bmain, Brush *brush, const short type) brush->gpencil_settings->draw_smoothlvl = 1; brush->gpencil_settings->draw_subdivide = 0; brush->gpencil_settings->simplify_f = 0.002f; + if (U.experimental.use_grease_pencil_version3) { + brush->gpencil_settings->simplify_px = 0.4f; + } brush->gpencil_settings->draw_random_press = 0.0f; brush->gpencil_settings->draw_jitter = 0.0f; diff --git a/source/blender/editors/sculpt_paint/grease_pencil_paint.cc b/source/blender/editors/sculpt_paint/grease_pencil_paint.cc index 1f747f8de46..fddbedb4509 100644 --- a/source/blender/editors/sculpt_paint/grease_pencil_paint.cc +++ b/source/blender/editors/sculpt_paint/grease_pencil_paint.cc @@ -795,43 +795,37 @@ static void smooth_stroke(bke::greasepencil::Drawing &drawing, } static void simplify_stroke(bke::greasepencil::Drawing &drawing, + Span screen_space_positions, const float epsilon, const int active_curve) { const bke::CurvesGeometry &curves = drawing.strokes(); + const IndexRange points = curves.points_by_curve()[active_curve]; + BLI_assert(screen_space_positions.size() == points.size()); - IndexMaskMemory memory; - IndexMask points_to_delete = geometry::simplify_curve_attribute( - curves.positions(), - IndexRange::from_single(active_curve), - curves.points_by_curve(), - curves.cyclic(), - epsilon, - curves.positions(), - memory); - - const VArray radii = drawing.radii(); - if (!radii.is_empty() && radii.is_span()) { - const IndexMask radii_mask = geometry::simplify_curve_attribute( - curves.positions(), - IndexRange::from_single(active_curve), - curves.points_by_curve(), - curves.cyclic(), - epsilon, - radii.get_internal_span(), - memory); - points_to_delete = IndexMask::from_intersection(points_to_delete, radii_mask, memory); + if (epsilon <= 0.0f) { + return; } + Array points_to_delete_arr(drawing.strokes().points_num(), false); + points_to_delete_arr.as_mutable_span().slice(points).fill(true); + geometry::curve_simplify(curves.positions().slice(points), + curves.cyclic()[active_curve], + epsilon, + screen_space_positions, + points_to_delete_arr.as_mutable_span().slice(points)); + + IndexMaskMemory memory; + const IndexMask points_to_delete = IndexMask::from_bools(points_to_delete_arr, memory); if (!points_to_delete.is_empty()) { drawing.strokes_for_write().remove_points(points_to_delete, {}); } } -static void trim_end_points(bke::greasepencil::Drawing &drawing, - const float epsilon, - const bool on_back, - const int active_curve) +static int trim_end_points(bke::greasepencil::Drawing &drawing, + const float epsilon, + const bool on_back, + const int active_curve) { const IndexRange points = drawing.strokes().points_by_curve()[active_curve]; bke::CurvesGeometry &curves = drawing.strokes_for_write(); @@ -849,13 +843,13 @@ static void trim_end_points(bke::greasepencil::Drawing &drawing, } if (num_points_to_remove <= 0) { - return; + return 0; } if (!on_back) { curves.resize(curves.points_num() - num_points_to_remove, curves.curves_num()); curves.offsets_for_write().last() = curves.points_num(); - return; + return num_points_to_remove; } bke::MutableAttributeAccessor attributes = curves.attributes_for_write(); @@ -891,6 +885,8 @@ static void trim_end_points(bke::greasepencil::Drawing &drawing, offsets[src_curve] = offsets[src_curve] - num_points_to_remove; } offsets.last() = curves.points_num(); + + return num_points_to_remove; } static void deselect_stroke(const bContext &C, @@ -939,15 +935,18 @@ void PaintOperation::on_stroke_done(const bContext &C) const int active_curve = on_back ? drawing.strokes().curves_range().first() : drawing.strokes().curves_range().last(); /* Remove trailing points with radii close to zero. */ - trim_end_points(drawing, 1e-5f, on_back, active_curve); + const int num_points_removed = trim_end_points(drawing, 1e-5f, on_back, active_curve); /* Set the selection of the newly drawn stroke to false. */ deselect_stroke(C, drawing, active_curve); if (do_post_processing) { if (settings->draw_smoothfac > 0.0f) { smooth_stroke(drawing, settings->draw_smoothfac, settings->draw_smoothlvl, active_curve); } - if (settings->simplify_f > 0.0f) { - simplify_stroke(drawing, settings->simplify_f, active_curve); + if (settings->simplify_px > 0.0f) { + simplify_stroke(drawing, + this->screen_space_smoothed_coords_.as_span().drop_back(num_points_removed), + settings->simplify_px, + active_curve); } } drawing.set_texture_matrices({texture_space_}, IndexRange::from_single(active_curve)); diff --git a/source/blender/geometry/GEO_simplify_curves.hh b/source/blender/geometry/GEO_simplify_curves.hh index 649770708c7..c81651267e4 100644 --- a/source/blender/geometry/GEO_simplify_curves.hh +++ b/source/blender/geometry/GEO_simplify_curves.hh @@ -20,4 +20,13 @@ IndexMask simplify_curve_attribute(const Span positions, GSpan attribute_data, IndexMaskMemory &memory); +/** + * Same as above, but only for a single curve. All spans are expected to be the size of the curve. + */ +void curve_simplify(const Span positions, + const bool cyclic, + const float epsilon, + const GSpan attribute_data, + MutableSpan points_to_delete); + } // namespace blender::geometry diff --git a/source/blender/geometry/intern/simplify_curves.cc b/source/blender/geometry/intern/simplify_curves.cc index 9aa161ddb66..866c4312388 100644 --- a/source/blender/geometry/intern/simplify_curves.cc +++ b/source/blender/geometry/intern/simplify_curves.cc @@ -117,6 +117,23 @@ static void curve_simplify(const Span positions, } } +void curve_simplify(const Span positions, + const bool cyclic, + const float epsilon, + const GSpan attribute_data, + MutableSpan points_to_delete) + +{ + bke::attribute_math::convert_to_static_type(attribute_data.type(), [&](auto dummy) { + using T = decltype(dummy); + if constexpr (std::is_same_v || std::is_same_v || + std::is_same_v) + { + curve_simplify(positions, cyclic, epsilon, attribute_data.typed(), points_to_delete); + } + }); +} + IndexMask simplify_curve_attribute(const Span positions, const IndexMask &curves_selection, const OffsetIndices points_by_curve, diff --git a/source/blender/makesdna/DNA_brush_types.h b/source/blender/makesdna/DNA_brush_types.h index 9d7f630ee0c..2413a0b9e57 100644 --- a/source/blender/makesdna/DNA_brush_types.h +++ b/source/blender/makesdna/DNA_brush_types.h @@ -133,7 +133,8 @@ typedef struct BrushGpencilSettings { /** Factor for external line thickness conversion to outline. */ float outline_fac; - char _pad1[4]; + /** Screen space simplify threshold. Points within this margin are treated as a straight line. */ + float simplify_px; /* optional link of material to replace default in context */ /** Material. */ diff --git a/source/blender/makesrna/intern/rna_brush.cc b/source/blender/makesrna/intern/rna_brush.cc index 9ee827f9bf7..406db1bd6e5 100644 --- a/source/blender/makesrna/intern/rna_brush.cc +++ b/source/blender/makesrna/intern/rna_brush.cc @@ -1576,6 +1576,17 @@ static void rna_def_gpencil_options(BlenderRNA *brna) RNA_def_property_ui_text(prop, "Simplify", "Factor of Simplify using adaptive algorithm"); RNA_def_parameter_clear_flags(prop, PROP_ANIMATABLE, ParameterFlag(0)); + prop = RNA_def_property(srna, "simplify_pixel_threshold", PROP_FLOAT, PROP_PIXEL); + RNA_def_property_float_sdna(prop, nullptr, "simplify_px"); + RNA_def_property_range(prop, 0, 10.0); + RNA_def_property_ui_range(prop, 0, 10.0, 1.0f, 1); + RNA_def_property_ui_text( + prop, + "Simplify", + "Threashold in screen space used for the simplify algorithm. Points within this threashold " + "are treated as if they were in a straight line"); + RNA_def_parameter_clear_flags(prop, PROP_ANIMATABLE, ParameterFlag(0)); + /* Curves for pressure */ prop = RNA_def_property(srna, "curve_sensitivity", PROP_POINTER, PROP_NONE); RNA_def_property_pointer_sdna(prop, nullptr, "curve_sensitivity");