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
94 changed files with 5945 additions and 53 deletions

View File

@ -382,6 +382,7 @@ def load():
use_alt_click_leader=kc_prefs.use_alt_click_leader,
use_pie_click_drag=kc_prefs.use_pie_click_drag,
use_file_single_click=kc_prefs.use_file_single_click,
experimental=prefs.experimental,
use_transform_navigation=kc_prefs.use_transform_navigation,
),
)

View File

@ -95,6 +95,8 @@ class Params:
# Since this means with RMB select enabled in edit-mode for e.g.
# `Ctrl-LMB` would be caught by box-select instead of add/extrude.
"tool_maybe_tweak_event",
# Access to bpy.context.preferences.experimental
"experimental",
# Changes some transformers modal key-map items to avoid conflicts with navigation operations
"use_transform_navigation",
)
@ -124,6 +126,7 @@ class Params:
use_file_single_click=False,
v3d_tilde_action='VIEW',
v3d_alt_mmb_drag_action='RELATIVE',
experimental=None,
use_transform_navigation=False,
):
from sys import platform
@ -225,6 +228,8 @@ class Params:
self.tool_maybe_tweak_event = {"type": self.tool_mouse, "value": self.tool_maybe_tweak_value}
self.use_transform_navigation = use_transform_navigation
self.experimental = experimental
# ------------------------------------------------------------------------------
# Constants
@ -3830,8 +3835,18 @@ def km_grease_pencil_stroke_paint_draw_brush(params):
{"items": items},
)
items.extend([
# Draw
# Draw
if params.experimental and params.experimental.use_grease_pencil_version3:
items.extend([
("grease_pencil.brush_stroke", {"type": 'LEFTMOUSE', "value": 'PRESS'},
{"properties": [("mode", 'NORMAL')]}),
("grease_pencil.brush_stroke", {"type": 'LEFTMOUSE', "value": 'PRESS', "ctrl": True},
{"properties": [("mode", 'INVERT')]}),
("grease_pencil.brush_stroke", {"type": 'LEFTMOUSE', "value": 'PRESS', "shift": True},
{"properties": [("mode", 'SMOOTH')]}),
])
else:
items.extend([
("gpencil.draw", {"type": 'LEFTMOUSE', "value": 'PRESS'},
{"properties": [("mode", 'DRAW'), ("wait_for_input", False)]}),
("gpencil.draw", {"type": 'LEFTMOUSE', "value": 'PRESS', "shift": True},
@ -3842,6 +3857,9 @@ def km_grease_pencil_stroke_paint_draw_brush(params):
# Erase
("gpencil.draw", {"type": 'LEFTMOUSE', "value": 'PRESS', "ctrl": True},
{"properties": [("mode", 'ERASER'), ("wait_for_input", False)]}),
])
items.extend([
# Constrain Guides Speedlines
# Freehand
("gpencil.draw", {"type": 'O', "value": 'PRESS'}, None),

View File

@ -500,6 +500,8 @@ class GreasePencilMaterialsPanel:
show_full_ui = (self.bl_space_type == 'PROPERTIES')
is_view3d = (self.bl_space_type == 'VIEW_3D')
is_grease_pencil_version3 = context.preferences.experimental.use_grease_pencil_version3
tool_settings = context.scene.tool_settings
gpencil_paint = tool_settings.gpencil_paint
brush = gpencil_paint.brush if gpencil_paint else None
@ -550,7 +552,7 @@ class GreasePencilMaterialsPanel:
icon_link = 'MESH_DATA' if slot.link == 'DATA' else 'OBJECT_DATA'
row.prop(slot, "link", icon=icon_link, icon_only=True)
if ob.data.use_stroke_edit_mode:
if not is_grease_pencil_version3 and ob.data.use_stroke_edit_mode:
row = layout.row(align=True)
row.operator("gpencil.stroke_change_color", text="Assign")
row.operator("gpencil.material_select", text="Select").deselect = False

View File

@ -1970,6 +1970,13 @@ class _defs_gpencil_paint:
@staticmethod
def generate_from_brushes(context):
if context and context.preferences.experimental.use_grease_pencil_version3:
return tuple([ToolDef.from_dict(dict(
idname="builtin_brush.draw",
label="Draw",
icon="brush.gpencil_draw.draw",
data_block='DRAW',
))])
return generate_from_enum_ex(
context,
idname_prefix="builtin_brush.",

View File

@ -2398,6 +2398,7 @@ class USERPREF_PT_experimental_prototypes(ExperimentalPanel, Panel):
({"property": "use_full_frame_compositor"}, ("blender/blender/issues/88150", "#88150")),
({"property": "enable_eevee_next"}, ("blender/blender/issues/93220", "#93220")),
({"property": "enable_workbench_next"}, ("blender/blender/issues/101619", "#101619")),
({"property": "use_grease_pencil_version3"}, ("blender/blender/projects/40", "Grease Pencil 3.0")),
({"property": "enable_overlay_next"}, ("blender/blender/issues/102179", "#102179")),
),
)

View File

@ -1981,7 +1981,7 @@ class VIEW3D_MT_paint_gpencil(Menu):
layout.operator("gpencil.vertex_color_brightness_contrast", text="Brightness/Contrast")
class VIEW3D_MT_select_gpencil(Menu):
class VIEW3D_MT_select_edit_gpencil(Menu):
bl_label = "Select"
def draw(self, context):
@ -5414,6 +5414,13 @@ class VIEW3D_MT_edit_gpencil_showhide(Menu):
layout.operator("gpencil.hide", text="Hide Inactive Layers").unselected = True
class VIEW3D_MT_edit_greasepencil(Menu):
bl_label = "Grease Pencil"
def draw(self, _context):
pass
class VIEW3D_MT_edit_curves(Menu):
bl_label = "Curves"
@ -8163,7 +8170,7 @@ classes = (
VIEW3D_MT_edit_lattice_context_menu,
VIEW3D_MT_select_edit_lattice,
VIEW3D_MT_select_edit_armature,
VIEW3D_MT_select_gpencil,
VIEW3D_MT_select_edit_gpencil,
VIEW3D_MT_select_paint_mask,
VIEW3D_MT_select_paint_mask_vertex,
VIEW3D_MT_edit_curves_select_more_less,
@ -8267,6 +8274,7 @@ classes = (
VIEW3D_MT_gpencil_simplify,
VIEW3D_MT_gpencil_autoweights,
VIEW3D_MT_gpencil_edit_context_menu,
VIEW3D_MT_edit_greasepencil,
VIEW3D_MT_edit_curve,
VIEW3D_MT_edit_curve_ctrlpoints,
VIEW3D_MT_edit_curve_segments,

View File

@ -32,6 +32,7 @@ set(SRC_DNA_INC
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_gpencil_legacy_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_gpencil_modifier_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_gpu_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_grease_pencil_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_image_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_ipo_types.h
${CMAKE_CURRENT_SOURCE_DIR}/makesdna/DNA_key_types.h

View File

@ -0,0 +1,29 @@
/* SPDX-License-Identifier: GPL-2.0-or-later
filedescriptor marked this conversation as resolved Outdated

Needs copyright.

Needs copyright.
* Copyright 2023 Blender Foundation. */
#pragma once
#include "DNA_grease_pencil_types.h"
/** \file
* \ingroup bke
* \brief Low-level operations for grease pencil that cannot be defined in the C++ header yet.
*/
#ifdef __cplusplus
extern "C" {
#endif
enum {
BKE_GREASEPENCIL_BATCH_DIRTY_ALL = 0,
};
extern void (*BKE_grease_pencil_batch_cache_dirty_tag_cb)(GreasePencil *grease_pencil, int mode);
extern void (*BKE_grease_pencil_batch_cache_free_cb)(GreasePencil *grease_pencil);
void BKE_grease_pencil_batch_cache_dirty_tag(GreasePencil *grease_pencil, int mode);
void BKE_grease_pencil_batch_cache_free(GreasePencil *grease_pencil);
#ifdef __cplusplus
}
#endif

View File

@ -0,0 +1,381 @@
/* SPDX-License-Identifier: GPL-2.0-or-later
filedescriptor marked this conversation as resolved Outdated

Needs copyright.

Needs copyright.
* Copyright 2023 Blender Foundation. */
#pragma once
/** \file
* \ingroup bke
* \brief Low-level operations for grease pencil.
*/
#include "BLI_function_ref.hh"
#include "BLI_map.hh"
#include "BLI_math_vector_types.hh"
#include "BLI_shared_cache.hh"
#include "BLI_utility_mixins.hh"
#include "DNA_gpencil_legacy_types.h"
#include "DNA_grease_pencil_types.h"
namespace blender::bke {
namespace greasepencil {
filedescriptor marked this conversation as resolved Outdated

Decide on the namespace here. Should it maybe be just gp?

Decide on the namespace here. Should it maybe be just `gp`?
/**
* A single point for a stroke that is currently being drawn.
*/
struct StrokePoint {
float3 position;
float radius;
float opacity;
float4 color;
};
/**
* Stroke cache for a stroke that is currently being drawn.
*/
struct StrokeCache {
Vector<StrokePoint> points;
Vector<uint3> triangles;

Is there a particular reason you remove copy assignment but keep copy construction defined?

Is there a particular reason you remove copy assignment but keep copy construction defined?

Just because copying should be explicit. And removing the copy assignment constructor avoids errors.

Just because copying should be explicit. And removing the copy assignment constructor avoids errors.
int mat = 0;
void clear()
{
this->points.clear_and_shrink();
this->triangles.clear_and_shrink();
this->mat = 0;
}
};
class DrawingRuntime {

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
public:
/**
* Triangle cache for all the strokes in the drawing.
*/
mutable SharedCache<Vector<uint3>> triangles_cache;
StrokeCache stroke_cache;
};
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.
*/
class TreeNode : public ::GreasePencilLayerTreeNode {
public:
TreeNode();
explicit TreeNode(GreasePencilLayerTreeNodeType type);
explicit TreeNode(GreasePencilLayerTreeNodeType type, StringRefNull name);

Try putting this class out of line.

Try putting this class out of line.
TreeNode(const TreeNode &other);
public:
/**
* \returns true if this node is a LayerGroup.
*/
bool is_group() const
{
return this->type == GP_LAYER_TREE_GROUP;
}
/**
Review

The _for_write() suffix doesn't seem super helpful here TBH, but I guess it doesn't hurt.

The `_for_write()` suffix doesn't seem super helpful here TBH, but I guess it doesn't hurt.
* \returns true if this node is a Layer.
*/
bool is_layer() const
{
return this->type == GP_LAYER_TREE_LEAF;
}
/**
* \returns this tree node as a LayerGroup.
* \note This results in undefined behavior if the node is not a LayerGroup.
*/
const LayerGroup &as_group() const;
/**
* \returns this tree node as a Layer.
* \note This results in undefined behavior if the node is not a Layer.
*/
const Layer &as_layer() const;
/**
* \returns this tree node as a mutable LayerGroup.
* \note This results in undefined behavior if the node is not a LayerGroup.
*/
LayerGroup &as_group_for_write();
/**
* \returns this tree node as a mutable Layer.
* \note This results in undefined behavior if the node is not a Layer.
*/
Layer &as_layer_for_write();
};

exclusive

`exclusive`
/**
* A layer mask stores a reference to a layer that will mask other layers.
*/

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).

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.

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.
class LayerMask : public ::GreasePencilLayerMask {
public:
LayerMask();
explicit LayerMask(StringRefNull name);
LayerMask(const LayerMask &other);
~LayerMask();
};
class LayerRuntime {
public:
/**
* This Map maps a scene frame number (key) to a GreasePencilFrame. This struct holds an index
* (drawing_index) to the drawing in the GreasePencil->drawings array. The frame number indicates
* the first frame the drawing is shown. The end time is implicitly defined by the next greater
* frame number (key) in the map. If the value mapped to (index) is -1, no drawing is shown at
* this frame.
*
* \example:
*
* {0: 0, 5: 1, 10: -1, 12: 2, 16: -1}
*
* In this example there are three drawings (drawing #0, drawing #1 and drawing #2). The first
* drawing starts at frame 0 and ends at frame 5 (exclusive). The second drawing starts at
* frame 5 and ends at frame 10. Finally, the third drawing starts at frame 12 and ends at
* frame 16.
*
* | | | | | | | | | | |1|1|1|1|1|1|1|
* Scene Frame: |0|1|2|3|4|5|6|7|8|9|0|1|2|3|4|5|6|...
* Drawing: [#0 ][#1 ] [#2 ]
*
* \note If a drawing references another data-block, all of the drawings in that data-block are
* mapped sequentially to the frames (frame-by-frame). If another frame starts, the rest of the
* referenced drawings are discarded. If the frame is longer than the number of referenced
* drawings, then the last referenced drawing is held for the rest of the duration.
*/
Map<int, GreasePencilFrame> frames_;

Better to return a span here maybe?

Better to return a span here maybe?
/**
* Caches a sorted vector of the keys of `frames_`.
*/
mutable SharedCache<Vector<int>> sorted_keys_cache_;
/**
* A vector of LayerMask. This layer will be masked by the layers referenced in the masks.
* A layer can have zero or more layer masks.

Put out of line.

Put out of line.
*/
Vector<LayerMask> masks_;
};

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.
/**
* A layer maps drawings to scene frames. It can be thought of as one independent channel in the
* timeline.
*/
class Layer : public ::GreasePencilLayer {
public:
Layer();
explicit Layer(StringRefNull name);
Layer(const Layer &other);
~Layer();
StringRefNull name() const
{
return this->base.name;
}
/**
* \returns the frames mapping.
*/
const Map<int, GreasePencilFrame> &frames() const;
Map<int, GreasePencilFrame> &frames_for_write();
bool is_visible() const;
bool is_locked() const;
/**
filedescriptor marked this conversation as resolved Outdated

Would be good to know why this is commented out, or it should be cleaned up before merge in main.

Would be good to know why this is commented out, or it should be cleaned up before merge in main.
* Inserts the frame into the layer. Fails if there exists a frame at \a frame_number already.
* \returns true on success.
*/
bool insert_frame(int frame_number, const GreasePencilFrame &frame);
bool insert_frame(int frame_number, GreasePencilFrame &&frame);
/**

Put out of line.

Put out of line.
* Inserts the frame into the layer. If there exists a frame at \a frame_number already, it is
* overwritten.
* \returns true on success.
*/
bool overwrite_frame(int frame_number, const GreasePencilFrame &frame);
bool overwrite_frame(int frame_number, GreasePencilFrame &&frame);
/**

Put out of line.

Put out of line.
* Returns the sorted (start) frame numbers of the frames of this layer.
* \note This will cache the keys lazily.

Not sure what is a 'pre-order vector'? or is a typo? Like pre-ordered vector? Same below.

Not sure what is a 'pre-order vector'? or is a typo? Like `pre-ordered vector`? Same below.
*/
Span<int> sorted_keys() const;
/**
* \returns the index of the drawing at frame \a frame or -1 if there is no drawing.
*/
int drawing_index_at(const int frame) const;
void tag_frames_map_changed();
/**
* Should be called whenever the keys in the frames map have changed. E.g. when new keys were

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.
* added, removed or updated.
*/
void tag_frames_map_keys_changed();
};
class LayerGroupRuntime {
public:
/**
* CacheMutex for `nodes_cache_` and `layer_cache_`;
*/
mutable CacheMutex nodes_cache_mutex_;
/**
* Caches all the nodes of this group in a single pre-ordered vector.
*/
mutable Vector<TreeNode *> nodes_cache_;
/**
* Caches all the layers in this group in a single pre-ordered vector.
*/
mutable Vector<Layer *> layer_cache_;
};
/**
* A LayerGroup is a grouping of zero or more Layers.
*/
class LayerGroup : public ::GreasePencilLayerTreeGroup {
public:
LayerGroup();
filedescriptor marked this conversation as resolved Outdated

Since the map will have to change to store more than just an index as the value, make sure to update the comment too.

Since the map will have to change to store more than just an index as the value, make sure to update the comment too.
explicit LayerGroup(StringRefNull name);
LayerGroup(const LayerGroup &other);
~LayerGroup();
public:
/**
* Adds a group at the end of this group.
*/
LayerGroup &add_group(LayerGroup *group);
LayerGroup &add_group(StringRefNull name);
/**
* Adds a layer at the end of this group and returns it.
*/
Layer &add_layer(Layer *layer);
Layer &add_layer(StringRefNull name);
/**
* Returns the number of direct nodes in this group.
*/
int64_t num_direct_nodes() const;
/**
* Returns the total number of nodes in this group.
*/
int64_t num_nodes_total() const;

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`.
/**
* Removes a child from the group by index. Does not free the memory.
* \note: Assumes the removed child is not the active layer.
*/
void remove_child(int64_t index);
/**
* Returns a `Span` of pointers to all the `TreeNode`s in this group.
*/
Span<const TreeNode *> nodes() const;
Span<TreeNode *> nodes_for_write();
/**
* Returns a `Span` of pointers to all the `Layer`s in this group.
*/
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.
void print_nodes(StringRefNull header) const;
private:
void ensure_nodes_cache() const;
void tag_nodes_cache_dirty() const;
};

Put out of line.

Put out of line.
namespace convert {
void legacy_gpencil_frame_to_grease_pencil_drawing(const bGPDframe &gpf,
GreasePencilDrawing &r_drawing);
void legacy_gpencil_to_grease_pencil(Main &main, GreasePencil &grease_pencil, bGPdata &gpd);
} // namespace convert
} // namespace greasepencil

Put out of line.

Put out of line.
class GreasePencilRuntime {
public:
/**
filedescriptor marked this conversation as resolved Outdated

Same remark as above about commented out code.

Same remark as above about commented out code.
* Allocated and freed by the drawing code. See `DRW_grease_pencil_batch_cache_*` functions.
*/
void *batch_cache = nullptr;

These two functions should never have to be called outside of the ID callbacks in grease_pencil.cc; IMO they make more sense as static functions there. Probably better to keep that storage for DNA thing as localized as possible.

These two functions should never have to be called outside of the `ID` callbacks in `grease_pencil.cc`; IMO they make more sense as static functions there. Probably better to keep that storage for DNA thing as localized as possible.
public:
GreasePencilRuntime() {}
~GreasePencilRuntime() {}
};

Member variables come before functions https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout

Member variables come before functions https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Class_Layout
} // namespace blender::bke
inline blender::bke::greasepencil::TreeNode &GreasePencilLayerTreeNode::wrap()
{
return *reinterpret_cast<blender::bke::greasepencil::TreeNode *>(this);
}
inline const blender::bke::greasepencil::TreeNode &GreasePencilLayerTreeNode::wrap() const
{
return *reinterpret_cast<const blender::bke::greasepencil::TreeNode *>(this);
}
inline blender::bke::greasepencil::Layer &GreasePencilLayer::wrap()
{
return *reinterpret_cast<blender::bke::greasepencil::Layer *>(this);
}

Put out of line.

Put out of line.
inline const blender::bke::greasepencil::Layer &GreasePencilLayer::wrap() const
{
return *reinterpret_cast<const blender::bke::greasepencil::Layer *>(this);
}
inline blender::bke::greasepencil::LayerGroup &GreasePencilLayerTreeGroup::wrap()
{
return *reinterpret_cast<blender::bke::greasepencil::LayerGroup *>(this);
}
inline const blender::bke::greasepencil::LayerGroup &GreasePencilLayerTreeGroup::wrap() const
{
return *reinterpret_cast<const blender::bke::greasepencil::LayerGroup *>(this);

This needs to be implemented.

This needs to be implemented.
}
inline bool GreasePencil::has_active_layer() const
{
return (this->active_layer != nullptr);
}
struct Main;
struct Depsgraph;
struct BoundBox;
struct Scene;
struct Object;
void *BKE_grease_pencil_add(Main *bmain, const char *name);
GreasePencil *BKE_grease_pencil_new_nomain();
BoundBox *BKE_grease_pencil_boundbox_get(Object *ob);
void BKE_grease_pencil_data_update(struct Depsgraph *depsgraph,
struct Scene *scene,
struct Object *object);
bool BKE_grease_pencil_references_cyclic_check(const GreasePencil *id_reference,
const GreasePencil *grease_pencil);

View File

@ -273,6 +273,7 @@ extern IDTypeInfo IDType_ID_CV;
extern IDTypeInfo IDType_ID_PT;
extern IDTypeInfo IDType_ID_VO;
extern IDTypeInfo IDType_ID_SIM;
extern IDTypeInfo IDType_ID_GP;
/** Empty shell mostly, but needed for read code. */
extern IDTypeInfo IDType_ID_LINK_PLACEHOLDER;

View File

@ -211,6 +211,7 @@ typedef struct Main {
ListBase paintcurves;
ListBase wm; /* Singleton (exception). */
ListBase gpencils; /* Legacy Grease Pencil. */
ListBase grease_pencils;
ListBase movieclips;
ListBase masks;
ListBase linestyles;

View File

@ -149,6 +149,8 @@ set(SRC
intern/gpencil_legacy.c
intern/gpencil_modifier_legacy.c
intern/gpencil_update_cache_legacy.c
intern/grease_pencil_convert_legacy.cc
intern/grease_pencil.cc
intern/icons.cc
intern/icons_rasterize.c
intern/idprop.c
@ -393,6 +395,8 @@ set(SRC
BKE_gpencil_legacy.h
BKE_gpencil_modifier_legacy.h
BKE_gpencil_update_cache_legacy.h
BKE_grease_pencil.h
BKE_grease_pencil.hh
BKE_icons.h
BKE_idprop.h
BKE_idprop.hh
@ -836,6 +840,7 @@ if(WITH_GTESTS)
intern/lib_remap_test.cc
intern/nla_test.cc
intern/tracking_test.cc
intern/grease_pencil_test.cc
)
set(TEST_INC
../editors/include

View File

@ -1184,6 +1184,8 @@ enum eContextObjectMode CTX_data_mode_enum_ex(const Object *obedit,
return CTX_MODE_EDIT_LATTICE;
case OB_CURVES:
return CTX_MODE_EDIT_CURVES;
case OB_GREASE_PENCIL:
return CTX_MODE_EDIT_GPENCIL;
}
}
else {

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,262 @@
/* SPDX-License-Identifier: GPL-2.0-or-later
* Copyright 2023 Blender Foundation. */
/** \file
* \ingroup bke
*/
#include "BKE_curves.hh"
#include "BKE_grease_pencil.hh"
#include "BKE_material.h"
#include "BLI_color.hh"
#include "BLI_listbase.h"
#include "BLI_math_vector_types.hh"
#include "BLI_vector.hh"
#include "DNA_gpencil_legacy_types.h"
#include "DNA_grease_pencil_types.h"
filedescriptor marked this conversation as resolved Outdated

legacy_gpencil_frame_to_grease_pencil_drawing

`legacy_gpencil_frame_to_grease_pencil_drawing`
namespace blender::bke::greasepencil::convert {
void legacy_gpencil_frame_to_grease_pencil_drawing(const bGPDframe &gpf,

Order return argument last, make the gpf argument const

Order return argument last, make the `gpf` argument const
GreasePencilDrawing &r_drawing)
{
/* Construct an empty CurvesGeometry in-place. */
new (&r_drawing.geometry) CurvesGeometry();
r_drawing.base.type = GP_DRAWING;
r_drawing.runtime = MEM_new<bke::greasepencil::DrawingRuntime>(__func__);
/* Get the number of points, number of strokes and the offsets for each stroke. */
Vector<int> offsets;
offsets.append(0);
int num_strokes = 0;
int num_points = 0;
LISTBASE_FOREACH (bGPDstroke *, gps, &gpf.strokes) {
num_points += gps->totpoints;
offsets.append(num_points);
num_strokes++;
}
/* Resize the CurvesGeometry. */
CurvesGeometry &curves = r_drawing.geometry.wrap();
curves.resize(num_points, num_strokes);
if (num_strokes > 0) {
curves.offsets_for_write().copy_from(offsets);
}
OffsetIndices<int> points_by_curve = curves.points_by_curve();
MutableAttributeAccessor attributes = curves.attributes_for_write();
/* All strokes are poly curves. */
curves.fill_curve_types(CURVE_TYPE_POLY);
/* Point Attributes. */
MutableSpan<float3> positions = curves.positions_for_write();
SpanAttributeWriter<float> radii = attributes.lookup_or_add_for_write_span<float>(
"radius", ATTR_DOMAIN_POINT);
SpanAttributeWriter<float> opacities = attributes.lookup_or_add_for_write_span<float>(
"opacity", ATTR_DOMAIN_POINT);
SpanAttributeWriter<float> delta_times = attributes.lookup_or_add_for_write_span<float>(
"delta_time", ATTR_DOMAIN_POINT);
SpanAttributeWriter<float> rotations = attributes.lookup_or_add_for_write_span<float>(
"rotation", ATTR_DOMAIN_POINT);
SpanAttributeWriter<ColorGeometry4f> vertex_colors =
attributes.lookup_or_add_for_write_span<ColorGeometry4f>("vertex_color", ATTR_DOMAIN_POINT);
SpanAttributeWriter<bool> selection = attributes.lookup_or_add_for_write_span<bool>(
".selection", ATTR_DOMAIN_POINT);
/* Curve Attributes. */
SpanAttributeWriter<bool> stroke_cyclic = attributes.lookup_or_add_for_write_span<bool>(
"cyclic", ATTR_DOMAIN_CURVE);
/* TODO: This should be a `double` attribute. */
SpanAttributeWriter<float> stroke_init_times = attributes.lookup_or_add_for_write_span<float>(
"init_time", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<int8_t> stroke_start_caps = attributes.lookup_or_add_for_write_span<int8_t>(
"start_cap", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<int8_t> stroke_end_caps = attributes.lookup_or_add_for_write_span<int8_t>(
"end_cap", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<float> stroke_hardnesses = attributes.lookup_or_add_for_write_span<float>(
"hardness", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<float> stroke_point_aspect_ratios =
attributes.lookup_or_add_for_write_span<float>("point_aspect_ratio", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<float2> stroke_fill_translations =
attributes.lookup_or_add_for_write_span<float2>("fill_translation", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<float> stroke_fill_rotations =
attributes.lookup_or_add_for_write_span<float>("fill_rotation", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<float2> stroke_fill_scales = attributes.lookup_or_add_for_write_span<float2>(
"fill_scale", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<ColorGeometry4f> stroke_fill_colors =
attributes.lookup_or_add_for_write_span<ColorGeometry4f>("fill_color", ATTR_DOMAIN_CURVE);
SpanAttributeWriter<int> stroke_materials = attributes.lookup_or_add_for_write_span<int>(
"material_index", ATTR_DOMAIN_CURVE);
int stroke_i = 0;
LISTBASE_FOREACH_INDEX (bGPDstroke *, gps, &gpf.strokes, stroke_i) {
/* TODO: check if gps->editcurve is not nullptr and parse bezier curve instead. */
/* Write curve attributes. */
stroke_cyclic.span[stroke_i] = (gps->flag & GP_STROKE_CYCLIC) != 0;
/* TODO: This should be a `double` attribute. */
stroke_init_times.span[stroke_i] = static_cast<float>(gps->inittime);
stroke_start_caps.span[stroke_i] = static_cast<int8_t>(gps->caps[0]);
stroke_end_caps.span[stroke_i] = static_cast<int8_t>(gps->caps[1]);
stroke_hardnesses.span[stroke_i] = gps->hardeness;
stroke_point_aspect_ratios.span[stroke_i] = gps->aspect_ratio[0] /
max_ff(gps->aspect_ratio[1], 1e-8);
stroke_fill_translations.span[stroke_i] = float2(gps->uv_translation);
stroke_fill_rotations.span[stroke_i] = gps->uv_rotation;
stroke_fill_scales.span[stroke_i] = float2(gps->uv_scale);
stroke_fill_colors.span[stroke_i] = ColorGeometry4f(gps->vert_color_fill);
stroke_materials.span[stroke_i] = gps->mat_nr;
/* Write point attributes. */
IndexRange stroke_points_range = points_by_curve[stroke_i];
if (stroke_points_range.size() == 0) {
continue;
}
Span<bGPDspoint> stroke_points{gps->points, gps->totpoints};
MutableSpan<float3> stroke_positions = positions.slice(stroke_points_range);
MutableSpan<float> stroke_radii = radii.span.slice(stroke_points_range);
MutableSpan<float> stroke_opacities = opacities.span.slice(stroke_points_range);
MutableSpan<float> stroke_deltatimes = delta_times.span.slice(stroke_points_range);
MutableSpan<float> stroke_rotations = rotations.span.slice(stroke_points_range);
MutableSpan<ColorGeometry4f> stroke_vertex_colors = vertex_colors.span.slice(
stroke_points_range);
MutableSpan<bool> stroke_selections = selection.span.slice(stroke_points_range);
/* Do first point. */
const bGPDspoint &first_pt = stroke_points.first();
stroke_positions.first() = float3(first_pt.x, first_pt.y, first_pt.z);
/* Store the actual radius of the stroke (without layer adjustment). */
stroke_radii.first() = gps->thickness * first_pt.pressure;
stroke_opacities.first() = first_pt.strength;
stroke_deltatimes.first() = 0;
stroke_rotations.first() = first_pt.uv_rot;
stroke_vertex_colors.first() = ColorGeometry4f(first_pt.vert_color);
stroke_selections.first() = (first_pt.flag & GP_SPOINT_SELECT) != 0;
/* Do the rest of the points. */
for (const int i : stroke_points.index_range().drop_back(1)) {
const int point_i = i + 1;
const bGPDspoint &pt_prev = stroke_points[point_i - 1];
const bGPDspoint &pt = stroke_points[point_i];
stroke_positions[point_i] = float3(pt.x, pt.y, pt.z);
/* Store the actual radius of the stroke (without layer adjustment). */
stroke_radii[point_i] = gps->thickness * pt.pressure;
stroke_opacities[point_i] = pt.strength;
stroke_deltatimes[point_i] = pt.time - pt_prev.time;
stroke_rotations[point_i] = pt.uv_rot;
stroke_vertex_colors[point_i] = ColorGeometry4f(pt.vert_color);
stroke_selections[point_i] = (pt.flag & GP_SPOINT_SELECT) != 0;
}
}

I thought we had some RAII helper which does finish() at the end of the scope.
Or maybe it was just an idea to have one? Maybe Jacques of Hans remember better :)

I thought we had some RAII helper which does ` finish()` at the end of the scope. Or maybe it was just an idea to have one? Maybe Jacques of Hans remember better :)

Right now regular we do this manually, but true, this could be better to use BLI_SCOPED_DEFER.

Right now regular we do this manually, but true, this could be better to use `BLI_SCOPED_DEFER`.

We don't really want to do non-trivial work in a class's destructor. It's also helpful to make this explicit, since sometimes it matters when it happens.

We don't really want to do non-trivial work in a class's destructor. It's also helpful to make this explicit, since sometimes it matters when it happens.

I don't really understand it, and can't say I'm sold on the motivation.
But in any case, at this point the discussion does not affect the PR, so lets consider it resolved here, and any followup we can do in the chat instead.

I don't really understand it, and can't say I'm sold on the motivation. But in any case, at this point the discussion does not affect the PR, so lets consider it resolved here, and any followup we can do in the chat instead.
radii.finish();
opacities.finish();
delta_times.finish();
rotations.finish();
vertex_colors.finish();
selection.finish();
stroke_cyclic.finish();
stroke_init_times.finish();
stroke_start_caps.finish();
stroke_end_caps.finish();
stroke_hardnesses.finish();
stroke_point_aspect_ratios.finish();
stroke_fill_translations.finish();
stroke_fill_rotations.finish();
stroke_fill_scales.finish();
stroke_fill_colors.finish();
stroke_materials.finish();
}
void legacy_gpencil_to_grease_pencil(Main &bmain, GreasePencil &grease_pencil, bGPdata &gpd)
{
using namespace blender::bke::greasepencil;
int num_layers = 0;
int num_drawings = 0;
LISTBASE_FOREACH (bGPDlayer *, gpl, &gpd.layers) {
num_drawings += BLI_listbase_count(&gpl->frames);
num_layers++;
}
grease_pencil.drawing_array_size = num_drawings;
grease_pencil.drawing_array = reinterpret_cast<GreasePencilDrawingBase **>(
MEM_cnew_array<GreasePencilDrawing *>(num_drawings, __func__));
int i = 0, layer_idx = 0;
LayerGroup &root_group = grease_pencil.root_group.wrap();
LISTBASE_FOREACH_INDEX (bGPDlayer *, gpl, &gpd.layers, layer_idx) {
/* Create a new layer. */
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);
SET_FLAG_FROM_TEST(
new_layer.base.flag, (gpl->flag & GP_LAYER_LOCKED), GP_LAYER_TREE_NODE_LOCKED);
SET_FLAG_FROM_TEST(
new_layer.base.flag, (gpl->flag & GP_LAYER_SELECT), GP_LAYER_TREE_NODE_SELECT);
SET_FLAG_FROM_TEST(
new_layer.base.flag, (gpl->flag & GP_LAYER_FRAMELOCK), GP_LAYER_TREE_NODE_MUTE);
SET_FLAG_FROM_TEST(
new_layer.base.flag, (gpl->flag & GP_LAYER_USE_LIGHTS), GP_LAYER_TREE_NODE_USE_LIGHTS);
SET_FLAG_FROM_TEST(new_layer.base.flag,
(gpl->onion_flag & GP_LAYER_ONIONSKIN),
GP_LAYER_TREE_NODE_USE_ONION_SKINNING);
new_layer.blend_mode = static_cast<int8_t>(gpl->blend_mode);
/* Convert the layer masks. */
LISTBASE_FOREACH (bGPDlayer_Mask *, mask, &gpl->mask_layers) {
LayerMask *new_mask = new LayerMask(mask->name);
new_mask->flag = mask->flag;
BLI_addtail(&new_layer.masks, new_mask);
}
new_layer.opacity = gpl->opacity;
LISTBASE_FOREACH (bGPDframe *, gpf, &gpl->frames) {
grease_pencil.drawing_array[i] = reinterpret_cast<GreasePencilDrawingBase *>(
MEM_new<GreasePencilDrawing>(__func__));
GreasePencilDrawing &drawing = *reinterpret_cast<GreasePencilDrawing *>(
grease_pencil.drawing_array[i]);
/* Convert the frame to a drawing. */
legacy_gpencil_frame_to_grease_pencil_drawing(*gpf, drawing);
GreasePencilFrame new_frame;
new_frame.drawing_index = i;
new_frame.type = gpf->key_type;
SET_FLAG_FROM_TEST(new_frame.flag, (gpf->flag & GP_FRAME_SELECT), GP_FRAME_SELECTED);
new_layer.insert_frame(gpf->framenum, std::move(new_frame));
i++;
}
if ((gpl->flag & GP_LAYER_ACTIVE) != 0) {
grease_pencil.active_layer = static_cast<GreasePencilLayer *>(&new_layer);
}
/* TODO: Update drawing user counts. */
}
/* Convert the onion skinning settings. */
grease_pencil.onion_skinning_settings.opacity = gpd.onion_factor;
grease_pencil.onion_skinning_settings.mode = gpd.onion_mode;
if (gpd.onion_keytype == -1) {
grease_pencil.onion_skinning_settings.filter = GREASE_PENCIL_ONION_SKINNING_FILTER_ALL;
}
else {
grease_pencil.onion_skinning_settings.filter = (1 << gpd.onion_keytype);
}
grease_pencil.onion_skinning_settings.num_frames_before = gpd.gstep;
grease_pencil.onion_skinning_settings.num_frames_after = gpd.gstep_next;
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);
BKE_id_materials_copy(&bmain, &gpd.id, &grease_pencil.id);
}
} // namespace blender::bke::greasepencil::convert

View File

@ -0,0 +1,181 @@
/* SPDX-License-Identifier: GPL-2.0-or-later
* Copyright 2023 Blender Foundation. */
#include "testing/testing.h"
#include "BKE_curves.hh"
#include "BKE_grease_pencil.hh"
#include "BKE_idtype.h"
#include "BKE_lib_id.h"
#include "BKE_main.h"
using namespace blender::bke::greasepencil;
namespace blender::bke::greasepencil::tests {
/* --------------------------------------------------------------------------------------------- */
/* Grease Pencil ID Tests. */
/* Note: Using a struct with constructor and destructor instead of a fixture here, to have all the
Use text fixtures: http://google.github.io/googletest/primer.html#same-data-multiple-tests

I used fixtures before, but decided to use this approach instead, because all the tests are then under the same greasepencil namespace. I didn't find a way to do this with fixtures, unless I name the class literally greasepencil.

I used fixtures before, but decided to use this approach instead, because all the tests are then under the same `greasepencil` namespace. I didn't find a way to do this with fixtures, unless I name the class literally `greasepencil`.

You can't do that either. It is at a very least discouraged to use the same name for fixture and "regular" test.

Typically the "namespace" defines context you're testing, and it is perfectly fine to have multiple of such contexts per file (euclidean_resection_test.cc). But with the monolithic nature of BKE/BLI tests there is no good way to achieve the same behavior.

I would love to improve the monolithic state of tests at some point (as it is really in a way every time I work on test), but that is outside of the scope of this patch.

Would be nice to add a comment on top of the GreasePencilIDTestContext which briefly summarizes your choice of this approach. Basically, so if someone else stumbles on this code and winders "why not fixtures" they have an answer.

You can't do that either. It is at a very least discouraged to use the same name for fixture and "regular" test. Typically the "namespace" defines context you're testing, and it is perfectly fine to have multiple of such contexts per file (`euclidean_resection_test.cc`). But with the monolithic nature of BKE/BLI tests there is no good way to achieve the same behavior. I would love to improve the monolithic state of tests at some point (as it is really in a way every time I work on test), but that is outside of the scope of this patch. Would be nice to add a comment on top of the `GreasePencilIDTestContext` which briefly summarizes your choice of this approach. Basically, so if someone else stumbles on this code and winders "why not fixtures" they have an answer.
* tests in the same group (`greasepencil`). */
struct GreasePencilIDTestContext {
Main *bmain = nullptr;
GreasePencilIDTestContext()
{
BKE_idtype_init();
bmain = BKE_main_new();
}
~GreasePencilIDTestContext()
{
BKE_main_free(bmain);
}
};
TEST(greasepencil, create_grease_pencil_id)
{
GreasePencilIDTestContext ctx;
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(BKE_id_new(ctx.bmain, ID_GP, "GP"));
EXPECT_EQ(grease_pencil.drawings().size(), 0);
EXPECT_EQ(grease_pencil.root_group.wrap().num_nodes_total(), 0);
filedescriptor marked this conversation as resolved Outdated

Add tests for load_layer_tree_from_storage and save_layer_tree_to_storage.

Add tests for `load_layer_tree_from_storage` and `save_layer_tree_to_storage`.