GPv3: Add edit mode lines to overlay #111596

casey-bianco-davis · 2023-08-28T04:51:42+02:00

casey-bianco-davis commented

2023-08-28 04:51:42 +02:00

Adds lines in edit mode to the overlay.

casey-bianco-davis added 1 commit 2023-08-28 04:51:50 +02:00

d59642a80f GPv3: Add edit mode lines to overlay

Adds lines in edit mode to the overlay.

casey-bianco-davis requested review from Falk David 2023-10-11 04:57:16 +02:00

casey-bianco-davis added 1 commit 2023-10-11 05:34:24 +02:00

e2f7977cd8 Merge branch 'main' into GPv3-edit-mode-lines

Falk David requested changes 2023-10-11 10:55:48 +02:00

Falk David left a comment

Thank you for tackling this! Appreciate it a lot :) I have two remarks:

Storing the edit line color in the object data is something that grease pencil 2 did indeed, but it's not something we should be doing moving forward. These colors should be theme colors. In this case, it should use the wire_edit color: UI_GetThemeColor4fv(TH_WIRE_EDIT, color); That should also remove the need for replaceColor in the shader.
The way you build the index buffer in grease_pencil_geom_batch_ensure works and is correct, but since you're building a different cache altogether it would be better to put that into its own static function. Then grease_pencil_geom_batch_ensure could have a const bool lines parameter and if that is true it calls the static function. Maybe something like grease_pencil_edit_lines_batch_ensure(...)

Thank you for tackling this! Appreciate it a lot :) I have two remarks: * Storing the edit line color in the object data is something that grease pencil 2 did indeed, but it's not something we should be doing moving forward. These colors should be theme colors. In this case, it should use the `wire_edit` color: `UI_GetThemeColor4fv(TH_WIRE_EDIT, color);` That should also remove the need for `replaceColor` in the shader. * The way you build the index buffer in `grease_pencil_geom_batch_ensure` works and is correct, but since you're building a different cache altogether it would be better to put that into its own `static` function. Then `grease_pencil_geom_batch_ensure` could have a `const bool lines` parameter and if that is true it calls the `static` function. Maybe something like `grease_pencil_edit_lines_batch_ensure(...)`

Falk David added this to the Grease Pencil project 2023-10-11 10:58:29 +02:00

casey-bianco-davis added 1 commit 2023-10-13 03:39:59 +02:00

73b80acf84 Cleanup: move line code to separator static function

casey-bianco-davis added 1 commit 2023-10-13 03:43:27 +02:00

a7c93dc214 Cleanup: undo moved code

casey-bianco-davis commented

2023-10-13 06:30:45 +02:00

On the line color. I agree it should in the theme and not per object and that replaceColor is not ideal. but wouldn't using TH_WIRE_EDIT make the lines the same color as mesh line? which is currently black and would not be visible on black grease pencil strokes, and because wire_edit is part of globalsBlock I don't know how to make it different for grease pencil vs mesh.

On the line color. I agree it should in the theme and not per object and that `replaceColor` is not ideal. but wouldn't using `TH_WIRE_EDIT` make the lines the same color as mesh line? which is currently black and would not be visible on black grease pencil strokes, and because `wire_edit` is part of `globalsBlock` I don't know how to make it different for grease pencil vs mesh.

casey-bianco-davis requested review from Falk David 2023-10-13 06:30:59 +02:00

Falk David commented

2023-10-13 10:57:50 +02:00

@casey-bianco-davis Basically that is my point. We shouldn't try to make grease pencil special in this way. All objects should use TH_WIRE_EDIT. And if the wire color doesn't work well for someone working with grease pencil, it's very rare that they would have to work both with meshes (edit mode) and grease pencil (edit mode) at the same time.

@casey-bianco-davis Basically that is my point. We shouldn't try to make grease pencil special in this way. All objects should use `TH_WIRE_EDIT`. And if the wire color doesn't work well for someone working with grease pencil, it's very rare that they would have to work both with meshes (edit mode) and grease pencil (edit mode) at the same time.

👍 1

Falk David commented

2023-10-13 10:58:52 +02:00

@pablovazquez @JulianEisel Would you agree to my statement above?

Pablo Vazquez commented

2023-10-13 12:12:19 +02:00

Would you agree to my statement above?

Yes. Editing Grease Pencil objects should look and feel just like regular Curves, Meshes, and other object types. If readability is a problem it should be tackled for all modes.

> Would you agree to my statement above? Yes. Editing Grease Pencil objects should look and feel just like regular Curves, Meshes, and other object types. If readability is a problem it should be tackled for all modes.

casey-bianco-davis added 1 commit 2023-10-14 01:44:36 +02:00

1f77879465 Remove custom line color

Falk David approved these changes 2023-10-16 13:59:12 +02:00

Falk David left a comment

Awesome! Thank you again!

Falk David referenced this issue from a commit

2023-10-16 13:59:36 +02:00

GPv3: Add edit mode lines to overlay

Falk David merged commit 0508e81b79 into main

2023-10-16 13:59:37 +02:00

casey-bianco-davis deleted branch GPv3-edit-mode-lines

2023-10-16 21:29:54 +02:00

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: Add edit mode lines to overlay #111596