GPv3: Brush radius unit option #120257

Falk David · 2024-04-04T15:43:32+02:00

Falk David commented

2024-04-04 15:43:32 +02:00

This implements and exposes the View/Scene brush radius option.

View: Brush units are in pixels. Zooming in and out in the viewport will make the brush stay the same size relative to the view, but grow/shrink relative to the scene.
Scene: Brush units are in meters. Zooming in and out in the viewport will make the brush grow/shrink relative to the view, but stay the same size relative to the scene.

The default radius unit is Scene, which is what GPv2 did.

The "2D Animation Template" was updated to disable using the radius in the unified paint settings. This means that using the template by default will use the brush radius, which also mimics the behavior of GPv2.

The user can change the radius brush unit from the Advanced panel.

This implements and exposes the `View`/`Scene` brush radius option. * `View`: Brush units are in pixels. Zooming in and out in the viewport will make the brush stay the same size relative to the view, but grow/shrink relative to the scene. * `Scene`: Brush units are in meters. Zooming in and out in the viewport will make the brush grow/shrink relative to the view, but stay the same size relative to the scene. The default radius unit is `Scene`, which is what GPv2 did. The "2D Animation Template" was updated to disable using the radius in the unified paint settings. This means that using the template by default will use the brush radius, which also mimics the behavior of GPv2. The user can change the radius brush unit from the `Advanced` panel. ![image](/attachments/e024c1c4-5b21-4159-b57e-68419d0d216b)

image.png

91 KiB

Falk David added 10 commits 2024-04-04 15:43:41 +02:00

8eccfdd8d3 WIP

b655094af9 Merge branch 'main' into gpv3-scene-brush-size

7be7556e49 WIP

720e683ae1 Merge branch 'main' into gpv3-scene-brush-size

d7848990c6 revert some changes

bc3b1cbd52 Merge branch 'main' into gpv3-scene-brush-size

2fe2cf06aa Use scene size brush in `brush_basic_grease_pencil_paint_settings`

1500de33af Merge branch 'main' into gpv3-scene-brush-size

36e4451902 Add UI for brush radius units

29ae26a24b Merge branch 'main' into gpv3-scene-brush-size

Falk David added 1 commit 2024-04-08 14:51:03 +02:00

c377d1b76d Merge branch 'main' into gpv3-scene-brush-size

Falk David added 2 commits 2024-04-09 12:49:41 +02:00

d5037db76d Merge branch 'main' into gpv3-scene-brush-size

7a8920c3f5 Cleanups

Falk David added 4 commits 2024-04-30 16:22:24 +02:00

6debd38aca Merge branch 'main' into gpv3-scene-brush-size

f9df4e585c Update 2D animation startup file

This makes sure to turn off the unified paint settings for the
brush size in the default `Scene`.

When loading the template, the draw tool uses the size
defined by the brush.
This is needed for GPv3, because we make use of the unified
paint settings, but don't want to use them when loading
the template.

2bccc0da4a Version the unprojected radius for grease pencil brushes

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

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

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

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

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

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

116ddf21f9 Merge branch 'main' into gpv3-scene-brush-size

Falk David changed title from ~~WIP: GPv3: Brush radius unit option~~ to GPv3: Brush radius unit option

2024-04-30 16:32:32 +02:00

Falk David added 2 commits 2024-04-30 16:43:05 +02:00

63bc5ff268 Remove unused code

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

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

buildbot/vexp-code-patch-darwin-x86_64 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

92b3765444 Move brush conversion to `greasepencil::convert::legacy_main`

Falk David commented

2024-04-30 16:43:28 +02:00

@blender-bot package

Blender Bot commented

2024-04-30 16:43:31 +02:00

Package build started. Download here when ready.

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

Falk David added this to the Grease Pencil project 2024-04-30 17:28:24 +02:00

Falk David requested review from Bastien Montagne 2024-04-30 17:58:13 +02:00

Matias Mendiola commented

2024-05-02 23:46:47 +02:00

Everything worked very well in my tests, just two point to think about:

When using scene units, having the radius size in meters (for example, 0.01 m) is not so user friendly for 2D artist. Maybe overrides the system units with millimeters or inches depending on the unit system could be a solution?
Having the option to unify the Radius for all brushes was always a pending task. is now the right time to implement it?

Everything worked very well in my tests, just two point to think about: 1. When using scene units, having the radius size in meters (for example, 0.01 m) is not so user friendly for 2D artist. Maybe overrides the system units with millimeters or inches depending on the unit system could be a solution? 2. Having the option to unify the Radius for all brushes was always a pending task. is now the right time to implement it? ![image](/attachments/36c9e81c-5b70-44a0-a06a-85cac279f60f)

image.png

3.0 KiB

👍 1

Falk David commented

2024-05-03 10:23:30 +02:00

@mendio Thanks for testing!

I worked on this a bit, but it's tricky to get right. What I did is allow a prop to use the "adaptive" unit display. This is normally a setting for the whole scene, but in this case it's useful for just that prop. That aproach was not accepted though. We'd need more input from the UI module.
This already works. It's just that the button is missing in the topbar. You can seit it in the tool settings in the side pane.

@mendio Thanks for testing! 1. I worked on this a bit, but it's tricky to get right. What I did is allow a prop to use the "adaptive" unit display. This is normally a setting for the whole scene, but in this case it's useful for just that prop. That aproach was not accepted though. We'd need more input from the UI module. 2. This already works. It's just that the button is missing in the topbar. You can seit it in the tool settings in the side pane.

