GPv3: Drawing Placement #115602

Falk David · 2023-11-30T11:49:18+01:00

Falk David commented

2023-11-30 11:49:18 +01:00

This PR adds the drawing placement modes from GPv2.

The drawing placement defines the depth (origin, view, surface, etc.) and a plane (view, cursor, xz, etc.).

This introduces a new helper class DrawingPlacement that does all of the internals to find the correct projection and just exposes a simple function to project from screen space to the drawing plane/surface.

Note: Drawing on other strokes currently doesn't work, because GPv3 can't be rendered to image yet. We use the depth buffer of the grease pencil render result to find the right depth.

This PR adds the drawing placement modes from GPv2. The drawing placement defines the depth (origin, view, surface, etc.) and a plane (view, cursor, xz, etc.). This introduces a new helper class `DrawingPlacement` that does all of the internals to find the correct projection and just exposes a simple function to project from screen space to the drawing plane/surface. Note: Drawing on other strokes currently doesn't work, because GPv3 can't be rendered to image yet. We use the depth buffer of the grease pencil render result to find the right depth. ![image](/attachments/20965794-6e81-4d93-9746-07d56cc35fa5) ![image](/attachments/bba959af-1814-43da-9596-d63c765b2857)

image.png

23 KiB

image.png

23 KiB

Falk David added 4 commits 2023-11-30 11:49:27 +01:00

a0a4223133 WIP: Drawing Placement

6e4276700e Merge branch 'main' into gpv3-drawing-placements

213f74704d Add gpencil_surface_offset` toolsetting

446fc01159 Progress

Falk David added 1 commit 2023-11-30 11:57:30 +01:00

6dfa50db0b Cleanup

Falk David requested review from Hans Goudey 2023-11-30 11:57:40 +01:00

Falk David requested review from Antonio Vazquez 2023-11-30 11:57:45 +01:00

Falk David changed title from ~~WIP: GPv3: Drawing Placement~~ to GPv3: Drawing Placement

2023-11-30 11:58:12 +01:00

Falk David added this to the Grease Pencil project 2023-11-30 12:01:13 +01:00

Falk David commented

2023-11-30 12:02:45 +01:00

Changes to be documented once this is merged:

Replaced the gpd.zdepth_offset property with a toolsetting: gpencil_surface_offset.

Changes to be documented once this is merged: Replaced the `gpd.zdepth_offset` property with a toolsetting: `gpencil_surface_offset`.

Hans Goudey reviewed 2023-12-01 21:49:45 +01:00

Hans Goudey left a comment

I didn't get the change to test this yet, but I guess there are others doing that anyway. My only point is about structuring this in a more data-oriented way then-- currently storing all the necessary data in a class and processing one point at a time doesn't seem to scale well. It's hard to judge whether the redundancy/complexity of storing the inputs in a class is worth it, but at least the API should still allow doing more work at a time.

source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc

						
				@ -30,0 +116,4 @@

				  ED_view3d_depth_override(depsgraph, region, view3d, nullptr, mode, &this->depth_cache_);

				}

				void DrawingPlacement::set_origin_to_nearest_stroke(const float2 co)

Hans Goudey commented

2023-12-01 21:47:16 +01:00

I'd structuring the API to process more than one element at a time. That's a principle of data oriented design-- there's rarely ever only one of something, and when there is, it's fast anyway because there's only one value. This would help to eliminate branching and decrease the overhead of "constant" operations per position.

Though we probably can't benefit from it much yet, since all the functions this uses process a single element at a time too.

I'd structuring the API to process more than one element at a time. That's a principle of data oriented design-- there's rarely ever only one of something, and when there is, it's fast anyway because there's only one value. This would help to eliminate branching and decrease the overhead of "constant" operations per position. Though we probably can't benefit from it much yet, since all the functions this uses process a single element at a time too.

Falk David commented

2023-12-04 11:59:07 +01:00

In the case of set_origin_to_nearest_stroke we're just changing the origin of the drawing plane. So this is a one-time operation.
For project I could add an API that would project multiple coordinates at a time.

In the case of `set_origin_to_nearest_stroke` we're just changing the `origin` of the drawing plane. So this is a one-time operation. For `project` I could add an API that would project multiple coordinates at a time.

👍 1

filedescriptor marked this conversation as resolved

Falk David added 2 commits 2023-12-04 15:04:43 +01:00

109a085d83 Merge branch 'main' into gpv3-drawing-placements

e0b5a03eee Project multiple coordinates at once

Hans Goudey reviewed 2023-12-04 15:12:09 +01:00

Hans Goudey left a comment

I'd probably just remove the 1 point at a time from the API to make it annoying to call, but this is fine too

source/blender/editors/grease_pencil/intern/grease_pencil_utils.cc Outdated

						
				@ -28,2 +31,4 @@

				namespace blender::ed::greasepencil {

				DrawingPlacement::DrawingPlacement(const Scene &scene,

				                                   const ARegion *region,

Hans Goudey commented

2023-12-04 15:12:07 +01:00

Looks like these aren't meant to be null ever, might as well make them references, even if you just store them as pointers again

filedescriptor marked this conversation as resolved

source/blender/editors/include/ED_grease_pencil.hh

						
				@ -54,0 +59,4 @@

				enum class DrawingPlacementPlane { View, Front, Side, Top, Cursor };

				class DrawingPlacement {

				 private:

Hans Goudey commented

2023-12-04 15:10:43 +01:00

private is redundant for classes

`private` is redundant for classes

filedescriptor marked this conversation as resolved

source/blender/editors/include/ED_grease_pencil.hh Outdated

						
				@ -54,0 +92,4 @@

				   * Projects a screen space coordinate to the local drawing space.

				   */

				  float3 project(const float2 co) const;

				  void project(const Span<float2> src, MutableSpan<float3> dst) const;

Hans Goudey commented

2023-12-04 15:11:02 +01:00

Remove const for by-value arguments in declarations

filedescriptor marked this conversation as resolved

Falk David added 1 commit 2023-12-04 15:36:48 +01:00

dc8668b08a Cleanup

Hans Goudey approved these changes 2023-12-04 15:59:35 +01:00

source/blender/editors/include/ED_grease_pencil.hh Outdated

						
				@ -54,0 +90,4 @@

				  /**

				   * Projects a screen space coordinate to the local drawing space.

				   */

				  float3 project(const float2 co) const;

Hans Goudey commented

2023-12-04 15:59:24 +01:00

Here too, and above!

👍 1

filedescriptor marked this conversation as resolved

Falk David added 1 commit 2023-12-04 16:42:11 +01:00

1c3b112de6 Remove const in declaration

Falk David merged commit 5fea1eda36 into main

2023-12-05 11:01:56 +01:00

Falk David referenced this issue from a commit

2023-12-05 11:01:56 +01:00

GPv3: Drawing Placements

Falk David deleted branch gpv3-drawing-placements

2023-12-05 11:01:58 +01:00

Falk David commented

2023-12-05 11:03:38 +01:00

Note: Some changes need to be documented (#114419):

Setting the drawing origin to "Surface" will grey-out the drawing plane (because it isn't used in this mode).
The offset from the surface is now a tool setting. It also respects world space and is not zoom-dependent.

Note: Some changes need to be documented (#114419): * Setting the drawing origin to "Surface" will grey-out the drawing plane (because it isn't used in this mode). * The offset from the surface is now a tool setting. It also respects world space and is not zoom-dependent.

Brian Savery (AMD) referenced this issue from a commit

2023-12-28 20:07:42 +01:00

GPv3: Drawing Placements

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

GPv3: Drawing Placement #115602