blender
/
blender
133
398
Fork 579
Code Issues 6.3k Pull Requests 747 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 +102 -103
4 changed files with 102 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 40fafee172 - Show all commits

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

@ -115,19 +115,19 @@ static void grease_pencil_blend_write(BlendWriter *writer, ID *id, const void *i
GreasePencilLayerTreeNode *node = grease_pencil->layer_tree_storage.nodes[i];
switch (node->type) {
case GREASE_PENCIL_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf
*>(node); BLO_write_struct(writer, GreasePencilLayerTreeLeaf, node_leaf);
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeLeaf, node_leaf);
/* Write layer data. */
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.keys);
BLO_write_int32_array(
writer, node_leaf->layer.frames_storage.size,
node_leaf->layer.frames_storage.values);
writer, node_leaf->layer.frames_storage.size, node_leaf->layer.frames_storage.values);
break;
Bastien Montagne commented 2023-05-25 12:26:10 +02:00
Outdated
Review
Copy Link

These two lines could use MEM_SAFE_FREE macro instead.

These two lines could use `MEM_SAFE_FREE` macro instead.
Hans Goudey commented 2023-05-25 18:50:26 +02:00
Outdated
Review
Copy Link

MEM_SAFE_FREE does not replace MEM_delete, it replaces MEM_freeN, which is different.

`MEM_SAFE_FREE` does not replace `MEM_delete`, it replaces `MEM_freeN`, which is different.
Falk David commented 2023-05-26 10:19:32 +02:00
Outdated
Review
Copy Link

I was under the impression that any memory allocated with MEM_new should use MEM_delete because we also need to run the destructor.

I was under the impression that any memory allocated with `MEM_new` should use `MEM_delete` because we also need to run the destructor.
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup
*>(node); BLO_write_struct(writer, GreasePencilLayerTreeGroup, group); break;
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_write_struct(writer, GreasePencilLayerTreeGroup, group);
break;
}
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.
}
}
@ -171,20 +171,19 @@ static void grease_pencil_blend_read_data(BlendDataReader *reader, ID *id)
GreasePencilLayerTreeNode *node = grease_pencil->layer_tree_storage.nodes[i];
switch (node->type) {
Hans Goudey commented 2023-05-03 16:26:06 +02:00
Outdated
Review
Copy Link

C++ cast here

C++ cast here
case GREASE_PENCIL_LAYER_TREE_LEAF: {
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf
*>(node); BLO_read_data_address(reader, &node_leaf);
GreasePencilLayerTreeLeaf *node_leaf = reinterpret_cast<GreasePencilLayerTreeLeaf *>(node);
BLO_read_data_address(reader, &node_leaf);
filedescriptor marked this conversation as resolved Outdated
Bastien Montagne commented 2023-05-25 14:48:41 +02:00
Outdated
Review
Copy Link

Code is missing re-reading active_layer address...

Code is missing re-reading `active_layer` address...
/* Read layer data. */
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size,
&node_leaf->layer.frames_storage.keys);
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.keys);
BLO_read_int32_array(
reader, node_leaf->layer.frames_storage.size,
&node_leaf->layer.frames_storage.values);
reader, node_leaf->layer.frames_storage.size, &node_leaf->layer.frames_storage.values);
break;
}
case GREASE_PENCIL_LAYER_TREE_GROUP: {
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup
*>(node); BLO_read_data_address(reader, &group); break;
GreasePencilLayerTreeGroup *group = reinterpret_cast<GreasePencilLayerTreeGroup *>(node);
BLO_read_data_address(reader, &group);
break;
}
}
}

7
source/blender/draw/engines/gpencil/gpencil_engine.cc
View File

