Page MenuHome

Sculpt: Line gestures and Mask Line tool
Needs ReviewPublic

Authored by Pablo Dobarro (pablodp606) on Wed, Aug 26, 7:54 PM.
Tags
None
Tokens
"Love" token, awarded by tiagoffcruz."Love" token, awarded by Alrob."Love" token, awarded by Kickflipkid687."Burninate" token, awarded by sparazza."Love" token, awarded by n-pigeon."Love" token, awarded by Brandon777."Like" token, awarded by Frozen_Death_Knight.

Details

Summary

This adds support for line gesture to SculptGestureContext and
implements a Mask Line tool, which affects everything to the right of a plane
defined by the straightline gesture.

For this to work, a new WM_gesture_straightline_oneshot_modal is needed
which only runs exec when the gesture is over.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D8722_1 (branched from master)
Build Status
Buildable 10059
Build 10059: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Wed, Aug 26, 7:54 PM
Pablo Dobarro (pablodp606) created this revision.
  • Remove unnecesary multiplication
  • Use a LineGestureData struct

Adding people from the windowmanager folder. @Julian Eisel (Severin), if you don't have time, please assign someone else ;)

But also curios to know from the UI whether the mindset of "from the right of the plane" is the best here.

source/blender/editors/sculpt_paint/paint_mask.c
251–255

What;'s the difference between normal/plane and true normal/plane?

source/blender/windowmanager/intern/wm_gesture_ops.c
902

What the oneshot refers to?

It needs for sure a tool setting to change behavior to the left side of the Plane.

Awesome stuff. :D

Julian Eisel (Severin) requested changes to this revision.Mon, Aug 31, 11:32 AM
Julian Eisel (Severin) added inline comments.
source/blender/windowmanager/intern/wm_gesture_ops.c
902

I also find that term confusing. At the very least, there should be a comment clearly explaining what this function does. It's quite mythical now.

920–921

This should not use hardcoded event checks, it should use a modal keymap like other modals gesture functions do. E.g. WM_gesture_straightline_modal().

This revision now requires changes to proceed.Mon, Aug 31, 11:32 AM
Pablo Dobarro (pablodp606) marked 4 inline comments as done.
  • Review Update: Use Modal keymap
Sergey Sharybin (sergey) resigned from this revision.Thu, Sep 3, 3:46 PM

WM_gesture_straightline_oneshot_modal is still not documented, and still unclear what oneshot is about.

The modal keymap is more up to Julian to review.

Julian Eisel (Severin) requested changes to this revision.Thu, Sep 3, 3:49 PM

Yeah, requesting changes since I think this definitely needs documentation.

This revision now requires changes to proceed.Thu, Sep 3, 3:49 PM

@Julian Eisel (Severin) There are some comments in WM_api.h, should I add something else?

Please explain what this gesture is. From the function name all I get is that it has something to do with a straight line. Everything else is a mythical and it's not easy to tell from the function body either. "Oneshoot" tells me nothing.
People reading that code don't have the video for context.

  • Review Update: update comments

@Julian Eisel (Severin) Are the comments ok now? I would like to commit this to start working on the rest of the operators in T80390. There are also some planned improvements to the gesture functionality there.

Julian Eisel (Severin) requested changes to this revision.Tue, Sep 8, 5:39 PM

As discussed in the module meeting, we need a better way of dealing with the icons.
So this is either to be committed as an experimental feature or the icon is ready before the commit.

source/blender/windowmanager/WM_api.h
643–645

The description still doesn't explain what the gesture or "oneshot" is. Without seeing the video, I'd have no clue what this gesture is, how it looks and behaves.

Also, please move the comments to the source files and make them Doxygen style, like the other comments there.

This revision now requires changes to proceed.Tue, Sep 8, 5:39 PM
Pablo Dobarro (pablodp606) marked an inline comment as done.
  • Update comments
  • Move tool to "Tools with Missing Icons" experimental section