Initial Grease Pencil 3.0 stage #106848

Merged
Falk David merged 224 commits from filedescriptor/blender:grease-pencil-v3 into main 2023-05-30 11:14:22 +02:00
13 changed files with 55 additions and 20 deletions
Showing only changes of commit 0e397f0aaf - Show all commits

View File

@ -273,6 +273,7 @@ extern IDTypeInfo IDType_ID_CV;
extern IDTypeInfo IDType_ID_PT;
extern IDTypeInfo IDType_ID_VO;
extern IDTypeInfo IDType_ID_SIM;
extern IDTypeInfo IDType_ID_GP;
/** Empty shell mostly, but needed for read code. */
extern IDTypeInfo IDType_ID_LINK_PLACEHOLDER;

View File

@ -33,14 +33,28 @@ using blender::Vector;
static void grease_pencil_init_data(ID *id)
{
using namespace blender::bke;
printf("grease_pencil_init_data\n");
GreasePencil *grease_pencil = (GreasePencil *)id;
grease_pencil->runtime = MEM_new<GreasePencilRuntime>(__func__);
}
static void grease_pencil_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, const int flag)
{
filedescriptor marked this conversation as resolved Outdated

Remove printfs.

Remove printfs.
using namespace blender::bke;
printf("grease_pencil_copy_data\n");
GreasePencil *grease_pencil_dst = (GreasePencil *)id_dst;
const GreasePencil *grease_pencil_src = (GreasePencil *)id_src;
grease_pencil_dst->runtime = MEM_new<GreasePencilRuntime>(__func__);
// grease_pencil_dst->runtime->root_group() = grease_pencil_src->runtime->root_group()
}
static void grease_pencil_free_data(ID *id)
{

C++ cast here (and above, I'll stop writing it now)

C++ cast here (and above, I'll stop writing it now)
printf("grease_pencil_free_data\n");

Personally I find grease_pencil_dst a longer name than it needs to be, compared to something like gp_dst, where it's easier to see the logic when reading the function IMO. I understand that's subjective though

Personally I find `grease_pencil_dst` a longer name than it needs to be, compared to something like `gp_dst`, where it's easier to see the logic when reading the function IMO. I understand that's subjective though

This used to be the case in the old grease pencil code, but tbh I find it better when it's more explicit. We ended up with gpf,gpd,gps etc. and it's really unreadable imo.

This used to be the case in the old grease pencil code, but tbh I find it better when it's more explicit. We ended up with `gpf`,`gpd`,`gps` etc. and it's really unreadable imo.
GreasePencil *grease_pencil = (GreasePencil *)id;
BKE_animdata_free(&grease_pencil->id, false);
@ -48,6 +62,9 @@ static void grease_pencil_free_data(ID *id)
// TODO: free layer tree
MEM_SAFE_FREE(grease_pencil->material_array);
MEM_delete(grease_pencil->runtime);
grease_pencil->runtime = nullptr;
}
static void grease_pencil_foreach_id(ID *id, LibraryForeachIDData *data)
@ -98,19 +115,19 @@ static void grease_pencil_blend_write(BlendWriter *writer, ID *id, const void *i
GreasePencilLayerTreeNode *node = grease_pencil->layer_tree_storage.nodes[i];
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeLeaf, node_leaf);
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf
*>(node); BLO_write_struct(writer, GreasePencilLayerTreeLeaf, node_leaf);
/* Write layer data. */
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.keys);
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.values);
writer, node_leaf->layer.frames_storage.size,
node_leaf->layer.frames_storage.values);

These two lines could use MEM_SAFE_FREE macro instead.

These two lines could use `MEM_SAFE_FREE` macro instead.

MEM_SAFE_FREE does not replace MEM_delete, it replaces MEM_freeN, which is different.

`MEM_SAFE_FREE` does not replace `MEM_delete`, it replaces `MEM_freeN`, which is different.

I was under the impression that any memory allocated with MEM_new should use MEM_delete because we also need to run the destructor.

I was under the impression that any memory allocated with `MEM_new` should use `MEM_delete` because we also need to run the destructor.
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeGroup, group);
break;
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup
*>(node); BLO_write_struct(writer, GreasePencilLayerTreeGroup, group); break;
}

How can you be sure here that the potential memory allocated to the batch_cache is freed?

if memory in batch_cache is managed by drawing code, a comment in the runtime struct should specify it.

