Page MenuHome

T83696: Add Additional menu to Effects panel
ClosedPublic

Authored by Antonio Vazquez (antoniov) on Dec 12 2020, 4:45 PM.

Details

Summary

This adds the missing options for the effects as it is done in modifiers.

Diff Detail

Repository
rB Blender
Branch
R292-copy-effects (branched from master)
Build Status
Buildable 11709
Build 11709: arc lint + arc unit

Event Timeline

Antonio Vazquez (antoniov) requested review of this revision.Dec 12 2020, 4:45 PM
  • GPencil: Don't use generic copy function
Hans Goudey (HooglyBoogly) requested changes to this revision.Dec 12 2020, 6:54 PM

Looks pretty good, just two pieces of feedback:

  1. The copied effect should be placed right after the one you're acting on
  2. The copy operator should have a shortcut in the keymaps-- blender default shift D and industry compatible ctrl-D

Side note: I'll be working on adding the "active modifier" concept for the gpencil modifiers, constraints, and effects for 2.92.

This revision now requires changes to proceed.Dec 12 2020, 6:54 PM
  • GPencil: Add Shift + D to duplicate and Add below

@Hans Goudey (HooglyBoogly) I don't find the Industry compatible keymaps. Where is this file? I looked and I have found only the normal keymaps.

Other issues solved.

  • GPencil: Add Keymap for Industry compatible
  • Merge branch 'master' into R292-copy-effects
yourhero (yourhero) retitled this revision from T83696: Add Additional menu to Effects panel to T83696: Add Additional menu to Effects panel Billie Jean is not my lover, she's just a girl that claims that I AM THE ONE. uuuu.Dec 13 2020, 10:35 PM
Robert Guetzkow (rjg) retitled this revision from T83696: Add Additional menu to Effects panel Billie Jean is not my lover, she's just a girl that claims that I AM THE ONE. uuuu to T83696: Add Additional menu to Effects panel.Dec 13 2020, 11:53 PM

Other than one note this looks good to me.

source/blender/editors/object/object_shader_fx.c
666

This shouldn't be necessary, right? There is no geometry being recalculated for effects?

I see it all over this file though, looks like it was just copied from the modifier code? Or this flag is being quite abused.

This revision is now accepted and ready to land.Dec 14 2020, 6:20 AM

@Hans Goudey (HooglyBoogly) Yes, it's needed..if you remove the depsgraph tag, the sreen is not updated. I guess the draw engine need it.

@Hans Goudey (HooglyBoogly) Yes, it's needed..if you remove the depsgraph tag, the sreen is not updated. I guess the draw engine need it.

Here is part of the description of that flag from`DNA_ID.h`:

* When object of other type is tagged with this flag it makes the modifier
* stack to be re-evaluated.

So maybe it's not a big deal, but it doesn't seem like the grease pencil modifier stack needs to be recalculated whenever a shader effect is changed.