Paths

Table of Contentst

Split Grease Pencil modifier stack
AbandonedPublic
Actions

Authored by Antonio Vazquez (antoniov) on Jun 19 2018, 11:54 AM.

Details

Reviewers

Dalai Felinto (dfelinto)
Joshua Leung (aligorith)
Sergey Sharybin (sergey)

Commits

rB82fb06a2fcf0: Remove scene from GpencilModifierData struct
rB4b91a1486828: Split Grease Pencil modifier stack

Summary

Separate all logic of grease pencil modifiers and remove any reference to grease pencil in the mesh modifiers.

Thanks to Dalai Felinto for his help splitting the code and helping to fix compiler issues.

Diff Detail

Repository

rB Blender

Branch

temp-greasepencil-object-stacksplit

Build Status

Buildable 1771
Build 1771: arc lint + arc unit

Event Timeline

Antonio Vazquez (antoniov) created this revision.Jun 19 2018, 11:54 AM

Dalai, could you review the patch before integrating in greasepencil-object branch?

Antonio Vazquez (antoniov) edited the summary of this revision. (Show Details)Jun 19 2018, 11:58 AM

Antonio Vazquez (antoniov) mentioned this in D2889: Grease Pencil Branch (greasepencil-object).Jun 19 2018, 12:00 PM

Joshua Leung (aligorith) requested changes to this revision.Jun 20 2018, 5:20 AM

Joshua Leung (aligorith) added a subscriber: Joshua Leung (aligorith).

Joshua Leung (aligorith) added inline comments.

source/blender/blenkernel/BKE_gpencil.h
117–118	Since we have a BKE GP Modifiers header now, these macros should go there too
161–162	These defines (and a few others that are specifically for GP modifiers) should go into the BKE GP modifiers header
161–162	-> BKE_gpencil_modifiers.h
163	These ones should stay here. They're not really modifier specific, but rather, can be used for general GP stuff too.
source/blender/blenkernel/BKE_gpencil_modifier.h
23	Header guards need updating. Suggest `__BKE_GPENCIL_MODIFIER_H__`
53	What's the point of this enum with just two entries? Seems redundant? EDIT: Checking the original enum this mirrors, it was used for indicating whether a modifier was a "Deformation" or a "Geometry" modifier. Perhaps we could be doing something similar here for "Stroke" vs "Geometry" modifiers, instead of checking the callbacks.
62	Mesh/CV flags seem redundant here. GP Modifiers only operate on GP objects so won't need these
84	Redundant. All of them here are GP modifiers, so all will have this enabled
203	Probably not needed?
source/blender/editors/include/ED_object.h
280	Why is this define repeated (see above, line 252)?
source/blender/editors/object/object_gpencil_modifier.c
19	;)
source/blender/makesrna/intern/rna_object.c
2280	IMO, this is not great from a UX standpoint (e.g. drivers, bpy, etc.), but it will probably have to do. Ideally, we could have the "object.modifiers" property redirect to the correct underlying listbase/types based on "ob.type", but, that also complicates code here.

This revision now requires changes to proceed.Jun 20 2018, 5:20 AM

Joshua Leung (aligorith) added inline comments.Jun 20 2018, 5:28 AM

source/blender/editors/space_outliner/outliner_draw.c
850–1052	I'd probably have put the GP cases first (since GP ones are the exception here), but either way it's fine.
source/blender/makesdna/DNA_gpencil_modifier_types.h
384	Shouldn't be needed? (Unless we have a mesh sequence modifier on the GP object I didn't know about)

It think Joshua's comments are valid, but also we can leave some nitpicking to be done in the original branch (greasepencil-object), not necessarily on this patch.

Split Grease Pencil modifier stackAbandonedPublicActions

Details

Diff Detail

Event Timeline

Large Diff

Revision ContentsChangeset List

Diff 11287

.arcconfig

build_files/cmake/macros.cmake

release/scripts/startup/bl_ui/properties_data_modifier.py

source/blender/CMakeLists.txt

source/blender/blenkernel/BKE_gpencil.h

source/blender/blenkernel/BKE_gpencil_modifier.h