Bastien Montagne requested changes 2024-05-03 11:44:40 +02:00

Dismissed

Bastien Montagne left a comment

Only checked the versioning part for now.

Besides moving it to regular versioning, think you also need apply the same treatment to GP brushes in BLO_update_defaults_startup_blend.

And both of these versioning calls should be disabled until to full switch to GPv3 is done, since I do not see how to prevent invalid behaviors otherwise...

Only checked the versioning part for now. Besides moving it to regular versioning, think you also need apply the same treatment to GP brushes in `BLO_update_defaults_startup_blend`. And both of these versioning calls should be disabled until to full switch to GPv3 is done, since I do not see how to prevent invalid behaviors otherwise...

source/blender/blenkernel/intern/grease_pencil_convert_legacy.cc Outdated

						
				@ -2961,6 +2961,20 @@ void legacy_gpencil_object(Main &bmain, Object &object)

				  legacy_gpencil_object_ex(conversion_data, object);

				}

				static void legacy_gpencil_brushes(Main &bmain)

Bastien Montagne commented

2024-05-03 11:41:22 +02:00

This should be part of the regular versioning code, it cannot be called here.

The main reason being, there is no way to prevent re-applying the conversion multiple time on the same data, since this code is executed outside of any fileversion control.

Further more, such basic conversion should be kept out of the do_versions_after_setup area - the less processing we have here, the better.

This should be part of the regular versioning code, it cannot be called here. The main reason being, there is no way to prevent re-applying the conversion multiple time on the same data, since this code is executed outside of any fileversion control. Further more, such basic conversion should be kept out of the `do_versions_after_setup` area - the less processing we have here, the better.

filedescriptor marked this conversation as resolved

Falk David commented

2024-05-03 11:58:39 +02:00

After an in-person discussion with @mont29 about the versioning:

Since GPv2 doesn't use the affected brush settings at all (to be double-checked), the versioning should be done as part of the regular versioning code with a subversion bump. This can be done as part of the PR then and not have to wait.
Instead of changing the 2D Animation template binary blend file, this should be handled by BLO_update_defaults_startup_blend instead (also to be double checked).

After an in-person discussion with @mont29 about the versioning: * Since GPv2 doesn't use the affected brush settings at all (to be double-checked), the versioning should be done as part of the regular versioning code with a subversion bump. This can be done as part of the PR then and not have to wait. * Instead of changing the 2D Animation template binary blend file, this should be handled by `BLO_update_defaults_startup_blend` instead (also to be double checked).

👍 1

Matias Mendiola commented

2024-05-03 15:22:32 +02:00

This already works. It's just that the button is missing in the topbar. You can seit it in the tool settings in the side pane.

haha...Great! just testing that and noticed that when turned on the unit system used change and also the default values for Radius and Strength

Scene radius unit should be the default for Grease Pencil brushes with 0.03 m radius, to get a line similar to the stroke primitive when you first draw. (Whether you use the 2D animation template or not)

> 2. This already works. It's just that the button is missing in the topbar. You can seit it in the tool settings in the side pane. haha...Great! just testing that and noticed that when turned on the unit system used change and also the default values for Radius and Strength ![image](/attachments/577bdb59-b45a-4ac1-8a05-ecb5e5e129e1) Scene radius unit should be the default for Grease Pencil brushes with 0.03 m radius, to get a line similar to the stroke primitive when you first draw. (Whether you use the 2D animation template or not) ![image](/attachments/3b9615b9-dbac-4061-8f7e-3b313a6442e1)

image.png

56 KiB

image.png

9.9 KiB

Falk David added 5 commits 2024-05-06 13:48:18 +02:00

895164af03 Merge branch 'main' into gpv3-scene-brush-size

dc130aadcc Merge branch 'main' into gpv3-scene-brush-size

c27cd40877 Revert changes to startup file

6a310249d4 Add versioning to the 2D_Animation template

af704457a1 Add versioning for the grease pencil brushes

Falk David added 2 commits 2024-05-07 13:32:33 +02:00

cc5d4b5fae Merge branch 'main' into gpv3-scene-brush-size

01a972bdce Use correct settings in presets

Falk David commented

2024-05-07 13:35:55 +02:00

@mendio The presets now use the correct settings. But I can't change the default for the unified paint settings, so if you don't use the template, then the scene will use unified paint settings and the brush will be in view space.

Maybe what we need to do is also show the unified paint settings toggle in the top bar so that you can easily turn that off without needing to open the side panel.

@mendio The presets now use the correct settings. But I can't change the default for the unified paint settings, so if you don't use the template, then the scene will use unified paint settings and the brush will be in view space. Maybe what we need to do is also show the unified paint settings toggle in the top bar so that you can easily turn that off without needing to open the side panel.

Falk David requested review from Bastien Montagne 2024-05-07 13:36:04 +02:00

Falk David added 1 commit 2024-05-07 13:38:07 +02:00

58c426b2c7 Remove conversion from `convert::greasepencil::legacy_main()`

Bastien Montagne approved these changes 2024-05-14 13:08:07 +02:00

Bastien Montagne left a comment

Versioning side of the PR looks good now, I believe the usability side of it has already been validated by the module, so LGTM.

Falk David added 1 commit 2024-05-14 14:49:23 +02:00