Page MenuHome

Corner styles for annotation line tool
ClosedPublic

Authored by Juanfran Matheu (jfmatheu) on May 3 2020, 1:54 PM.

Details

Summary

Hi, this patch adds different kind of shapes/styles for the stroke corners while using the annotation line tool.

[UPDATED]

Current Styles: (following @Matias Mendiola (mendio) mockup)

  • Arrow (closed arrow)
  • Open Arrow
  • Segment
  • Square

Removed:

  • Inverted
  • Diamond
  • Triangle

[UPDATED VIDEO]

For future it would be great to have icons, it would be more intuitive (and less space) with previews of what each end / start of line does, like the google slides one as reference:

Regards,
JF

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Juanfran Matheu (jfmatheu) requested review of this revision.May 3 2020, 1:54 PM

Full refactor of the code...

  • Review all float constants.
  • Review if you can use constfor variables that don't change.
source/blender/editors/gpencil/annotate_paint.c
428

3.0f & 5.0f

431

2.0f & -2.0f

433

1.0f

490

This line must be removed or the comment style must be /* */

494

too much (( ))

(tGPspoint *)gpd->runtime.sbuffer;

512

remove

639

Review if these variables are already correct. I think the duplicate of the stroke filled the variables.

723

remove comments

2701

add end of line

Juanfran Matheu (jfmatheu) marked 9 inline comments as done.
source/blender/editors/gpencil/annotate_paint.c
494

pt = (tGPspoint *)gpd->runtime.sbuffer;

609

1.0f; for floats

Juanfran Matheu (jfmatheu) marked 2 inline comments as done.

After testing, I'm not sure about Diamond and Triangle shapes... maybe it would be better a simple line perpendicular to use to draw "segments".

Also, I have see some problems in the 2D panel.

source/blender/editors/gpencil/annotate_draw.c
238

Remove these 2 lines

After testing, I'm not sure about Diamond and Triangle shapes... maybe it would be better a simple line perpendicular to use to draw "segments".

Also, I have see some problems in the 2D panel.

@Antonio Vazquez (antoniov)
About Shapes:

  • Triangle is the inverted version of the closed arrow, is the same code but with a negative mult that inverts the position, so it costs nothing to keep.
  • Having a segment start/end of line is fine, I think we can re-use the code of the open arrow and invert open arrow for that as they follow the same structure, I think I only need to vary the angle where the calculations are done.
  • About the diamond, it's a common shape in this kind of tools, but well.. sure a more squared diamond shape. The cost of having it is relatively low as well as opens a way to bit more complex shapes as have one point more than any other and different structure of how points are following, being 3 groups in this matter: {open arrow + inverted}, {closed arrow + triangle}, {diamond}.
  • Having it or not having it is indifferent, it is already done but it can be removed for sure if You think there are *many styles* yet.

About the Interface:

  • Not sure why... what's the sense in Blender that the same code is used for header (horizontal) and panel (vertical)?. But yes, I can play a bit with it to fit it well in both places.
Juanfran Matheu (jfmatheu) marked an inline comment as done.EditedMay 4 2020, 2:50 PM

Can't find the problem, seems like Blender UI thing related(?
In VIEW_3D it looks nice but not in other spaces... there's a checkbox that it's not in view 3d

Also, for UI team and GP team, what option do you see more consistent over the tool header:

  1. "Style" with both enum properties aligned
  2. Each property splitted with "Start" and "End" texts
  3. Any suggestion?

IMO the line start-end for Annotation should be only the most simple and useful types and avoid lines overlapping.
My proposal:

@Matias Mendiola (mendio) I agree...only doubts about circle (or rounded) shape...this needs a lot of points and I think it don't worth enough to increase internal data structures.

@Matias Mendiola (mendio) I agree...only doubts about circle (or rounded) shape...this needs a lot of points and I think it don't worth enough to increase internal data structures.

For the circle/poly it's only 8 points, but you know better the technical implications

IMO the line start-end for Annotation should be only the most simple and useful types and avoid lines overlapping.
My proposal:

@Matias Mendiola (mendio)
Hi Matias, first of all thanks for the proposal and the mockup!

About line overlapping:

  • In the closed arrow I can solve it with some low modifications, no problem. How the code works, the arrow is a separated stroke and what you "move" is the end point of the line stroke, the arrow only adapts to that point, that means if we pull the arrow stroke outsides (arrow direction) it will be offset as you can see in the triangle shape, no overlap but not precise (not right under the mouse), the other solution is compressing the line but it will be more of a task and change too much the actual code, I'm not sure if it would be worth it. What are your opinions about this?

About arrow selection:

  • I'm ok with this.
  • I see the "invert" versions of both arrows are not preset. In terms of code, both, the original version and inverted have the exact same code but a signal that determine what direction should it turns to, so removing that won't be that much than removing few lines.
  • The perpendicular line won't be so much difficult to do as only varying the angle of the first one is fine.
  • For the circle one I would suggest to not include as far as possible if it's not +90% esential. I can do it for sure but in the worst must be simpler to not increase that much just for one style... What about rectangle or perfect squared diamond (rhombo)

About line overlapping:

  • In the closed arrow I can solve it with some low modifications, no problem. How the code works, the arrow is a separated stroke and what you "move" is the end point of the line stroke, the arrow only adapts to that point, that means if we pull the arrow stroke outsides (arrow direction) it will be offset as you can see in the triangle shape, no overlap but not precise (not right under the mouse), the other solution is compressing the line but it will be more of a task and change too much the actual code, I'm not sure if it would be worth it. What are your opinions about this?

