blender
/
blender
133
398
Fork 579
Code Issues 6.3k Pull Requests 746 Projects 19 Wiki Activity

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
Conversation 8 Commits 224 Files Changed 94 +48 -18
2 changed files with 48 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 52a4453b95 - Show all commits

52
source/blender/blenkernel/intern/grease_pencil.cc
View File

@ -650,7 +650,9 @@ const greasepencil::Layer &GreasePencilRuntime::active_layer() const
greasepencil::Layer &GreasePencilRuntime::active_layer_for_write()
{
BLI_assert(this->active_layer_index_ >= 0);
return this->root_group_for_write().children_for_write()[this->active_layer_index_]->as_layer_for_write();
return this->root_group_for_write()
.children_for_write()[this->active_layer_index_]
->as_layer_for_write();
}
void GreasePencilRuntime::set_active_layer_index(int index)
@ -1137,8 +1139,10 @@ static void read_layer_from_storage(blender::bke::greasepencil::LayerGroup &curr
new_layer.viewlayer_name = BLI_strdup(node_leaf->layer.viewlayer_name);
}
Vector<LayerMask> masks = new_layer.masks_for_write();
LISTBASE_FOREACH (bGPDlayer_Mask *, mask, &node_leaf->layer.masks_storage) {
LayerMask new_mask = LayerMask(mask->name);
GreasePencilLayerMaskStorage *masks_storage = &node_leaf->layer.masks_storage;
for (int mask_i = 0; mask_i < masks_storage->masks_size; mask_i++) {
GreasePencilLayerMask *mask = &masks_storage->masks[mask_i];
Hans Goudey commented 2023-05-02 23:51:09 +02:00
Outdated
Review
Copy Link

GreasePencilLayerMask *mask = &masks_storage -> GreasePencilLayerMask &mask = masks_storage

`GreasePencilLayerMask *mask = &masks_storage` -> `GreasePencilLayerMask &mask = masks_storage`
LayerMask new_mask = LayerMask(mask->layer_name);
new_mask.flag = mask->flag;
masks.append(std::move(new_mask));
}
@ -1224,12 +1228,17 @@ static void save_layer_to_storage(const blender::bke::greasepencil::Layer &node,
if (node.viewlayer_name) {
new_leaf->layer.viewlayer_name = BLI_strdup(node.viewlayer_name);
}
BLI_listbase_clear(&new_leaf->layer.masks_storage);
for (const bke::greasepencil::LayerMask &mask : node.masks()) {
GreasePencilLayerMask *new_mask = MEM_cnew<GreasePencilLayerMask>(__func__);
new_mask->layer_name = BLI_strdup(mask.layer_name);
new_mask->flag = mask.flag;
BLI_addtail(&new_leaf->layer.masks_storage, new_mask);
GreasePencilLayerMaskStorage *masks_storage = &new_leaf->layer.masks_storage;
masks_storage->masks_size = node.masks().size();
if (masks_storage->masks_size > 0) {
Hans Goudey commented 2023-04-25 04:28:57 +02:00
Outdated
Review
Copy Link

These two things are handled by bke::CurvesGeometry::blend_read() now.

These two things are handled by `bke::CurvesGeometry::blend_read()` now.
masks_storage->masks = MEM_cnew_array<GreasePencilLayerMask>(masks_storage->masks_size,
__func__);
for (const int mask_i : node.masks().index_range()) {
Hans Goudey commented 2023-04-25 04:29:33 +02:00
Outdated
Review
Copy Link

Dirty is the initial state of a SharedCache, no need to tag it that way explicitly here.

Dirty is the initial state of a `SharedCache`, no need to tag it that way explicitly here.
const bke::greasepencil::LayerMask &mask = node.masks()[mask_i];
GreasePencilLayerMask *new_mask = &masks_storage->masks[mask_i];
new_mask->layer_name = BLI_strdup(mask.layer_name);
new_mask->flag = mask.flag;
}
}
new_leaf->layer.opacity = node.opacity;
copy_v4_v4(new_leaf->layer.tint_color, node.tint_color);
@ -1328,8 +1337,12 @@ void GreasePencil::read_layer_tree_storage(BlendDataReader *reader)
BLO_read_data_address(reader, &node_leaf->layer.frames_storage.values);
Hans Goudey commented 2023-05-03 16:24:21 +02:00
Outdated
Review
Copy Link

Use reinterpret_cast over C style cast

Use `reinterpret_cast` over C style cast
BLO_read_data_address(reader, &node_leaf->layer.parsubstr);
BLO_read_data_address(reader, &node_leaf->layer.viewlayer_name);
BLO_read_list(reader, &node_leaf->layer.masks_storage);
LISTBASE_FOREACH (GreasePencilLayerMask *, mask, &node_leaf->layer.masks_storage) {
/* Read layer masks. */
GreasePencilLayerMaskStorage *masks_storage = &node_leaf->layer.masks_storage;
BLO_read_data_address(reader, &masks_storage->masks);
for (int mask_i = 0; mask_i < masks_storage->masks_size; mask_i++) {
GreasePencilLayerMask *mask = &masks_storage->masks[mask_i];
BLO_read_data_address(reader, &mask->layer_name);
}
break;
@ -1363,8 +1376,13 @@ void GreasePencil::write_layer_tree_storage(BlendWriter *writer)
node_leaf->layer.frames_storage.values);
BLO_write_string(writer, node_leaf->layer.parsubstr);
BLO_write_string(writer, node_leaf->layer.viewlayer_name);
BLO_write_struct_list(writer, GreasePencilLayerMask, &node_leaf->layer.masks_storage);
LISTBASE_FOREACH (GreasePencilLayerMask *, mask, &node_leaf->layer.masks_storage) {
/* Write layer masks. */
GreasePencilLayerMaskStorage *masks_storage = &node_leaf->layer.masks_storage;
BLO_write_struct_array(
writer, GreasePencilLayerMask, masks_storage->masks_size, masks_storage->masks);
for (int mask_i = 0; mask_i < masks_storage->masks_size; mask_i++) {
GreasePencilLayerMask *mask = &masks_storage->masks[mask_i];
BLO_write_string(writer, mask->layer_name);
}
break;
@ -1402,11 +1420,15 @@ void GreasePencil::free_layer_tree_storage()
if (node_leaf->layer.viewlayer_name) {
MEM_freeN(node_leaf->layer.viewlayer_name);
}
LISTBASE_FOREACH_MUTABLE (GreasePencilLayerMask *, mask, &node_leaf->layer.masks_storage) {
GreasePencilLayerMaskStorage *masks_storage = &node_leaf->layer.masks_storage;
for (int mask_i = 0; mask_i < masks_storage->masks_size; mask_i++) {
GreasePencilLayerMask *mask = &masks_storage->masks[mask_i];
if (mask->layer_name) {
MEM_freeN(mask->layer_name);
}
MEM_freeN(mask);
}
if (masks_storage->masks) {
MEM_freeN(masks_storage->masks);
}
MEM_freeN(node_leaf);
break;

14
source/blender/makesdna/DNA_grease_pencil_types.h
View File

@ -172,7 +172,6 @@ typedef enum GreasePencilLayerMaskFlag {
* A grease pencil layer mask stores the name of a layer that is the mask.
*/
typedef struct GreasePencilLayerMask {
struct GreasePencilLayerMask *next, *prev;
/**
* The name of the layer that is the mask.
*/
@ -184,6 +183,15 @@ typedef struct GreasePencilLayerMask {
char _pad[6];
Hans Goudey commented 2023-04-25 04:32:41 +02:00
Outdated
Review
Copy Link

Pretty sure null-terminated goes without saying for char * pointers in DNA, I don't think it's worth mentioning here

Pretty sure null-terminated goes without saying for `char *` pointers in DNA, I don't think it's worth mentioning here
Sergey Sharybin commented 2023-04-25 10:10:22 +02:00
Outdated
Review
Copy Link

Not sure about dynamic names for strings. It is not something typically used in the DNA.

Not sure about dynamic names for strings. It is not something typically used in the DNA.
Hans Goudey commented 2023-04-25 14:52:41 +02:00
Outdated
Review
Copy Link

It's been done more recently I think. It's properly integrated with RNA now too, to avoid that boilerplate. It's nice not to have to worry about choosing a future-proof length.

It's been done more recently I think. It's properly integrated with RNA now too, to avoid that boilerplate. It's nice not to have to worry about choosing a future-proof length.
} GreasePencilLayerMask;
/**
* Storage for the layer masks.
*/
typedef struct GreasePencilLayerMaskStorage {
GreasePencilLayerMask *masks;
int masks_size;
char _pad[4];
} GreasePencilLayerMaskStorage;
/**
* Type of parent of a layer. #GreasePencilLayer.parent_type
*/
@ -237,9 +245,9 @@ typedef struct GreasePencilLayer {
*/
char *viewlayer_name;
/**
* List of `GreasePencilLayerMask`. Only used for storage in the .blend file.
* Only used for storage in the .blend file.
*/
ListBase masks_storage;
GreasePencilLayerMaskStorage masks_storage;
Hans Goudey commented 2023-04-25 05:06:34 +02:00
Outdated
Review
Copy Link

I wonder if you'd consider making this an array rather than a ListBase in DNA. Then the memory could be reused when the file is loaded, fewer small allocations would be needed, and the C++ type wouldn't have to have next and prev pointers even during runtime.

I wonder if you'd consider making this an array rather than a `ListBase` in DNA. Then the memory could be reused when the file is loaded, fewer small allocations would be needed, and the C++ type wouldn't have to have `next` and `prev` pointers even during runtime.
/**
* Opacity of the layer.
*/