Would be better to have a proper destructor for these structs anyway imho, even if not really needed right now.

How can you be sure here that the potential memory allocated to the `batch_cache` is freed? if memory in `batch_cache` is managed by drawing code, a comment in the runtime struct should specify it. Would be better to have a proper destructor for these structs anyway imho, even if not really needed right now.
}
}
@ -154,19 +171,20 @@ static void grease_pencil_blend_read_data(BlendDataReader *reader, ID *id)
GreasePencilLayerTreeNode *node = grease_pencil->layer_tree_storage.nodes[i];
switch (node->type) {

C++ cast here

C++ cast here
case GREASE_PENCIL_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_read_data_address(reader, &node_leaf);
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf
*>(node); BLO_read_data_address(reader, &node_leaf);
filedescriptor marked this conversation as resolved Outdated

Code is missing re-reading active_layer address...

Code is missing re-reading `active_layer` address...
/* Read layer data. */
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.keys);
reader, node_leaf->layer.frames_storage.size,
&node_leaf->layer.frames_storage.keys);
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.values);
reader, node_leaf->layer.frames_storage.size,
&node_leaf->layer.frames_storage.values);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_read_data_address(reader, &group);
break;
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup
*>(node); BLO_read_data_address(reader, &group); break;
}
}
}
@ -228,7 +246,7 @@ void *BKE_grease_pencil_add(Main *bmain, const char *name)
BoundBox *BKE_grease_pencil_boundbox_get(Object *ob)
{
BLI_assert(ob->type == OB_CURVES);
BLI_assert(ob->type == OB_GREASE_PENCIL);
const GreasePencil *grease_pencil = static_cast<const GreasePencil *>(ob->data);
if (ob->runtime.bb != nullptr && (ob->runtime.bb->flag & BOUNDBOX_DIRTY) == 0) {

View File

@ -95,6 +95,7 @@ static void id_type_init(void)
INIT_TYPE(ID_PT);
INIT_TYPE(ID_VO);
INIT_TYPE(ID_SIM);
INIT_TYPE(ID_GP);
/* Special naughty boy... */
BLI_assert(IDType_ID_LINK_PLACEHOLDER.main_listbase_index == INDEX_ID_NULL);
@ -254,6 +255,7 @@ uint64_t BKE_idtype_idcode_to_idfilter(const short idcode)
CASE_IDFILTER(WM);
CASE_IDFILTER(WO);
CASE_IDFILTER(WS);
CASE_IDFILTER(GP);
}
BLI_assert_unreachable();
@ -368,6 +370,7 @@ int BKE_idtype_idcode_to_index(const short idcode)
CASE_IDINDEX(WM);
CASE_IDINDEX(WO);
CASE_IDINDEX(WS);
CASE_IDINDEX(GP);
}
/* Special naughty boy... */

View File

@ -649,6 +649,8 @@ ListBase *which_libbase(Main *bmain, short type)
return &(bmain->volumes);
case ID_SIM:
return &(bmain->simulations);
case ID_GP:
return &(bmain->greasepencils);
}
return NULL;
}

View File

@ -1657,6 +1657,10 @@ void DepsgraphNodeBuilder::build_object_data_geometry_datablock(ID *obdata)
op_node->set_as_entry();
break;
}
case ID_GP: {
/* TODO. */
break;
}
default:
BLI_assert_msg(0, "Should not happen");
break;

View File

