blender
/
blender
134
398
Fork 579
Code Issues 6.3k Pull Requests 750 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 +233 -54
4 changed files with 233 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 80e88a1ebd - Show all commits

18
source/blender/blenkernel/BKE_grease_pencil.hh
View File

@ -10,11 +10,8 @@
#include "BLI_function_ref.hh"
#include "BLI_map.hh"
#include "BLI_math_vector.h"
#include "BLI_math_vector_types.hh"
#include "BLI_shared_cache.hh"
#include "BLI_stack.hh"
#include "BLI_string.h"
#include "BLI_utility_mixins.hh"
#include "DNA_gpencil_legacy_types.h"
@ -49,7 +46,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode, NonMovable {
*/
constexpr bool is_group() const
{
return this->type == GREASE_PENCIL_LAYER_TREE_GROUP;
return this->type == GP_LAYER_TREE_GROUP;
}
Hans Goudey commented 2023-05-02 23:30:37 +02:00
Outdated
Review
Copy Link

Is defining these as constexpr helpful? I sort of doubt any real computation is done on these nodes at compile time. But maybe?

Is defining these as `constexpr` helpful? I sort of doubt any real computation is done on these nodes at compile time. But maybe?
Hans Goudey commented 2023-05-03 16:08:15 +02:00
Outdated
Review
Copy Link

The const in the const bool return type means nothing here

The `const` in the `const bool` return type means nothing here
/**
@ -57,7 +54,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode, NonMovable {
*/
constexpr bool is_layer() const
{
return this->type == GREASE_PENCIL_LAYER_TREE_LEAF;
return this->type == GP_LAYER_TREE_LEAF;
}
/**
@ -89,7 +86,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode, NonMovable {
* A layer maps drawings to scene frames. It can be thought of as one independent channel in the
* timeline.
*/
class Layer : public TreeNode, ::GreasePencilLayer {
class Layer : public TreeNode, public ::GreasePencilLayer {
private:
/**
* This Map maps a scene frame number (key) to a GreasePencilFrame. This struct holds an index
@ -120,9 +117,10 @@ class Layer : public TreeNode, ::GreasePencilLayer {
mutable SharedCache<Vector<int>> sorted_keys_cache_;
Bastien Montagne commented 2023-05-06 17:14:03 +02:00
Outdated
Review
Copy Link

exclusive

`exclusive`
public:
Layer() : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF), frames_() {}
explicit Layer(StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_LEAF, name), frames_() {}
Layer();
Sergey Sharybin commented 2023-04-25 09:22:25 +02:00
Outdated
Review
Copy Link

I am a bit confused here.

Following the text, it seems it should be Frame and Drawing (not Time and Frame).

Or, the description needs to be updated (and a good argument made why the common terms Time and Frame are redefined in the grease pencil).

I am a bit confused here. Following the text, it seems it should be `Frame` and `Drawing` (not `Time` and `Frame`). Or, the description needs to be updated (and a good argument made why the common terms `Time` and `Frame` are redefined in the grease pencil).
Falk David commented 2023-04-25 17:01:03 +02:00
Outdated
Review
Copy Link

Ah yes, Frame is the term we use for a Drawing at a particular time. I agree that this is very confusing here. I am not sure what a better term would be.

Ah yes, `Frame` is the term we use for a `Drawing` at a particular time. I agree that this is very confusing here. I am not sure what a better term would be.
Sergey Sharybin commented 2023-04-28 16:51:27 +02:00
Outdated
Review
Copy Link

I see. The GreasePencilFrame is nice and unambiguous. And within a context of GP it is not bad to stick to general rule "frame means grease pencil frame".

But when you refer to frame as a moment in time, is the best to use "scene frame". So, it will be something rhe first drawing starts at scene frame 0 and ends at scene frame 5 (exclusive) (btw exclusive not excusive)`.

Also use the same "scene frame" in diagram below. Generally time in Blender is scene frame number divided by the scene fps, and is measured in seconds.

I see. The `GreasePencilFrame` is nice and unambiguous. And within a context of GP it is not bad to stick to general rule "frame means grease pencil frame". But when you refer to frame as a moment in time, is the best to use "scene frame". So, it will be something `rhe first drawing starts at scene frame 0 and ends at scene frame 5 (exclusive)` (btw exclusive not excusive)`. Also use the same "scene frame" in diagram below. Generally time in Blender is scene frame number divided by the scene fps, and is measured in seconds.
explicit Layer(StringRefNull name);
Layer(const Layer &other);
~Layer();
/**
* \returns the frames mapping.
@ -167,8 +165,8 @@ class LayerGroup : public TreeNode {
using LayerIndexIterFn = FunctionRef<void(int64_t, Layer &)>;
Hans Goudey commented 2023-05-03 16:11:22 +02:00
Outdated
Review
Copy Link

The function is "is_locked" and the comment says "return if it is locked". This sort of comment doesn't add anything IMO, just wastes space. If there is a comment, maybe it should use a different word besides "locked" to describe the state. Or there doesn't need to be a comment at all.

The function is "is_locked" and the comment says "return if it is locked". This sort of comment doesn't add anything IMO, just wastes space. If there is a comment, maybe it should use a different word besides "locked" to describe the state. Or there doesn't need to be a comment at all.
public:
LayerGroup() : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP) {}
explicit LayerGroup(const StringRefNull name) : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP, name) {}
LayerGroup() : TreeNode(GP_LAYER_TREE_GROUP) {}
explicit LayerGroup(const StringRefNull name) : TreeNode(GP_LAYER_TREE_GROUP, name) {}
LayerGroup(const LayerGroup &other);
public:

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

@ -21,6 +21,7 @@
#include "BLI_polyfill_2d.h"
#include "BLI_span.hh"
#include "BLI_stack.hh"
#include "BLI_string.h"
#include "BLO_read_write.h"
@ -67,7 +68,7 @@ static void grease_pencil_copy_data(Main * /*bmain*/,
for (int i = 0; i < grease_pencil_src->drawing_array_size; i++) {
const GreasePencilDrawingOrReference *src_drawing_or_ref = grease_pencil_src->drawing_array[i];
switch (src_drawing_or_ref->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
const GreasePencilDrawing *src_drawing = reinterpret_cast<const GreasePencilDrawing *>(
src_drawing_or_ref);
grease_pencil_dst->drawing_array[i] = reinterpret_cast<GreasePencilDrawingOrReference *>(
@ -83,7 +84,7 @@ static void grease_pencil_copy_data(Main * /*bmain*/,
dst_drawing->runtime->triangles_cache = src_drawing->runtime->triangles_cache;
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
const GreasePencilDrawingReference *src_drawing_reference =
reinterpret_cast<const GreasePencilDrawingReference *>(src_drawing_or_ref);
grease_pencil_dst->drawing_array[i] = reinterpret_cast<GreasePencilDrawingOrReference *>(
@ -128,7 +129,7 @@ static void grease_pencil_foreach_id(ID *id, LibraryForeachIDData *data)
}
for (int i = 0; i < grease_pencil->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = grease_pencil->drawing_array[i];
Bastien Montagne commented 2023-05-26 12:51:09 +02:00
Outdated
Review
Copy Link

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.
if (drawing_or_ref->type == GREASE_PENCIL_DRAWING_REFERENCE) {
if (drawing_or_ref->type == GP_DRAWING_REFERENCE) {
GreasePencilDrawingReference *drawing_reference =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
BKE_LIB_FOREACHID_PROCESS_IDSUPER(data, drawing_reference->id_reference, IDWALK_CB_USER);
@ -194,7 +195,7 @@ static void grease_pencil_blend_read_lib(BlendLibReader *reader, ID *id)
}
for (int i = 0; i < grease_pencil->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = grease_pencil->drawing_array[i];
if (drawing_or_ref->type == GREASE_PENCIL_DRAWING_REFERENCE) {
if (drawing_or_ref->type == GP_DRAWING_REFERENCE) {
GreasePencilDrawingReference *drawing_reference =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
BLO_read_id_address(reader, grease_pencil->id.lib, &drawing_reference->id_reference);
@ -210,7 +211,7 @@ static void grease_pencil_blend_read_expand(BlendExpander *expander, ID *id)
}
for (int i = 0; i < grease_pencil->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = grease_pencil->drawing_array[i];
if (drawing_or_ref->type == GREASE_PENCIL_DRAWING_REFERENCE) {
if (drawing_or_ref->type == GP_DRAWING_REFERENCE) {
GreasePencilDrawingReference *drawing_reference =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
BLO_expand(expander, drawing_reference->id_reference);
@ -297,9 +298,38 @@ Layer &TreeNode::as_layer_for_write()
return *static_cast<Layer *>(this);
}
Layer::Layer(const Layer &other) : TreeNode::TreeNode(other)
Layer::Layer() : TreeNode::TreeNode(GP_LAYER_TREE_LEAF), frames_()
{
this->frames_ = other.frames_;
this->parsubstr = nullptr;
this->viewlayer_name = nullptr;
filedescriptor marked this conversation as resolved Outdated
Falk David commented 2023-04-12 10:48:52 +02:00
Outdated
Review
Copy Link

Make sure to not overwrite min and max but account any value already present. E.g. use math::min_max(...).

Make sure to not overwrite `min` and `max` but account any value already present. E.g. use `math::min_max(...)`.
}
Layer::Layer(StringRefNull name) : TreeNode::TreeNode(GP_LAYER_TREE_LEAF, name), frames_()
{
this->parsubstr = nullptr;
this->viewlayer_name = nullptr;
}
Layer::Layer(const Layer &other) : TreeNode::TreeNode(other), frames_(other.frames_)
{
this->parsubstr = nullptr;
this->viewlayer_name = nullptr;
if (other.parsubstr != nullptr) {
this->parsubstr = BLI_strdup(other.parsubstr);
}
if (other.viewlayer_name != nullptr) {
this->viewlayer_name = BLI_strdup(other.viewlayer_name);
}
}
Layer::~Layer()
{
if (this->parsubstr != nullptr) {
MEM_freeN(this->parsubstr);
}
if (this->viewlayer_name != nullptr) {
MEM_freeN(this->viewlayer_name);
}
}
const Map<int, GreasePencilFrame> &Layer::frames() const
@ -597,7 +627,7 @@ BoundBox *BKE_grease_pencil_boundbox_get(Object *ob)
for (int i = 0; i < grease_pencil->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = grease_pencil->drawing_array[i];
switch (drawing_or_ref->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_or_ref);
const blender::bke::CurvesGeometry &curves = drawing->geometry.wrap();
@ -607,7 +637,7 @@ BoundBox *BKE_grease_pencil_boundbox_get(Object *ob)
}
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
/* TODO */
break;
}
@ -850,7 +880,7 @@ void GreasePencil::remove_drawing(int index_to_remove)
GreasePencilDrawingOrReference *drawing_or_ref_to_remove =
this->drawings_for_write()[last_drawing_index];
switch (drawing_or_ref_to_remove->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
GreasePencilDrawing *drawing_to_remove = reinterpret_cast<GreasePencilDrawing *>(
drawing_or_ref_to_remove);
drawing_to_remove->geometry.wrap().~CurvesGeometry();
@ -859,7 +889,7 @@ void GreasePencil::remove_drawing(int index_to_remove)
MEM_freeN(drawing_to_remove);
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
GreasePencilDrawingReference *drawing_reference_to_remove =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref_to_remove);
MEM_freeN(drawing_reference_to_remove);
@ -891,11 +921,11 @@ void GreasePencil::foreach_visible_drawing(
continue;
}
GreasePencilDrawingOrReference *drawing_or_reference = drawings[index];
if (drawing_or_reference->type == GREASE_PENCIL_DRAWING) {
if (drawing_or_reference->type == GP_DRAWING) {
GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_or_reference);
function(*drawing);
}
else if (drawing_or_reference->type == GREASE_PENCIL_DRAWING_REFERENCE) {
else if (drawing_or_reference->type == GP_DRAWING_REFERENCE) {
/* TODO */
}
}
@ -943,7 +973,7 @@ void GreasePencil::read_drawing_array(BlendDataReader *reader)
BLO_read_data_address(reader, &this->drawing_array[i]);
GreasePencilDrawingOrReference *drawing_or_ref = this->drawing_array[i];
switch (drawing_or_ref->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_or_ref);
drawing->geometry.wrap().blend_read(*reader);
/* Initialize runtime data. */
@ -953,7 +983,7 @@ void GreasePencil::read_drawing_array(BlendDataReader *reader)
drawing->runtime->triangles_cache.tag_dirty();
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
GreasePencilDrawingReference *drawing_reference =
Hans Goudey commented 2023-04-25 04:19:31 +02:00
Outdated
Review
Copy Link

Pass a slice of the result vector to BLI_polyfill_calc_arena directly rather than allocating a temporary array here

Pass a slice of the result vector to `BLI_polyfill_calc_arena` directly rather than allocating a temporary array here
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
BLO_read_data_address(reader, &drawing_reference->id_reference);
@ -969,13 +999,13 @@ void GreasePencil::write_drawing_array(BlendWriter *writer)
for (int i = 0; i < this->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = this->drawing_array[i];
switch (drawing_or_ref->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_or_ref);
BLO_write_struct(writer, GreasePencilDrawing, drawing);
drawing->geometry.wrap().blend_write(*writer, this->id);
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
GreasePencilDrawingReference *drawing_reference =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
BLO_write_struct(writer, GreasePencilDrawingReference, drawing_reference);
@ -993,7 +1023,7 @@ void GreasePencil::free_drawing_array()
for (int i = 0; i < this->drawing_array_size; i++) {
GreasePencilDrawingOrReference *drawing_or_ref = this->drawing_array[i];
switch (drawing_or_ref->type) {
case GREASE_PENCIL_DRAWING: {
case GP_DRAWING: {
GreasePencilDrawing *drawing = reinterpret_cast<GreasePencilDrawing *>(drawing_or_ref);
drawing->geometry.wrap().~CurvesGeometry();
MEM_delete(drawing->runtime);
@ -1001,7 +1031,7 @@ void GreasePencil::free_drawing_array()
MEM_freeN(drawing);
break;
}
case GREASE_PENCIL_DRAWING_REFERENCE: {
case GP_DRAWING_REFERENCE: {
GreasePencilDrawingReference *drawing_reference =
reinterpret_cast<GreasePencilDrawingReference *>(drawing_or_ref);
MEM_freeN(drawing_reference);
Hans Goudey commented 2023-04-25 04:24:39 +02:00
Outdated
Review
Copy Link

What about replacing these with slightly more generic functions like this?

template<typename T> static void grow_array(T **array,  int *size, const int add_size) {}
template<typename T> static void shring_array(T **array,  int *size, const int shrink_size) {}

That would allow for simpler variable naming inside, and would tell the reader "there's nothing special going on here, just resizing some arrays" which makes it easier to skim

What about replacing these with slightly more generic functions like this? ``` template<typename T> static void grow_array(T **array, int *size, const int add_size) {} template<typename T> static void shring_array(T **array, int *size, const int shrink_size) {} ``` That would allow for simpler variable naming inside, and would tell the reader "there's nothing special going on here, just resizing some arrays" which makes it easier to skim
@ -1054,6 +1084,22 @@ static void save_layer_to_storage(const blender::bke::greasepencil::Layer &node,
values[i] = node.frames().lookup(keys[i]);
Hans Goudey commented 2023-04-25 04:25:46 +02:00
Outdated
Review
Copy Link

IndexRange(new_drawings.size()) -> new_drawings.index_range()

Maybe regex this one? It's probably in this patch more than once

`IndexRange(new_drawings.size())` -> `new_drawings.index_range()` Maybe regex this one? It's probably in this patch more than once
}
}
new_leaf->layer.parent_type = node.parent_type;
new_leaf->layer.blend_mode = node.blend_mode;
new_leaf->layer.parent = node.parent;
if (node.parsubstr) {
new_leaf->layer.parsubstr = BLI_strdup(node.parsubstr);
}
if (node.viewlayer_name) {
Hans Goudey commented 2023-04-25 04:26:22 +02:00
Outdated
Review
Copy Link

Declare all arguments const in definitions

Declare all arguments const in definitions
new_leaf->layer.viewlayer_name = BLI_strdup(node.viewlayer_name);
}
new_leaf->layer.opacity = node.opacity;
copy_v4_v4(new_leaf->layer.tint_color, node.tint_color);
copy_v3_v3(new_leaf->layer.location, node.location);
Hans Goudey commented 2023-04-25 04:27:33 +02:00
Outdated
Review
Copy Link

Is the - meant to be a 1)? I think 1. is a bit more common in code comments than 1)

Is the `-` meant to be a `1)`? I think `1.` is a bit more common in code comments than `1)`
copy_v3_v3(new_leaf->layer.rotation, node.rotation);
copy_v3_v3(new_leaf->layer.scale, node.scale);
/* Store pointer. */
*dst = reinterpret_cast<GreasePencilLayerTreeNode *>(new_leaf);
}
@ -1098,6 +1144,32 @@ void GreasePencil::save_layer_tree_to_storage()
this->layer_tree_storage.active_layer_index = this->runtime->active_layer_index();
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`
}
static void read_layer_from_storage(blender::bke::greasepencil::LayerGroup &current_group,
GreasePencilLayerTreeLeaf *node_leaf)
{
using namespace blender::bke::greasepencil;
Layer new_layer(node_leaf->base.name);
for (int i = 0; i < node_leaf->layer.frames_storage.size; i++) {
new_layer.insert_frame(node_leaf->layer.frames_storage.keys[i],
node_leaf->layer.frames_storage.values[i]);
}
new_layer.parent_type = node_leaf->layer.parent_type;
new_layer.blend_mode = node_leaf->layer.blend_mode;
new_layer.parent = node_leaf->layer.parent;
if (node_leaf->layer.parsubstr) {
new_layer.parsubstr = BLI_strdup(node_leaf->layer.parsubstr);
}
if (node_leaf->layer.viewlayer_name) {
new_layer.viewlayer_name = BLI_strdup(node_leaf->layer.viewlayer_name);
}
new_layer.opacity = node_leaf->layer.opacity;
copy_v4_v4(new_layer.tint_color, node_leaf->layer.tint_color);
copy_v3_v3(new_layer.location, node_leaf->layer.location);
copy_v3_v3(new_layer.rotation, node_leaf->layer.rotation);
copy_v3_v3(new_layer.scale, node_leaf->layer.scale);
current_group.add_layer(std::move(new_layer));
}
static int read_layer_node_recursive(blender::bke::greasepencil::LayerGroup &current_group,
GreasePencilLayerTreeNode **nodes,
int node_index)
@ -1106,19 +1178,15 @@ static int read_layer_node_recursive(blender::bke::greasepencil::LayerGroup &cur
GreasePencilLayerTreeNode *node = nodes[node_index];
int total_nodes_read = 0;
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
case GP_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
Layer new_layer(node_leaf->base.name);
for (int i = 0; i < node_leaf->layer.frames_storage.size; i++) {
new_layer.insert_frame(node_leaf->layer.frames_storage.keys[i],
node_leaf->layer.frames_storage.values[i]);
}
current_group.add_layer(std::move(new_layer));
read_layer_from_storage(current_group, node_leaf);
total_nodes_read++;
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
case GP_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
/* Read layer group. */
LayerGroup new_group(group->base.name);
for (int i = 0; i < group->children_num; i++) {
filedescriptor marked this conversation as resolved Outdated
Bastien Montagne commented 2023-05-25 14:05:56 +02:00
Outdated
Review
Copy Link

Once again this is re-generating data on write, which will systematically destroy the partial undo process, since it means that the data will always be modified on each and every undo step.

Once again this is re-generating data on write, which will systematically destroy the partial undo process, since it means that the data will always be modified on each and every undo step.
total_nodes_read += read_layer_node_recursive(
@ -1141,7 +1209,7 @@ void GreasePencil::load_layer_tree_from_storage()
/* The first node should be the root group. */
GreasePencilLayerTreeNode *root = reinterpret_cast<GreasePencilLayerTreeNode *>(
this->layer_tree_storage.nodes[0]);
BLI_assert(root->type == GREASE_PENCIL_LAYER_TREE_GROUP);
BLI_assert(root->type == GP_LAYER_TREE_GROUP);
int total_nodes_read = 0;
for (int i = 0; i < reinterpret_cast<GreasePencilLayerTreeGroup *>(root)->children_num; i++) {
total_nodes_read += read_layer_node_recursive(this->runtime->root_group_for_write(),
@ -1159,16 +1227,18 @@ void GreasePencil::read_layer_tree_storage(BlendDataReader *reader)
BLO_read_data_address(reader, &this->layer_tree_storage.nodes[i]);
GreasePencilLayerTreeNode *node = this->layer_tree_storage.nodes[i];
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
case GP_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_read_data_address(reader, &node_leaf->base.name);
/* Read layer data. */
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.
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.keys);
BLO_read_data_address(reader, &node_leaf->layer.frames_storage.values);
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.
BLO_read_data_address(reader, &node_leaf->layer.parsubstr);
BLO_read_data_address(reader, &node_leaf->layer.viewlayer_name);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
case GP_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_read_data_address(reader, &group->base.name);
break;
@ -1184,7 +1254,7 @@ void GreasePencil::write_layer_tree_storage(BlendWriter *writer)
for (int i = 0; i < this->layer_tree_storage.nodes_num; i++) {
GreasePencilLayerTreeNode *node = this->layer_tree_storage.nodes[i];
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
case GP_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeLeaf, node_leaf);
BLO_write_string(writer, node_leaf->base.name);
@ -1195,9 +1265,11 @@ void GreasePencil::write_layer_tree_storage(BlendWriter *writer)
GreasePencilFrame,
node_leaf->layer.frames_storage.size,
node_leaf->layer.frames_storage.values);
BLO_write_string(writer, node_leaf->layer.parsubstr);
BLO_write_string(writer, node_leaf->layer.viewlayer_name);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
case GP_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeGroup, group);
BLO_write_string(writer, group->base.name);
@ -1218,16 +1290,22 @@ void GreasePencil::free_layer_tree_storage()
MEM_freeN(node->name);
}
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
case GP_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
if (node_leaf->layer.frames_storage.size > 0) {
MEM_freeN(node_leaf->layer.frames_storage.keys);
MEM_freeN(node_leaf->layer.frames_storage.values);
}
if (node_leaf->layer.parsubstr) {
MEM_freeN(node_leaf->layer.parsubstr);
}
if (node_leaf->layer.viewlayer_name) {
MEM_freeN(node_leaf->layer.viewlayer_name);
}
MEM_freeN(node_leaf);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
case GP_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
MEM_freeN(group);
break;

35
source/blender/blenkernel/intern/grease_pencil_convert_legacy.cc
View File

@ -186,18 +186,47 @@ void legacy_gpencil_to_grease_pencil(Main &bmain, GreasePencil &grease_pencil, b
int i = 0, layer_idx = 0;
int active_layer_index = 0;
LISTBASE_FOREACH_INDEX (bGPDlayer *, gpl, &gpd.layers, layer_idx) {
Layer &new_layer = grease_pencil.root_group_for_write().add_layer(Layer(gpl->info));
if ((gpl->flag & GP_LAYER_ACTIVE) != 0) {
active_layer_index = layer_idx;
}
Layer &new_layer = grease_pencil.root_group_for_write().add_layer(Layer(gpl->info));
/* Flags. */
SET_FLAG_FROM_TEST(new_layer.flag, (gpl->flag & GP_LAYER_HIDE), GP_LAYER_TREE_NODE_HIDE);
SET_FLAG_FROM_TEST(new_layer.flag, (gpl->flag & GP_LAYER_LOCKED), GP_LAYER_TREE_NODE_LOCKED);
SET_FLAG_FROM_TEST(new_layer.flag, (gpl->flag & GP_LAYER_SELECT), GP_LAYER_TREE_NODE_SELECT);
SET_FLAG_FROM_TEST(new_layer.flag, (gpl->flag & GP_LAYER_FRAMELOCK), GP_LAYER_TREE_NODE_MUTE);
SET_FLAG_FROM_TEST(
new_layer.flag, (gpl->flag & GP_LAYER_USE_LIGHTS), GP_LAYER_TREE_NODE_USE_LIGHTS);
new_layer.parent_type = static_cast<int8_t>(gpl->partype);
new_layer.blend_mode = static_cast<int8_t>(gpl->blend_mode);
new_layer.thickness_adjustment = static_cast<int16_t>(gpl->line_change);
/* Copy the pointer to the parent. */
new_layer.parent = gpl->parent;
size_t parsubstr_len = BLI_strnlen(gpl->parsubstr, MAX_ID_NAME - 2);
if (parsubstr_len > 0) {
new_layer.parsubstr = BLI_strdupn(gpl->parsubstr, parsubstr_len);
}
size_t viewlayername_len = BLI_strnlen(gpl->viewlayername, MAX_ID_NAME - 2);
if (viewlayername_len > 0) {
new_layer.viewlayer_name = BLI_strdupn(gpl->viewlayername, viewlayername_len);
}
new_layer.opacity = gpl->opacity;
copy_v4_v4(new_layer.tint_color, gpl->tintcolor);
copy_v3_v3(new_layer.location, gpl->location);
copy_v3_v3(new_layer.rotation, gpl->rotation);
copy_v3_v3(new_layer.scale, gpl->scale);
LISTBASE_FOREACH (bGPDframe *, gpf, &gpl->frames) {
grease_pencil.drawing_array[i] = reinterpret_cast<GreasePencilDrawingOrReference *>(
MEM_new<GreasePencilDrawing>(__func__));
GreasePencilDrawing &drawing = *reinterpret_cast<GreasePencilDrawing *>(
grease_pencil.drawing_array[i]);
drawing.base.type = GREASE_PENCIL_DRAWING;
drawing.base.type = GP_DRAWING;
drawing.runtime = MEM_new<bke::GreasePencilDrawingRuntime>(__func__);
/* TODO: copy flag. */
/* TODO: set flag. */
/* Convert the frame to a drawing. */
legacy_gpencil_frame_to_grease_pencil_drawing(drawing, *gpf);

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

@ -38,6 +38,7 @@ extern "C" {
struct GreasePencil;
struct BlendDataReader;
struct BlendWriter;
struct Object;
typedef enum GreasePencilStrokeCapType {
GP_STROKE_CAP_TYPE_ROUND = 0,
@ -47,8 +48,8 @@ typedef enum GreasePencilStrokeCapType {
} GreasePencilStrokeCapType;
typedef enum GreasePencilDrawingType {
GREASE_PENCIL_DRAWING = 0,
GREASE_PENCIL_DRAWING_REFERENCE = 1,
GP_DRAWING = 0,
GP_DRAWING_REFERENCE = 1,
} GreasePencilDrawingType;
typedef struct GreasePencilDrawingOrReference {
@ -56,7 +57,7 @@ typedef struct GreasePencilDrawingOrReference {
* One of `GreasePencilDrawingType`.
* Indicates if this is an actual drawing or a drawing referenced from another object.
*/
uint8_t type;
int8_t type;
char _pad[3];
/**
* Flag. Used to set e.g. the selection status.
@ -120,7 +121,7 @@ typedef struct GreasePencilFrame {
/**
* Keyframe type.
*/
uint8_t type;
int8_t type;
char _pad[3];
} GreasePencilFrame;
@ -138,25 +139,97 @@ typedef struct GreasePencilLayerFramesMapStorage {
char _pad[4];
} GreasePencilLayerFramesMapStorage;
/**
* Type of parent of a layer. #GreasePencilLayer.parent_type
*/
typedef enum GreasePencilLayerParentType {
GP_LAYER_PARENT_OBJECT = 0,
GP_LAYER_PARENT_BONE = 1,
} GreasePencilLayerParentType;
/**
* Layer blending modes. #GreasePencilLayer.blend_mode
*/
typedef enum GreasePencilLayerBlendMode {
GP_LAYER_BLEND_NONE = 0,
GP_LAYER_BLEND_HARDLIGHT = 1,
GP_LAYER_BLEND_ADD = 2,
GP_LAYER_BLEND_SUBTRACT = 3,
GP_LAYER_BLEND_MULTIPLY = 4,
GP_LAYER_BLEND_DIVIDE = 5,
} GreasePencilLayerBlendMode;
/**
* A grease pencil layer is a collection of drawings mapped to a specific time on the timeline.
*/
typedef struct GreasePencilLayer {
/* Only used for storage in the .blend file. */
GreasePencilLayerFramesMapStorage frames_storage;
/**
* Layer parent type. See `GreasePencilLayerParentType`.
*/
int8_t parent_type;
/**
* Layer blend mode. See `GreasePencilLayerBlendMode`.
*/
int8_t blend_mode;
/**
* Thickness adjustment of all strokes in this layer.
*/
int16_t thickness_adjustment;
char _pad[4];
/**
* Parent object. Can be nullptr.
*/
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.
struct Object *parent;
/**
* Parent sub-object info. E.g. the name of a bone if the parent is an armature.
* \note Null-terminated.
*/
char *parsubstr;
/**
* Name of a view layer. If used, the layer is only rendered on the specified view layer. Not
* used for viewport rendering.
* \note Null-terminated.
*/
char *viewlayer_name;
/**
* Opacity of the layer.
*/
float opacity;
/**
* Color used to tint the layer. Alpha value is used as a factor.
*/
float tint_color[4];
/**
* Layer transfrom.
* \note rotation is in euler format.
*/
float location[3], rotation[3], scale[3];
} GreasePencilLayer;
typedef enum GreasePencilLayerTreeNodeType {
GREASE_PENCIL_LAYER_TREE_LEAF = 0,
GREASE_PENCIL_LAYER_TREE_GROUP = 1,
GP_LAYER_TREE_LEAF = 0,
GP_LAYER_TREE_GROUP = 1,
} GreasePencilLayerTreeNodeType;
/**
* Flags for layer tree nodes. #GreasePencilLayerTreeNode.flag
*/
typedef enum GreasePencilLayerTreeNodeFlag {
GP_LAYER_TREE_NODE_HIDE = (1 << 0),
GP_LAYER_TREE_NODE_LOCKED = (1 << 1),
GP_LAYER_TREE_NODE_SELECT = (1 << 2),
GP_LAYER_TREE_NODE_MUTE = (1 << 3),
GP_LAYER_TREE_NODE_USE_LIGHTS = (1 << 4),
} GreasePencilLayerTreeNodeFlag;
typedef struct GreasePencilLayerTreeNode {
/**
* One of `GreasePencilLayerTreeNodeType`.
* Indicates the type of struct this element is.
*/
uint8_t type;
int8_t type;
/**
* Color tag.
@ -165,6 +238,7 @@ typedef struct GreasePencilLayerTreeNode {
/**
* Flag. Used to set e.g. the selection, visibility, ... status.
* See `GreasePencilLayerTreeNodeFlag`.
*/
uint32_t flag;