From 57ca937a7cbec24c2ea644b14aafcaa91c8900e5 Mon Sep 17 00:00:00 2001 From: YimingWu Date: Tue, 8 Oct 2024 15:35:31 +0200 Subject: [PATCH] Fix #128714: GPv3: Don't erase behind the camera If points are beind the camera, we don't really want to erase them. This patch marks invalid coordinates thus preventing them from intersecting with a eraser. The reason for using a large value to indicate "invalid coordinate"s are: - No need to further break down the way we process `src_to_dist` point matching array for `hard_eraser` `soft_eraser`, makes the entire logic much easier. - No eraser is gonna be touching such a large coordinate of `1e20`. Technically there's this case where if a segment crosses the near or far clipping plane (to handle this correctly, you'll need to split that segment into two at the clipping plane position and it increases complexity a lot), and then you will have undefined erasing behaviour, however the worse case is that the one segment was completely removed, and in such case I think it's acceptable. Pull Request: https://projects.blender.org/blender/blender/pulls/128738 --- .../sculpt_paint/grease_pencil_erase.cc | 90 ++++++++++--------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/source/blender/editors/sculpt_paint/grease_pencil_erase.cc b/source/blender/editors/sculpt_paint/grease_pencil_erase.cc index 63b8038f1a4..32bed696bb6 100644 --- a/source/blender/editors/sculpt_paint/grease_pencil_erase.cc +++ b/source/blender/editors/sculpt_paint/grease_pencil_erase.cc @@ -867,52 +867,58 @@ struct EraseOperationExecutor { GreasePencil &grease_pencil = *static_cast(obact->data); bool changed = false; - const auto execute_eraser_on_drawing = [&](const int layer_index, - const int frame_number, - Drawing &drawing) { - const Layer &layer = grease_pencil.layer(layer_index); - const bke::CurvesGeometry &src = drawing.strokes(); + const auto execute_eraser_on_drawing = + [&](const int layer_index, const int frame_number, Drawing &drawing) { + const Layer &layer = grease_pencil.layer(layer_index); + const bke::CurvesGeometry &src = drawing.strokes(); - /* Evaluated geometry. */ - bke::crazyspace::GeometryDeformation deformation = - bke::crazyspace::get_evaluated_grease_pencil_drawing_deformation( - ob_eval, *obact, layer_index, frame_number); + /* Evaluated geometry. */ + bke::crazyspace::GeometryDeformation deformation = + bke::crazyspace::get_evaluated_grease_pencil_drawing_deformation( + ob_eval, *obact, layer_index, frame_number); - /* Compute screen space positions. */ - Array screen_space_positions(src.points_num()); - threading::parallel_for(src.points_range(), 4096, [&](const IndexRange src_points) { - for (const int src_point : src_points) { - ED_view3d_project_float_global(region, - math::transform_point(layer.to_world_space(*ob_eval), - deformation.positions[src_point]), - screen_space_positions[src_point], - V3D_PROJ_TEST_NOP); - } - }); + /* Compute screen space positions. */ + Array screen_space_positions(src.points_num()); + threading::parallel_for(src.points_range(), 4096, [&](const IndexRange src_points) { + for (const int src_point : src_points) { + const int result = ED_view3d_project_float_global( + region, + math::transform_point(layer.to_world_space(*ob_eval), + deformation.positions[src_point]), + screen_space_positions[src_point], + V3D_PROJ_TEST_CLIP_NEAR | V3D_PROJ_TEST_CLIP_FAR); + if (result != V3D_PROJ_RET_OK) { + /* Set the screen space position to a impossibly far coordinate for all the points + * that are outside near/far clipping planes, this is to prevent accidental + * intersections with strokes not visibly present in the camera. */ + screen_space_positions[src_point] = float2(1e20); + } + } + }); - /* Erasing operator. */ - bke::CurvesGeometry dst; - bool erased = false; - switch (self.eraser_mode_) { - case GP_BRUSH_ERASER_STROKE: - erased = stroke_eraser(src, screen_space_positions, dst); - break; - case GP_BRUSH_ERASER_HARD: - erased = hard_eraser(src, screen_space_positions, dst, self.keep_caps_); - break; - case GP_BRUSH_ERASER_SOFT: - erased = soft_eraser(src, screen_space_positions, dst, self.keep_caps_); - break; - } + /* Erasing operator. */ + bke::CurvesGeometry dst; + bool erased = false; + switch (self.eraser_mode_) { + case GP_BRUSH_ERASER_STROKE: + erased = stroke_eraser(src, screen_space_positions, dst); + break; + case GP_BRUSH_ERASER_HARD: + erased = hard_eraser(src, screen_space_positions, dst, self.keep_caps_); + break; + case GP_BRUSH_ERASER_SOFT: + erased = soft_eraser(src, screen_space_positions, dst, self.keep_caps_); + break; + } - if (erased) { - /* Set the new geometry. */ - drawing.geometry.wrap() = std::move(dst); - drawing.tag_topology_changed(); - changed = true; - self.affected_drawings_.add(&drawing); - } - }; + if (erased) { + /* Set the new geometry. */ + drawing.geometry.wrap() = std::move(dst); + drawing.tag_topology_changed(); + changed = true; + self.affected_drawings_.add(&drawing); + } + }; if (self.active_layer_only_) { /* Erase only on the drawing at the current frame of the active layer. */