@ -952,7 +952,7 @@ void DepsgraphRelationBuilder::build_object_data(Object *object)
case OB_GPENCIL_LEGACY:
case OB_CURVES:
case OB_POINTCLOUD:
case OB_VOLUME:
case OB_VOLUME:
case OB_GREASE_PENCIL: {
build_object_data_geometry(object);
/* TODO(sergey): Only for until we support granular
@ -2508,6 +2508,8 @@ void DepsgraphRelationBuilder::build_object_data_geometry_datablock(ID *obdata)
}
break;
}
case ID_GP:
break;
default:
BLI_assert_msg(0, "Should not happen");
break;

View File

@ -1102,7 +1102,7 @@ static const char *template_id_browse_tip(const StructRNA *type)
case ID_PA:
return N_("Browse Particle Settings to be linked");
case ID_GD_LEGACY:
return N_("Browse Grease Pencil Data to be linked");
return N_("Browse Grease Pencil (legacy) Data to be linked");
case ID_MC:
return N_("Browse Movie Clip to be linked");
case ID_MSK:
@ -1125,6 +1125,8 @@ static const char *template_id_browse_tip(const StructRNA *type)
return N_("Browse Volume Data to be linked");
case ID_SIM:
return N_("Browse Simulation to be linked");
case ID_GP:
return N_("Browse Grease Pencil Data to be linked");
/* Use generic text. */
case ID_LI:

View File

@ -133,7 +133,8 @@ struct TreeElementIcon {
ID_CV, \
ID_PT, \
ID_VO, \
ID_SIM) || /* Only in 'blendfile' mode ... :/ */ \
ID_SIM, \
ID_GP) || /* Only in 'blendfile' mode ... :/ */ \
ELEM(GS((_id)->name), \
ID_SCR, \
ID_WM, \

View File

@ -165,6 +165,7 @@ static void get_element_operation_type(
case ID_PT:
case ID_VO:
case ID_SIM:
case ID_GP:
is_standard_id = true;
break;
case ID_WM:

View File

@ -74,6 +74,7 @@ std::unique_ptr<TreeElementID> TreeElementID::createFromID(TreeElement &legacy_t
case ID_PAL:
case ID_PC:
case ID_CF:
case ID_GP:
return std::make_unique<TreeElementID>(legacy_te, id);
case ID_IP:
BLI_assert_unreachable();

View File

@ -40,7 +40,7 @@ const EnumPropertyItem rna_enum_id_type_items[] = {
{ID_CU_LEGACY, "CURVE", ICON_CURVE_DATA, "Curve", ""},
{ID_CV, "CURVES", ICON_CURVES_DATA, "Curves", ""},
{ID_VF, "FONT", ICON_FONT_DATA, "Font", ""},
{ID_GD_LEGACY, "GREASEPENCIL", ICON_GREASEPENCIL, "Grease Pencil", ""},
{ID_GD_LEGACY, "GREASEPENCIL", ICON_GREASEPENCIL, "Grease Pencil (legacy)", ""},

This change should also be conditioned to #ifdef WITH_GREASE_PENCIL_V3 ?

This change should also be conditioned to `#ifdef WITH_GREASE_PENCIL_V3` ?

Should not be ifdef'd, and missing entry for the new GP RNA type

Should not be ifdef'd, and missing entry for the new GP RNA type
{ID_IM, "IMAGE", ICON_IMAGE_DATA, "Image", ""},
{ID_KE, "KEY", ICON_SHAPEKEY_DATA, "Key", ""},
{ID_LT, "LATTICE", ICON_LATTICE_DATA, "Lattice", ""},

View File

@ -332,7 +332,7 @@ void RNA_def_main(BlenderRNA *brna)
{"grease_pencils",
"GreasePencil",
"rna_Main_gpencils_begin",
"Grease Pencil",
"Grease Pencil (legacy)",
"Grease Pencil data-blocks",
RNA_def_main_gpencil},
{"movieclips",

View File

@ -255,7 +255,7 @@ const EnumPropertyItem rna_enum_object_type_items[] = {
{OB_CURVES, "CURVES", ICON_OUTLINER_OB_CURVES, "Hair Curves", ""},
{OB_POINTCLOUD, "POINTCLOUD", ICON_OUTLINER_OB_POINTCLOUD, "Point Cloud", ""},
{OB_VOLUME, "VOLUME", ICON_OUTLINER_OB_VOLUME, "Volume", ""},

This change should also be conditioned to #ifdef WITH_GREASE_PENCIL_V3 ?

This change should also be conditioned to `#ifdef WITH_GREASE_PENCIL_V3` ?
{OB_GPENCIL_LEGACY, "GPENCIL", ICON_OUTLINER_OB_GREASEPENCIL, "Grease Pencil", ""},
{OB_GPENCIL_LEGACY, "GPENCIL", ICON_OUTLINER_OB_GREASEPENCIL, "Grease Pencil (legacy)", ""},
{OB_GREASE_PENCIL, "GREASEPENCIL", ICON_OUTLINER_OB_GREASEPENCIL, "Grease Pencil", ""},
RNA_ENUM_ITEM_SEPR,
{OB_ARMATURE, "ARMATURE", ICON_OUTLINER_OB_ARMATURE, "Armature", ""},