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
4 changed files with 27 additions and 13 deletions
Showing only changes of commit 47090c658d - Show all commits

View File

@ -25,6 +25,12 @@ namespace gpencil {
class LayerGroup;
class Layer;
/**
* A TreeNode represents one node in the layer tree.
* It can either be a layer or a group. The node has zero children if it is a layer or zero or more
* children if it is a group.
* This class is mainly used for iteration over the layer tree.
*/
class TreeNode : public ::GreasePencilLayerTreeNode {
using TreeNodeIterFn = FunctionRef<void(TreeNode &)>;
using LayerIterFn = FunctionRef<void(Layer &)>;
@ -210,6 +216,10 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
}
};
/**
* A layer maps drawings to scene frames. It can be thought of as one independent channel in the
* timeline.
*/

Returning an element added to a vector by reference is quite dangerous since they're invalidated by further additions, we usually don't do that. Either returning an index or not having these wrapper functions seems like the best choices IMO

Returning an element added to a vector by reference is quite dangerous since they're invalidated by further additions, we usually don't do that. Either returning an index or not having these wrapper functions seems like the best choices IMO

From quick look it seems that it should actually be emplace_layer() or something like this.
And for that it is typically very handy to return reference. And not only that, since C++17 it is expected behavior for vector type containers as well.

From quick look it seems that it should actually be `emplace_layer()` or something like this. And for that it is typically very handy to return reference. And not only that, since C++17 it is expected behavior for vector type containers as well.

I think emplace_layer would be a better name indeed.

I think `emplace_layer` would be a better name indeed.
class Layer : public TreeNode, ::GreasePencilLayer {
private:
/**
@ -268,6 +278,7 @@ class Layer : public TreeNode, ::GreasePencilLayer {
Map<int, int> &frames_for_write()
{
sorted_keys_cache_.tag_dirty();
return frames_;
}
@ -325,6 +336,9 @@ class Layer : public TreeNode, ::GreasePencilLayer {
}
};
/**
* A LayerGroup is a grouping of zero or more Layers.
*/
class LayerGroup : public TreeNode {
public:
LayerGroup() : TreeNode(GREASE_PENCIL_LAYER_TREE_GROUP) {}
@ -389,7 +403,7 @@ class LayerGroup : public TreeNode {
return children_[index].get()->as_layer();
}
int num_children()
int num_children() const
{
return children_.size();
}

View File

@ -41,7 +41,7 @@ static void grease_pencil_init_data(ID *id)
{
using namespace blender::bke;
printf("grease_pencil_init_data\n");
// printf("grease_pencil_init_data\n");
filedescriptor marked this conversation as resolved Outdated

Remove printfs.

Remove printfs.
GreasePencil *grease_pencil = (GreasePencil *)id;
grease_pencil->runtime = MEM_new<GreasePencilRuntime>(__func__);
}
@ -53,7 +53,7 @@ static void grease_pencil_copy_data(Main * /*bmain*/,
{
using namespace blender;
printf("grease_pencil_copy_data\n");
// printf("grease_pencil_copy_data\n");

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

C++ cast here (and above, I'll stop writing it now)
GreasePencil *grease_pencil_dst = (GreasePencil *)id_dst;

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.
const GreasePencil *grease_pencil_src = (GreasePencil *)id_src;
@ -107,7 +107,7 @@ static void grease_pencil_copy_data(Main * /*bmain*/,
static void grease_pencil_free_data(ID *id)
{
printf("grease_pencil_free_data\n");
// printf("grease_pencil_free_data\n");
GreasePencil *grease_pencil = (GreasePencil *)id;
BKE_animdata_free(&grease_pencil->id, false);
@ -773,8 +773,8 @@ static void read_layer_node_recursive(blender::bke::gpencil::LayerGroup &current
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.frames_for_write().add(node_leaf->layer.frames_storage.keys[i],
node_leaf->layer.frames_storage.values[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));
break;

View File

@ -63,11 +63,11 @@ TEST(gpencil, remove_drawing)
Layer layer1("Layer1");
Layer layer2("Layer2");
layer1.frames_for_write().add(0, 0);
layer1.frames_for_write().add(10, 1);
layer1.frames_for_write().add(20, 2);
layer1.insert_frame(0, 0);
layer1.insert_frame(10, 1);
layer1.insert_frame(20, 2);
layer2.frames_for_write().add(0, 1);
layer2.insert_frame(0, 1);
grease_pencil.root_group().add_layer(std::move(layer1));
grease_pencil.root_group().add_layer(std::move(layer2));

View File

@ -162,8 +162,6 @@ typedef struct GreasePencilLayerTreeStorage {
/**
* The grease pencil data-block.
* It holds a set of grease pencil drawings, a tree of layer groups, and a set of layers whithin
* each layer group.
*/
typedef struct GreasePencil {
ID id;
@ -171,7 +169,9 @@ typedef struct GreasePencil {
struct AnimData *adt;
/**
* An array of GreasePencilDrawing's.
* An array of pointers to drawings. The drawing can own it's data or reference it from another
* data-block. Note that the order of this array is arbitrary. The mapping of drawings to frames
* is done by the layers. See the `Layer` class in `BKE_grease_pencil.hh`.
*/
GreasePencilDrawingOrReference **drawing_array;
int drawing_array_size;