@ -5,7 +5,6 @@
* \ingroup draw
*/
#include "BKE_gpencil_legacy.h"
#include "BKE_gpencil_modifier_legacy.h"
#include "BLI_listbase_wrapper.hh"
#include "DEG_depsgraph_query.h"
@ -139,9 +138,11 @@ class Instance {
void object_sync(Manager &manager, ObjectRef &object_ref)
{
switch (object_ref.object->type) {
case OB_GPENCIL_LEGACY:
// case OB_GPENCIL_LEGACY:
// objects.sync_gpencil(manager, object_ref, main_fb_, main_ps_);
// break;
case OB_GREASE_PENCIL:
objects.sync_gpencil(manager, object_ref, main_fb_, main_ps_);
break;
case OB_LAMP:
lights.sync(object_ref);
break;

18
source/blender/draw/engines/gpencil/gpencil_layer.hh
View File

@ -7,7 +7,7 @@
#pragma once
#include "BKE_gpencil_legacy.h"
#include "BKE_grease_pencil.hh"
#include "BKE_image.h"
filedescriptor marked this conversation as resolved
Sergey Sharybin commented 2023-04-25 09:49:35 +02:00
Review
Copy Link

I am not sure all of those are needed here.
I.e. I do not see any BKE_image API used in this header. Might be missing something.

I am not sure all of those are needed here. I.e. I do not see any `BKE_image` API used in this header. Might be missing something.
#include "DRW_gpu_wrapper.hh"
#include "DRW_render.h"
@ -30,17 +30,17 @@ class LayerModule {
layers_buf_.clear();
}
void sync(const Object *object, const bGPDlayer *gpl, bool &do_layer_blending)
void sync(const Object *object, const bke::gpencil::Layer &layer, bool &do_layer_blending)
{
UNUSED_VARS(object, gpl);
UNUSED_VARS(object, layer);
Sergey Sharybin commented 2023-04-25 09:48:46 +02:00
Outdated
Review
Copy Link

In C++ use void sync(const Object * /*object*/, const bke::greasepencil::Layer & /*layer*/, bool &do_layer_blending)

In C++ use `void sync(const Object * /*object*/, const bke::greasepencil::Layer & /*layer*/, bool &do_layer_blending)`
/* TODO(fclem): All of this is placeholder. */
gpLayer layer;
layer.vertex_color_opacity = 0.0f;
layer.opacity = 1.0f;
layer.thickness_offset = 0.0f;
layer.tint = float4(1.0f, 1.0f, 1.0f, 0.0f);
gpLayer gp_layer;
gp_layer.vertex_color_opacity = 0.0f;
gp_layer.opacity = 1.0f;
gp_layer.thickness_offset = 0.0f;
gp_layer.tint = float4(1.0f, 1.0f, 1.0f, 0.0f);
layers_buf_.append(layer);
layers_buf_.append(gp_layer);
do_layer_blending = false;
}

153
source/blender/draw/engines/gpencil/gpencil_object.hh
View File

@ -7,7 +7,7 @@
#pragma once
#include "BKE_gpencil_legacy.h"
#include "BKE_grease_pencil.hh"
#include "BKE_image.h"
#include "DRW_gpu_wrapper.hh"
#include "DRW_render.h"
@ -117,15 +117,16 @@ class ObjectModule {
Framebuffer &main_fb,
PassSortable &main_ps)
{
Object *object = object_ref.object;
bGPdata *gpd = static_cast<bGPdata *>(object->data);
ListBaseWrapper<const bGPDlayer> layers(&gpd->layers);
using namespace blender::bke::gpencil;
if (BLI_listbase_is_empty(&gpd->layers)) {
Object *object = object_ref.object;
GreasePencil &grease_pencil = *static_cast<GreasePencil *>(object->data);
if (grease_pencil.drawings().is_empty()) {
return;
}
const bool is_stroke_order_3d = (gpd->draw_mode == GP_DRAWMODE_3D);
const bool is_stroke_order_3d = true; /* TODO */
bool do_material_holdout = false;
bool do_layer_blending = false;
bool object_has_vfx = false; // TODO vfx.object_has_vfx(gpd);
@ -136,9 +137,8 @@ class ObjectModule {
}
uint layer_offset = layers_.object_offset_get();
for (const bGPDlayer *layer : layers) {
layers_.sync(object, layer, do_layer_blending);
}
grease_pencil.runtime->root_group().foreach_layer_pre_order(
[&](Layer &layer) { layers_.sync(object, layer, do_layer_blending); });
/* Order rendering using camera Z distance. */
float3 position = float3(object->object_to_world[3]);
@ -160,45 +160,43 @@ class ObjectModule {
object_subpass.state_set(state);
object_subpass.shader_set(shaders_.static_shader_get(GREASE_PENCIL));
Vector<FrameData, 5> frames;
displayed_frame_select(frames, layers);
grease_pencil.foreach_visible_drawing(
current_frame_, [&](GreasePencilDrawing &drawing, bke::gpencil::Layer &layer) {
/* TODO(fclem): Pass per frame object matrix here. */
ResourceHandle handle = manager.resource_handle(object_ref);
gpObject &ob = objects_buf_.get_or_resize(handle.resource_index());
Sergey Sharybin commented 2023-04-25 09:53:01 +02:00
Outdated
Review
Copy Link

Typically dead code is discouraged.
Not sure what is it part of, so can't really give strong suggestions.

Typically dead code is discouraged. Not sure what is it part of, so can't really give strong suggestions.
ob.is_shadeless = false;
ob.stroke_order3d = false;
ob.tint = float4(1.0); // frame_tint_get(gpd, frame.gpf, current_frame_);
ob.layer_offset = layer_offset;
ob.material_offset = material_offset;
for (const FrameData &frame : frames) {
/* TODO(fclem): Pass per frame object matrix here. */
ResourceHandle handle = manager.resource_handle(object_ref);
gpObject &ob = objects_buf_.get_or_resize(handle.resource_index());
ob.is_shadeless = false;
ob.stroke_order3d = false;
ob.tint = frame_tint_get(gpd, frame.gpf, current_frame_);
ob.layer_offset = layer_offset;
ob.material_offset = material_offset;
GPUVertBuf *position_tx = DRW_cache_gpencil_position_buffer_get(object, current_frame_);
GPUVertBuf *color_tx = DRW_cache_gpencil_color_buffer_get(object, current_frame_);
GPUBatch *geom = DRW_cache_gpencil_get(object, current_frame_);
GPUVertBuf *position_tx = DRW_cache_gpencil_position_buffer_get(object, frame.gpf->framenum);
GPUVertBuf *color_tx = DRW_cache_gpencil_color_buffer_get(object, frame.gpf->framenum);
GPUBatch *geom = DRW_cache_gpencil_get(object, frame.gpf->framenum);
if (do_layer_blending) {
// for (const LayerData &layer : frame.layers) {
// UNUSED_VARS(layer);
// if (has_blending(layer)) {
// object_subpass.framebuffer_set(*vfx_fb.current());
// }
if (do_layer_blending) {
for (const LayerData &layer : frame.layers) {
UNUSED_VARS(layer);
// if (has_blending(layer)) {
// object_subpass.framebuffer_set(*vfx_fb.current());
// }
/* TODO(fclem): Only draw subrange of geometry for this layer. */
object_subpass.draw(geom, handle);
/* TODO(fclem): Only draw subrange of geometry for this layer. */
object_subpass.draw(geom, handle);
// if (has_blending(layer)) {
// layer_blend_sync(object_ref, object_subpass);
// }
}
}
else {
/* Fast path. */
object_subpass.bind_texture("gp_pos_tx", position_tx);
object_subpass.bind_texture("gp_col_tx", color_tx);
object_subpass.draw(geom, handle);
}
}
// if (has_blending(layer)) {
// layer_blend_sync(object_ref, object_subpass);
Sergey Sharybin commented 2023-04-25 09:53:24 +02:00
Review
Copy Link

Add a comment what is this about, or remove.

Add a comment what is this about, or remove.
// }
// }
}
else {
/* Fast path. */
object_subpass.bind_texture("gp_pos_tx", position_tx);
object_subpass.bind_texture("gp_col_tx", color_tx);
object_subpass.draw(geom, handle);
}
});
#if 0
if (object_has_vfx) {
@ -262,43 +260,44 @@ class ObjectModule {
}
private:
static float4 frame_tint_get(const bGPdata *gpd, const bGPDframe *gpf, int /* current_frame */)
{
/* TODO(fclem): Onion color should rely on time and or frame id and not on runtime.onion_id.
* This should be evaluated at draw time as it is just a way of displaying the data. */
const bool use_onion_custom_col = (gpd->onion_flag & GP_ONION_GHOST_PREVCOL) != 0;
const bool use_onion_fade = (gpd->onion_flag & GP_ONION_FADE) != 0;
const bool use_next_col = gpf->runtime.onion_id > 0.0f;
// static float4 frame_tint_get(const bGPdata *gpd, const bGPDframe *gpf, int /* current_frame
// */)
// {
// /* TODO(fclem): Onion color should rely on time and or frame id and not on runtime.onion_id.
// * This should be evaluated at draw time as it is just a way of displaying the data. */
// const bool use_onion_custom_col = (gpd->onion_flag & GP_ONION_GHOST_PREVCOL) != 0;
// const bool use_onion_fade = (gpd->onion_flag & GP_ONION_FADE) != 0;
// const bool use_next_col = gpf->runtime.onion_id > 0.0f;
const float *onion_col_custom = (use_onion_custom_col) ?
(use_next_col ? gpd->gcolor_next : gpd->gcolor_prev) :
U.gpencil_new_layer_col;
// const float *onion_col_custom = (use_onion_custom_col) ?
// (use_next_col ? gpd->gcolor_next : gpd->gcolor_prev) :
// U.gpencil_new_layer_col;
float4 tint = {UNPACK3(onion_col_custom), 1.0f};
// float4 tint = {UNPACK3(onion_col_custom), 1.0f};
tint[3] = use_onion_fade ? (1.0f / abs(gpf->runtime.onion_id)) : 0.5f;
tint[3] *= gpd->onion_factor;
tint[3] = (gpd->onion_factor > 0.0f) ? clamp_f(tint[3], 0.1f, 1.0f) :
clamp_f(tint[3], 0.01f, 1.0f);
return tint;
}
// tint[3] = use_onion_fade ? (1.0f / abs(gpf->runtime.onion_id)) : 0.5f;
// tint[3] *= gpd->onion_factor;
// tint[3] = (gpd->onion_factor > 0.0f) ? clamp_f(tint[3], 0.1f, 1.0f) :
// clamp_f(tint[3], 0.01f, 1.0f);
// return tint;
// }
void displayed_frame_select(Vector<FrameData, 5> &frames,
ListBaseWrapper<const bGPDlayer> layers)
{
/* TODO(fclem): Select onion skin frames. */
/** \note Change data layout to be Frame major instead of Layer major.
* Hopefully the GPencil data layout will be closer to that in the future. */
FrameData frame_data;
frame_data.gpf = layers.get(0)->actframe;
for (const bGPDlayer *layer : layers) {
LayerData layer_data;
layer_data.gpf = layer->actframe;
layer_data.gpl = layer;
frame_data.layers.append(layer_data);
}
frames.append(frame_data);
}
// void displayed_frame_select(Vector<FrameData, 5> &frames,
// ListBaseWrapper<const bGPDlayer> layers)
// {
// /* TODO(fclem): Select onion skin frames. */
// /** \note Change data layout to be Frame major instead of Layer major.
// * Hopefully the GPencil data layout will be closer to that in the future. */
// FrameData frame_data;
// frame_data.gpf = layers.get(0)->actframe;
// for (const bGPDlayer *layer : layers) {
// LayerData layer_data;
// layer_data.gpf = layer->actframe;
// layer_data.gpl = layer;
// frame_data.layers.append(layer_data);
// }
// frames.append(frame_data);
// }
};
} // namespace blender::gpencil