Sculpt: Add Line Trim tool #120845

Sean Kim · 2024-04-20T00:43:30+02:00

Sean Kim commented

2024-04-20 00:43:30 +02:00

This PR adds the Line Trim tool to Sculpt mode. It is exposed via the toolbar along with the other Trim tools and as an entry in the Sculpt menu.

Added Features

Line Trim tool

Technical Approach

Line gestures are represented as two points in screen space: the start and end of the gesture. Trim tools work by taking a set of screen points as the top face of a volume, projecting a copy of the face into the scene and then using that as the operand shape for boolean operations. To behave as users would expect, the Line Trim tool does the following steps to make the initial face:

Take the sculpted object's bounding box.
Project the bounding box into screen space
Take the magnitude of the diagonal line made from the bounding box
Use the magnitude multiplied by an arbitrary factor to ensure the initial line is long enough to fully divide the object.
Create two points by moving in a perpendicular direction from start and end points.
Use the resulting four points as the vertices of the quad in screen space.

Differences with Other Trim Tools

Line Trim only supports the Difference mode. As such, the corresponding tool options have been disabled in the header.

Alternatives

Instead of using a boolean operation, this could be achieved by using a bisect operation when using the Fixed projection mode. While this may result in a better performing tool, it is not guaranteed and requires extra work to integrate this approach.

This PR adds the *Line Trim* tool to Sculpt mode. It is exposed via the toolbar along with the other *Trim* tools and as an entry in the *Sculpt* menu. ![line_trim.gif](/attachments/11b5c928-fd33-4a3f-ae62-586795ce61b3) ## Added Features * *Line Trim* tool ## Technical Approach Line gestures are represented as two points in screen space: the `start` and `end` of the gesture. Trim tools work by taking a set of screen points as the top face of a volume, projecting a copy of the face into the scene and then using that as the operand shape for boolean operations. To behave as users would expect, the *Line Trim* tool does the following steps to make the initial face: 1. Take the sculpted object's bounding box. 2. Project the bounding box into screen space 3. Take the magnitude of the diagonal line made from the bounding box 4. Use the magnitude multiplied by an arbitrary factor to ensure the initial line is long enough to fully divide the object. 5. Create two points by moving in a perpendicular direction from start and end points. 6. Use the resulting four points as the vertices of the quad in screen space. ## Differences with Other Trim Tools * Line Trim **only** supports the **Difference** mode. As such, the corresponding tool options have been disabled in the header. ## Alternatives * Instead of using a boolean operation, this could be achieved by using a bisect operation when using the *Fixed* projection mode. While this may result in a better performing tool, it is not guaranteed and requires extra work to integrate this approach.

line_trim.gif

2.8 MiB

❤️ 2

Sean Kim added 1 commit 2024-04-20 00:43:41 +02:00

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

c84f4051ab Sculpt: Add Line Trim tool

Sean Kim requested review from Hans Goudey 2024-04-20 00:43:55 +02:00

Sean Kim requested review from Daniel Bystedt 2024-04-20 00:44:04 +02:00

Hans Goudey commented

2024-04-20 00:55:34 +02:00

@blender-bot package

Blender Bot commented

2024-04-20 00:55:38 +02:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR120845) when ready.

Sean Kim referenced this pull request

2024-04-20 01:09:19 +02:00

Sculpt Mode Gesture Tools #80390

Daniel Bystedt commented

2024-04-30 16:41:40 +02:00

I am not getting expected behaviour from this build. Using the line trim tool in sculpt mode gives me a cut line on parts of the mesh. Check the attached video

Strange cut.mp4

4.3 MiB

Sean Kim commented

2024-04-30 16:47:57 +02:00

I am not getting expected behaviour from this build. Using the line trim tool in sculpt mode gives me a cut line on parts of the mesh. Check the attached video

Very strange behavior... I'll take a second look at this, I didn't encounter this at all in my testing.

> I am not getting expected behaviour from this build. Using the line trim tool in sculpt mode gives me a cut line on parts of the mesh. Check the attached video Very strange behavior... I'll take a second look at this, I didn't encounter this at all in my testing.

Sean Kim added 1 commit 2024-04-30 18:41:14 +02:00

d5fb28cc79 Fix span referencing stack data going out of scope

Sean Kim commented

2024-04-30 18:42:24 +02:00

Ok, I've fixed the issue, I think I was invoking undefined behavior that just happened to be working on my laptop.

Sean Kim reviewed 2024-04-30 18:46:50 +02:00

source/blender/editors/sculpt_paint/sculpt_trim.cc Outdated

						
				@ -281,3 +331,3 @@

				  ARegion *region = vc->region;

				  const Span<float2> screen_points = gesture_data.gesture_points;

				  std::array<float2, 4> line_points;

Sean Kim commented

2024-04-30 18:46:50 +02:00

This handling of the line_points as looks rather awkward to me as it currently is, but I wasn't able to think of a good way of handling this. Another alternative here could be to modify the internal GestureData blender::Array in the above line_gesture_to_screen_points but that also had a code smell. Any thoughts on a good approach here?

This handling of the `line_points` as looks rather awkward to me as it currently is, but I wasn't able to think of a good way of handling this. Another alternative here could be to modify the internal `GestureData` `blender::Array` in the above `line_gesture_to_screen_points` but that also had a code smell. Any thoughts on a good approach here?

Hans Goudey commented

2024-04-30 20:09:44 +02:00

I think this is fine. But another potentially simpler solution would be changing const Span<float2> screen_points to Array<float2>. The copying shouldn't really matter here.

I think this is fine. But another potentially simpler solution would be changing `const Span<float2> screen_points` to `Array<float2>`. The copying shouldn't really matter here.

Sean-Kim marked this conversation as resolved

Sean Kim added 1 commit 2024-04-30 22:06:17 +02:00