I know what you mean, if possible the nice solution should be that the arrows end right under the cursor. Maybe @Antonio Vazquez (antoniov) could give you some clues on how to solve this

About arrow selection:

  • I'm ok with this.
  • I see the "invert" versions of both arrows are not preset. In terms of code, both, the original version and inverted have the exact same code but a signal that determine what direction should it turns to, so removing that won't be that much than removing few lines.
  • The perpendicular line won't be so much difficult to do as only varying the angle of the first one is fine.
  • For the circle one I would suggest to not include as far as possible if it's not +90% esential. I can do it for sure but in the worst must be simpler to not increase that much just for one style... What about rectangle or perfect squared diamond (rhombo)

yes, a square could be a fair replacement, without rotation IMO.

About the controls placement on the top bar is better to move them after the placement control to avoid jump when you change between Annotation tools

Start-End types updated

I know what you mean, if possible the nice solution should be that the arrows end right under the cursor. Maybe @Antonio Vazquez (antoniov) could give you some clues on how to solve this

That would be very welcome.

yes, a square could be a fair replacement, without rotation IMO.

That's nice

About the controls placement on the top bar is better to move them after the placement control to avoid jump when you change between Annotation tools

Great, I will take note of that, thanks for the feedback

Juanfran Matheu (jfmatheu) updated this revision to Diff 24405.EditedMay 5 2020, 1:04 PM

Changes over all places: stroke points calcs, immediate drawing and conversion to annotation.

Added 2 new types (segment and square) and removed 3 of them (invert, diamond, triangle).
Remake of both arrows (open/closed).

Overlapping solved.
Now mouse position is right after the end shape while dragging instead of before it.

Juanfran Matheu (jfmatheu) edited the summary of this revision. (Show Details)

2d panel fix
trail space cleanup

This revision is now accepted and ready to land.May 5 2020, 6:06 PM
Hans Goudey (HooglyBoogly) requested changes to this revision.May 5 2020, 6:53 PM

This looks good to me! Hopefully it will make annotations useful in more situations.

I can't accept this on behalf of the "UI" group, but I can offer some feedback on the small UI layouts.

release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
198

This split doesn't really work in my opinion. The properties should be split up rather than grouped together:

Start [ Type ]      End [ Type ]
206

With column headings recently added to Blender we're adding the labels closer to the properties, and also not using a colon here.

(See https://wiki.blender.org/wiki/Reference/Release_Notes/2.90/User_Interface#General)

You can emulate the column layout like this:

col.prop(props, "arrowstyle_start", text="Style Start")
col.prop(props, "arrowstyle_end", text="End")

This will also save space and make it more clear what "style" is talking about.

This revision now requires changes to proceed.May 5 2020, 6:53 PM
Juanfran Matheu (jfmatheu) updated this revision to Diff 24415.EditedMay 5 2020, 7:51 PM

Applied requested changes in UI

Juanfran Matheu (jfmatheu) marked 2 inline comments as done.May 5 2020, 7:51 PM

Looks great, thanks

source/blender/editors/gpencil/annotate_paint.c
425

Small typo: length

441

here too

This revision is now accepted and ready to land.May 5 2020, 10:33 PM
Juanfran Matheu (jfmatheu) marked 2 inline comments as done.May 5 2020, 10:39 PM
Julian Eisel (Severin) requested changes to this revision.May 6 2020, 1:28 AM

Nice one, guess this is almost ready. Just one note and a few unused-vars to be addressed.
There's arrow_start and arrow_end in a permanent runtime struct, but it is still re-filled on every redraw apparently. There's not much sense in keeping it stored then, why isn't geometry just drawn directly?

Also, we this doesn't entirely follow our code-style, see https://wiki.blender.org/wiki/Tools/ClangFormat.

source/blender/editors/gpencil/annotate_paint.c
434

gpd is unused.

676

Unused.

712

Unused.

724

Unused.

This revision now requires changes to proceed.May 6 2020, 1:28 AM
Juanfran Matheu (jfmatheu) marked 4 inline comments as done.May 6 2020, 10:40 AM

Nice one, guess this is almost ready. Just one note and a few unused-vars to be addressed.

Yuk, the last refactor turned into too much unused vars that I missed, thanks for pointing me out!

There's arrow_start and arrow_end in a permanent runtime struct, but it is still re-filled on every redraw apparently. There's not much sense in keeping it stored then, why isn't geometry just drawn directly?

The temporal struct tGPsdata is not accesible via draw so the only way we saw was to store it in bGPdata_Runtime... The draw is not in the operator but a separated process. By design points are calculated in annotate_paint.c, immediate draw (for preview) in annotate_draw.c and convert them to annotation stroke in annotation_paint.c.

Also, we this doesn't entirely follow our code-style, see https://wiki.blender.org/wiki/Tools/ClangFormat.

I will take a look on that, thanks again.
@Julian Eisel (Severin)

clang format and clean unused vars

Didn't test again, but assuming the warnings are all fixed this looks good to go.

This revision was not accepted when it landed; it landed in state Needs Review.May 6 2020, 3:58 PM
This revision was automatically updated to reflect the committed changes.