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 65 additions and 9 deletions
Showing only changes of commit b5912421c0 - Show all commits

View File

@ -292,6 +292,12 @@ class LayerGroup : public ::GreasePencilLayerTreeGroup {
Span<const Layer *> layers() const;
Span<Layer *> layers_for_write();
/**
* Returns a pointer to the layer with \a name. If no such layer was found, returns nullptr.

= {} doesn't change anything here since Vector has a default constructor. It can be removed I think. Same with below

`= {}` doesn't change anything here since `Vector` has a default constructor. It can be removed I think. Same with below
*/

Put out of line.

Put out of line.
const Layer *find_layer_by_name(StringRefNull name) const;
Layer *find_layer_by_name(StringRefNull name);
/**
* Print the nodes. For debugging purposes.
*/

Looks like this should be private maybe? It has a _ suffix.

Looks like this should be private maybe? It has a `_` suffix.

View File

@ -103,6 +103,12 @@ static void grease_pencil_copy_data(Main * /*bmain*/,
new (&grease_pencil_dst->root_group)
bke::greasepencil::LayerGroup(grease_pencil_src->root_group.wrap());
/* Set active layer. */
if (grease_pencil_src->has_active_layer()) {
grease_pencil_dst->active_layer = grease_pencil_dst->find_layer_by_name(
grease_pencil_src->active_layer->wrap().name());
}
/* Duplicate runtime data. */
if (grease_pencil_src->runtime) {
grease_pencil_dst->runtime = MEM_new<bke::GreasePencilRuntime>(__func__,
filedescriptor marked this conversation as resolved Outdated

This cannot be 'just' copied like that, would assign the same batch_cache value to both copies e.g.

In general runtime data should not need to be copied anyway.

This cannot be 'just' copied like that, would assign the same `batch_cache` value to both copies e.g. In general runtime data should not need to be copied anyway.
@ -185,7 +191,6 @@ static void grease_pencil_blend_read_data(BlendDataReader *reader, ID *id)
BLO_read_pointer_array(reader, reinterpret_cast<void **>(&grease_pencil->material_array));
grease_pencil->runtime = MEM_new<blender::bke::GreasePencilRuntime>(__func__);

C++ cast here

C++ cast here
// grease_pencil->load_layer_tree_from_storage();
}
static void grease_pencil_blend_read_lib(BlendLibReader *reader, ID *id)
@ -601,6 +606,26 @@ Span<Layer *> LayerGroup::layers_for_write()
return this->runtime->layer_cache_.as_span();
}
const Layer *LayerGroup::find_layer_by_name(const StringRefNull name) const
{
for (const Layer *layer : this->layers()) {
if (StringRef(layer->name()) == StringRef(name)) {
return layer;
}
}
return nullptr;
}
Layer *LayerGroup::find_layer_by_name(const StringRefNull name)
{
for (Layer *layer : this->layers_for_write()) {
if (StringRef(layer->name()) == StringRef(name)) {
return layer;
}
}
return nullptr;
}
void LayerGroup::print_nodes(StringRefNull header) const
{
std::cout << header << std::endl;
@ -615,10 +640,10 @@ void LayerGroup::print_nodes(StringRefNull header) const
std::cout << " ";
}
if (node->is_layer()) {
std::cout << node->name;
std::cout << StringRefNull(node->name);
}
else if (node->is_group()) {
std::cout << node->name << ": ";
std::cout << StringRefNull(node->name) << ": ";
LISTBASE_FOREACH_BACKWARD (GreasePencilLayerTreeNode *, child_, &node->as_group().children) {
TreeNode *child = reinterpret_cast<TreeNode *>(child_);
next_node.push(std::make_pair(indent + 1, child));
@ -626,6 +651,7 @@ void LayerGroup::print_nodes(StringRefNull header) const
}
std::cout << std::endl;
}
std::cout << std::endl;
}
void LayerGroup::ensure_nodes_cache() const
@ -1054,6 +1080,27 @@ blender::Span<blender::bke::greasepencil::Layer *> GreasePencil::layers_for_writ
return this->root_group.wrap().layers_for_write();
}
blender::bke::greasepencil::Layer &GreasePencil::add_layer(
blender::bke::greasepencil::LayerGroup &group, const blender::StringRefNull name)

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
{
using namespace blender;
StringRefNull checked_name;
/* TODO: Check for name collisions and resolve them. */
return group.add_layer(name);
}
const blender::bke::greasepencil::Layer *GreasePencil::find_layer_by_name(
const blender::StringRefNull name) const
{

Declare all arguments const in definitions

Declare all arguments const in definitions
return this->root_group.wrap().find_layer_by_name(name);
}
blender::bke::greasepencil::Layer *GreasePencil::find_layer_by_name(
const blender::StringRefNull name)

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)`
{
return this->root_group.wrap().find_layer_by_name(name);
}
void GreasePencil::print_layer_tree()
{
using namespace blender::bke::greasepencil;

View File

@ -188,10 +188,11 @@ void legacy_gpencil_to_grease_pencil(Main &bmain, GreasePencil &grease_pencil, b
MEM_cnew_array<GreasePencilDrawing *>(num_drawings, __func__));
int i = 0, layer_idx = 0;
LayerGroup &root_goup = grease_pencil.root_group.wrap();
LayerGroup &root_group = grease_pencil.root_group.wrap();
LISTBASE_FOREACH_INDEX (bGPDlayer *, gpl, &gpd.layers, layer_idx) {
/* Create a new layer. */
Layer &new_layer = root_goup.add_layer(StringRefNull(gpl->info, BLI_strnlen(gpl->info, 128)));
Layer &new_layer = grease_pencil.add_layer(
root_group, StringRefNull(gpl->info, BLI_strnlen(gpl->info, 128)));
/* Flags. */
SET_FLAG_FROM_TEST(new_layer.base.flag, (gpl->flag & GP_LAYER_HIDE), GP_LAYER_TREE_NODE_HIDE);
@ -239,8 +240,6 @@ void legacy_gpencil_to_grease_pencil(Main &bmain, GreasePencil &grease_pencil, b
}
/* TODO: Update drawing user counts. */
new_layer.tag_frames_map_keys_changed();
}
/* Convert the onion skinning settings. */
@ -257,8 +256,6 @@ void legacy_gpencil_to_grease_pencil(Main &bmain, GreasePencil &grease_pencil, b
copy_v3_v3(grease_pencil.onion_skinning_settings.color_before, gpd.gcolor_prev);
copy_v3_v3(grease_pencil.onion_skinning_settings.color_after, gpd.gcolor_next);
// grease_pencil.runtime->set_active_layer_index(active_layer_index);
BKE_id_materials_copy(&bmain, &gpd.id, &grease_pencil.id);
}

View File

@ -442,6 +442,12 @@ typedef struct GreasePencil {
blender::Span<blender::bke::greasepencil::Layer *> layers_for_write();
bool has_active_layer() const;
blender::bke::greasepencil::Layer &add_layer(blender::bke::greasepencil::LayerGroup &group,
blender::StringRefNull name);
const blender::bke::greasepencil::Layer *find_layer_by_name(blender::StringRefNull name) const;
blender::bke::greasepencil::Layer *find_layer_by_name(blender::StringRefNull name);

It probably shouldn't be possible to get a span of non-const pointers from a const GreasePencil:

Span<const GreasePencilDrawingBase *>

It probably shouldn't be possible to get a span of non-const pointers from a const `GreasePencil`: `Span<const GreasePencilDrawingBase *>`
void add_empty_drawings(int add_size);
void remove_drawing(int index);

This sort of API function (dealing with one index at a time) can be a bit dangerous since it can easily lead to quadratic behavior if it's called many times. I wonder if a remove_drawings(IndexMask drawings_to_remove) function would work a bit better

This sort of API function (dealing with one index at a time) can be a bit dangerous since it can easily lead to quadratic behavior if it's called many times. I wonder if a `remove_drawings(IndexMask drawings_to_remove)` function would work a bit better