From c20399442d5b0b17c8daf7f4cb61fd75f57f4375 Mon Sep 17 00:00:00 2001 From: Falk David Date: Fri, 20 Sep 2024 16:52:41 +0200 Subject: [PATCH] Fix #127906: Issues with reproject operator There were some issues with the operator: * The `keep_original` option could easily crash blender. * The surface projection did not work correctly. This commits fixes those issues. The `keep_original` was missing a tag for the topology that changes in the drawing. The surface projection needed to cache the viewport depths. This is done once and then used to initialize the `DrawingPlacement`. --- .../intern/grease_pencil_edit.cc | 28 ++++++++++--------- .../intern/grease_pencil_utils.cc | 19 +++++++------ .../editors/include/ED_grease_pencil.hh | 3 +- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc index 844cf3f1d3e..4a5d7521925 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_edit.cc @@ -2713,13 +2713,15 @@ static int grease_pencil_reproject_exec(bContext *C, wmOperator *op) GreasePencil &grease_pencil = *static_cast(object->data); const float offset = RNA_float_get(op->ptr, "offset"); + ViewDepths *view_depths = nullptr; + if (mode == ReprojectMode::Surface) { + ED_view3d_depth_override(depsgraph, region, v3d, nullptr, V3D_DEPTH_NO_GPENCIL, &view_depths); + } + const bke::AttrDomain selection_domain = ED_grease_pencil_selection_domain_get( scene.toolsettings); const int oldframe = int(DEG_get_ctime(depsgraph)); - /* TODO: This can probably be optimized further for the non-Surface projection use case by - * considering all drawings for the parallel loop instead of having to partition by frame number. - */ if (keep_original) { const Vector drawings = retrieve_editable_drawings(scene, grease_pencil); threading::parallel_for_each(drawings, [&](const MutableDrawingInfo &info) { @@ -2737,9 +2739,13 @@ static int grease_pencil_reproject_exec(bContext *C, wmOperator *op) else if (selection_domain == bke::AttrDomain::Point) { curves::duplicate_points(curves, elements); } + info.drawing.tag_topology_changed(); }); } + /* TODO: This can probably be optimized further for the non-Surface projection use case by + * considering all drawings for the parallel loop instead of having to partition by frame number. + */ std::atomic changed = false; Array> drawings_per_frame = retrieve_editable_drawings_grouped_per_frame(scene, grease_pencil); @@ -2765,17 +2771,13 @@ static int grease_pencil_reproject_exec(bContext *C, wmOperator *op) const bke::greasepencil::Layer &layer = *grease_pencil.layer(info.layer_index); bke::CurvesGeometry &curves = info.drawing.strokes_for_write(); const DrawingPlacement drawing_placement( - scene, *region, *v3d, *object, &layer, mode, offset); - const OffsetIndices points_by_curve = curves.points_by_curve(); + scene, *region, *v3d, *object, &layer, mode, offset, view_depths); + MutableSpan positions = curves.positions_for_write(); - for (const int curve_index : curves.curves_range()) { - const IndexRange curve_points = points_by_curve[curve_index]; - const IndexMask curve_points_to_reproject = points_to_reproject.slice_content( - curve_points); - curve_points_to_reproject.foreach_index(GrainSize(4096), [&](const int point_i) { - positions[point_i] = drawing_placement.reproject(positions[point_i]); - }); - } + points_to_reproject.foreach_index(GrainSize(4096), [&](const int point_i) { + positions[point_i] = drawing_placement.reproject(positions[point_i]); + }); + info.drawing.tag_positions_changed(); changed.store(true, std::memory_order_relaxed); }); diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc index e7a5db40c8d..ca4e60514f7 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc @@ -120,8 +120,12 @@ DrawingPlacement::DrawingPlacement(const Scene &scene, const Object &eval_object, const bke::greasepencil::Layer *layer, const ReprojectMode reproject_mode, - const float surface_offset) - : region_(®ion), view3d_(&view3d), surface_offset_(surface_offset) + const float surface_offset, + ViewDepths *view_depths) + : region_(®ion), + view3d_(&view3d), + surface_offset_(surface_offset), + depth_cache_(view_depths) { layer_space_to_world_space_ = (layer != nullptr) ? layer->to_world_space(eval_object) : eval_object.object_to_world(); @@ -347,15 +351,12 @@ void DrawingPlacement::project(const Span src, MutableSpan dst) float3 DrawingPlacement::reproject(const float3 pos) const { + const float3 world_pos = math::transform_point(layer_space_to_world_space_, pos); float3 proj_point; if (depth_ == DrawingPlacementDepth::Surface) { /* First project the position into view space. */ float2 co; - if (ED_view3d_project_float_global(region_, - math::transform_point(layer_space_to_world_space_, pos), - co, - V3D_PROJ_TEST_NOP) != V3D_PROJ_RET_OK) - { + if (ED_view3d_project_float_global(region_, world_pos, co, V3D_PROJ_TEST_NOP)) { /* Can't reproject the point. */ return pos; } @@ -369,10 +370,10 @@ float3 DrawingPlacement::reproject(const float3 pos) const float3 ray_co, ray_no; if (rv3d->is_persp) { ray_co = float3(rv3d->viewinv[3]); - ray_no = math::normalize(ray_co - math::transform_point(layer_space_to_world_space_, pos)); + ray_no = math::normalize(ray_co - world_pos); } else { - ray_co = math::transform_point(layer_space_to_world_space_, pos); + ray_co = world_pos; ray_no = -float3(rv3d->viewinv[2]); } float4 plane; diff --git a/source/blender/editors/include/ED_grease_pencil.hh b/source/blender/editors/include/ED_grease_pencil.hh index 35cd3cae3ec..115c5b25539 100644 --- a/source/blender/editors/include/ED_grease_pencil.hh +++ b/source/blender/editors/include/ED_grease_pencil.hh @@ -131,7 +131,8 @@ class DrawingPlacement { const Object &eval_object, const bke::greasepencil::Layer *layer, ReprojectMode reproject_mode, - float surface_offset = 0.0f); + float surface_offset = 0.0f, + ViewDepths *view_depths = nullptr); DrawingPlacement(const DrawingPlacement &other); DrawingPlacement(DrawingPlacement &&other); DrawingPlacement &operator=(const DrawingPlacement &other);