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
3 changed files with 36 additions and 50 deletions
Showing only changes of commit a97d624e95 - Show all commits

View File

@ -26,7 +26,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
using ItemIterFn = FunctionRef<void(TreeNode &)>;
protected:
Vector<TreeNode, 0> children_;
Vector<std::unique_ptr<TreeNode>> children_;
public:
TreeNode(GreasePencilLayerTreeNodeType type)
@ -48,24 +48,8 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
this->name = other.name;
other.name = nullptr;
}

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?

The const in the const bool return type means nothing here

The `const` in the `const bool` return type means nothing here
TreeNode &operator=(const TreeNode &other)
{
if (this != &other) {
if (this->name) {
MEM_freeN(this->name);
}
this->name = BLI_strdup(other.name);
}
return *this;
}
TreeNode &operator=(TreeNode &&other)
{
if (this != &other) {
this->name = other.name;
other.name = nullptr;
}
return *this;
}
TreeNode &operator=(const TreeNode &other) = delete;
TreeNode &operator=(TreeNode &&other) = delete;
~TreeNode()
{
if (this->name) {
@ -86,7 +70,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
{
if (root != nullptr) {
for (auto it = root->children_.rbegin(); it != root->children_.rend(); it++) {
next_node_.push(&(*it));
next_node_.push((*it).get());

Try putting this class out of line.

Try putting this class out of line.
}
}
}
@ -106,7 +90,7 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
BLI_assert(!next_node_.is_empty());
TreeNode &next_node = *next_node_.pop();
for (auto it = next_node.children_.rbegin(); it != next_node.children_.rend(); it++) {
next_node_.push(&(*it));
next_node_.push((*it).get());
}
return *this;
}
@ -155,22 +139,16 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
public:
constexpr bool is_group() const
{
return type == GREASE_PENCIL_LAYER_TREE_GROUP;
return this->type == GREASE_PENCIL_LAYER_TREE_GROUP;
}
constexpr bool is_layer() const
{
return type == GREASE_PENCIL_LAYER_TREE_LEAF;
return this->type == GREASE_PENCIL_LAYER_TREE_LEAF;
}
LayerGroup &as_group()
{
return *static_cast<LayerGroup *>(this);
}
Layer &as_layer()
{
return *static_cast<Layer *>(this);
}
LayerGroup &as_group();
Layer &as_layer();
PreOrderRange children_in_pre_order()
{
@ -179,15 +157,15 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
void foreach_children_pre_order(ItemIterFn function)
{
for (TreeNode &child : children_) {
child.foreach_children_pre_order_recursive_(function);
for (auto &child : children_) {
child->foreach_children_pre_order_recursive_(function);
}
}

Put out of line.

Put out of line.
void foreach_leaf_pre_order(ItemIterFn function)
{

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.
for (TreeNode &child : children_) {
child.foreach_leaf_pre_order_recursive_(function);
for (auto &child : children_) {
child->foreach_leaf_pre_order_recursive_(function);
}
}
@ -195,8 +173,8 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
void foreach_children_pre_order_recursive_(ItemIterFn function)
{
function(*this);
for (TreeNode &child : children_) {
child.foreach_children_pre_order_recursive_(function);
for (auto &child : children_) {
child->foreach_children_pre_order_recursive_(function);
}
}
@ -205,8 +183,8 @@ class TreeNode : public ::GreasePencilLayerTreeNode {
if (children_.size() == 0) {
function(*this);
}
for (TreeNode &child : children_) {
child.foreach_children_pre_order_recursive_(function);
for (auto &child : children_) {
child->foreach_children_pre_order_recursive_(function);
}
}
};
@ -287,22 +265,22 @@ class LayerGroup : public TreeNode {
void add_group(LayerGroup &group)
{
children_.append(group);
children_.append(std::make_unique<LayerGroup>(group));
}
void add_group(LayerGroup &&group)
{
children_.append(std::move(group));
children_.append(std::make_unique<LayerGroup>(group));
}
void add_layer(Layer &layer)

Not sure why the greasepencil namespace ends up here? Also means that e.g. the fairly generically-named StrokePoint struct is in the fairly generic blender::bke namespace, which does not sounds great to me? not to mention 'C-style' namespace in names like GreasePencilDrawingRuntime.

Not sure why the `greasepencil` namespace ends up here? Also means that e.g. the fairly generically-named `StrokePoint` struct is in the fairly generic `blender::bke` namespace, which does not sounds great to me? not to mention 'C-style' namespace in names like `GreasePencilDrawingRuntime`.
{
children_.append(layer);
children_.append(std::make_unique<Layer>(layer));
}
void add_layer(Layer &&layer)
{
children_.append(std::move(layer));
children_.append(std::make_unique<Layer>(layer));
}
int num_children()

View File

@ -267,6 +267,20 @@ BoundBox *BKE_grease_pencil_boundbox_get(Object *ob)
return ob->runtime.bb;
}
namespace blender::bke::gpencil {
LayerGroup &TreeNode::as_group()
{
return *static_cast<LayerGroup *>(this);
}
Layer &TreeNode::as_layer()
{
return *static_cast<Layer *>(this);
}
} // namespace blender::bke::gpencil
// Span<GreasePencilDrawingOrReference> blender::bke::GreasePencil::drawings() const
// {
// return Span<GreasePencilDrawingOrReference>{this->drawing_array, this->drawing_array_size};

View File

@ -20,13 +20,7 @@ TEST(gpencil, build_layer_tree)
root.add_group(std::move(group));
root.add_layer(Layer("Group2"));
root.foreach_children_pre_order([](LayerGroup &child) { std::cout << child.name << std::endl; });
// root.remove_child(0);
// for (LayerGroup &child : root.children_in_pre_order()) {
// std::cout << child.name << std::endl;
// }
root.foreach_children_pre_order([](TreeNode &child) { std::cout << child.name << std::endl; });
}
} // namespace blender::bke::gpencil::tests