source/blender/blenkernel/BKE_modifier.h

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/gpencil_modifier.c

source/blender/blenkernel/intern/object.c

source/blender/blenloader/intern/readfile.c

source/blender/blenloader/intern/writefile.c

source/blender/depsgraph/intern/builder/deg_builder_nodes.cc

source/blender/draw/engines/gpencil/gpencil_draw_utils.c

source/blender/editors/gpencil/gpencil_data.c

source/blender/editors/include/ED_object.h

source/blender/editors/include/UI_interface.h

source/blender/editors/interface/interface_templates.c

source/blender/editors/object/CMakeLists.txt

source/blender/editors/object/object_gpencil_modifier.c

source/blender/editors/object/object_intern.h

source/blender/editors/object/object_modifier.c

source/blender/editors/object/object_ops.c

source/blender/editors/space_outliner/outliner_draw.c

source/blender/gpencil_modifiers/CMakeLists.txt

source/blender/gpencil_modifiers/MOD_gpencil_modifiertypes.h

source/blender/gpencil_modifiers/intern/MOD_gpencil_util.h

source/blender/gpencil_modifiers/intern/MOD_gpencil_util.c

source/blender/gpencil_modifiers/intern/MOD_gpencilbuild.c

source/blender/gpencil_modifiers/intern/MOD_gpencilcolor.c

source/blender/gpencil_modifiers/intern/MOD_gpencilhook.c

source/blender/gpencil_modifiers/intern/MOD_gpencilinstance.c

source/blender/gpencil_modifiers/intern/MOD_gpencillattice.c

source/blender/gpencil_modifiers/intern/MOD_gpencilnoise.c

source/blender/gpencil_modifiers/intern/MOD_gpenciloffset.c

source/blender/gpencil_modifiers/intern/MOD_gpencilopacity.c

source/blender/gpencil_modifiers/intern/MOD_gpencilsimplify.c

source/blender/gpencil_modifiers/intern/MOD_gpencilsmooth.c

source/blender/gpencil_modifiers/intern/MOD_gpencilsubdiv.c

source/blender/gpencil_modifiers/intern/MOD_gpencilthick.c

source/blender/gpencil_modifiers/intern/MOD_gpenciltint.c

source/blender/makesdna/DNA_gpencil_modifier_types.h

source/blender/makesdna/DNA_modifier_types.h

source/blender/makesdna/DNA_object_types.h

source/blender/makesdna/intern/makesdna.c

source/blender/makesrna/RNA_access.h

source/blender/makesrna/RNA_enum_types.h

source/blender/makesrna/intern/CMakeLists.txt

source/blender/makesrna/intern/makesrna.c

source/blender/makesrna/intern/rna_gpencil_modifier.c

source/blender/makesrna/intern/rna_internal.h

source/blender/makesrna/intern/rna_modifier.c

source/blender/makesrna/intern/rna_object.c

source/blender/makesrna/intern/rna_ui_api.c

source/blender/modifiers/CMakeLists.txt

source/blender/modifiers/MOD_modifiertypes.h

source/blender/modifiers/intern/MOD_armature.c

source/blender/modifiers/intern/MOD_array.c

source/blender/modifiers/intern/MOD_bevel.c

source/blender/modifiers/intern/MOD_boolean.c

source/blender/modifiers/intern/MOD_build.c

source/blender/modifiers/intern/MOD_cast.c

source/blender/modifiers/intern/MOD_cloth.c

source/blender/modifiers/intern/MOD_collision.c

source/blender/modifiers/intern/MOD_correctivesmooth.c

source/blender/modifiers/intern/MOD_curve.c

source/blender/modifiers/intern/MOD_datatransfer.c

source/blender/modifiers/intern/MOD_decimate.c

source/blender/modifiers/intern/MOD_displace.c

source/blender/modifiers/intern/MOD_dynamicpaint.c

source/blender/modifiers/intern/MOD_edgesplit.c

source/blender/modifiers/intern/MOD_explode.c

source/blender/modifiers/intern/MOD_fluidsim.c

Split Grease Pencil modifier stack
AbandonedPublic
Actions

Revision Contents
Changeset List