From 2e82f95237b28b111f4b780c29d8f261adc95dae Mon Sep 17 00:00:00 2001 From: YimingWu Date: Tue, 12 Nov 2024 18:51:45 +0100 Subject: [PATCH 1/2] Fix: LineArt: Cache mechanism fixes The cache mechanism for line art is changed during migration to GPv3, however the code path failed to handle following cases which could lead to a few problems: - Line art cache isn't deleted after last line art modifier because it coule be hidden, causing memory leaks. - A modifier inside a multiple line art modifier sequence that doesn't use cache would prematurely delete line art cache, causing subsequent line art modifier to give empty result. - When the first line art modifier is hidden (in viewport/render), the cache is not created correctly, leading to crashes. Now the new code logic addresses these problems properly by: - Making sure the last visible line art modifier deletes cache. - Giving a fresh cache pointer for modifiers that doesn't use global cache. - Line art cache is correctly ensured when there are modifiers that are hidden Pull Request: https://projects.blender.org/blender/blender/pulls/129953 --- .../intern/grease_pencil_lineart.cc | 6 ++- .../editors/include/ED_grease_pencil.hh | 2 +- .../blender/modifiers/intern/MOD_lineart.cc | 52 ++++++++++++------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/source/blender/editors/grease_pencil/intern/grease_pencil_lineart.cc b/source/blender/editors/grease_pencil/intern/grease_pencil_lineart.cc index 1e2d6fb7392..7415bfbac02 100644 --- a/source/blender/editors/grease_pencil/intern/grease_pencil_lineart.cc +++ b/source/blender/editors/grease_pencil/intern/grease_pencil_lineart.cc @@ -66,10 +66,10 @@ void get_lineart_modifier_limits(const Object &ob, void set_lineart_modifier_limits(GreasePencilLineartModifierData &lmd, const blender::ed::greasepencil::LineartLimitInfo &info, - const bool is_first_lineart) + const bool cache_is_ready) { BLI_assert(lmd.modifier.type == eModifierType_GreasePencilLineart); - if (is_first_lineart || lmd.flags & MOD_LINEART_USE_CACHE) { + if ((!cache_is_ready) || (lmd.flags & MOD_LINEART_USE_CACHE)) { lmd.level_start_override = info.min_level; lmd.level_end_override = info.max_level; lmd.edge_types_override = info.edge_types; @@ -87,6 +87,8 @@ void set_lineart_modifier_limits(GreasePencilLineartModifierData &lmd, GreasePencilLineartModifierData *get_first_lineart_modifier(const Object &ob) { + /* This function always gets the first line art modifier regardless of their visibility, because + * cached line art configuration are always inside the first line art modifier. */ LISTBASE_FOREACH (ModifierData *, i_md, &ob.modifiers) { if (i_md->type == eModifierType_GreasePencilLineart) { return reinterpret_cast(i_md); diff --git a/source/blender/editors/include/ED_grease_pencil.hh b/source/blender/editors/include/ED_grease_pencil.hh index 6ea20ec6a3d..ee254105415 100644 --- a/source/blender/editors/include/ED_grease_pencil.hh +++ b/source/blender/editors/include/ED_grease_pencil.hh @@ -891,7 +891,7 @@ struct LineartLimitInfo { void get_lineart_modifier_limits(const Object &ob, LineartLimitInfo &info); void set_lineart_modifier_limits(GreasePencilLineartModifierData &lmd, const LineartLimitInfo &info, - const bool is_first_lineart); + const bool cache_is_ready); GreasePencilLineartModifierData *get_first_lineart_modifier(const Object &ob); diff --git a/source/blender/modifiers/intern/MOD_lineart.cc b/source/blender/modifiers/intern/MOD_lineart.cc index 783bf3a9ba8..ac8bcf65871 100644 --- a/source/blender/modifiers/intern/MOD_lineart.cc +++ b/source/blender/modifiers/intern/MOD_lineart.cc @@ -55,7 +55,7 @@ static bool is_first_lineart(const GreasePencilLineartModifierData &md) return true; } -static bool is_last_line_art(const GreasePencilLineartModifierData &md) +static bool is_last_line_art(const GreasePencilLineartModifierData &md, const bool use_render) { if (md.modifier.type != eModifierType_GreasePencilLineart) { return false; @@ -63,7 +63,12 @@ static bool is_last_line_art(const GreasePencilLineartModifierData &md) ModifierData *imd = md.modifier.next; while (imd != nullptr) { if (imd->type == eModifierType_GreasePencilLineart) { - return false; + if (use_render && (imd->mode & eModifierMode_Render)) { + return false; + } + if ((!use_render) && (imd->mode & eModifierMode_Realtime)) { + return false; + } } imd = imd->next; } @@ -767,7 +772,8 @@ static void panel_register(ARegionType *region_type) static void generate_strokes(ModifierData &md, const ModifierEvalContext &ctx, GreasePencil &grease_pencil, - GreasePencilLineartModifierData &first_lineart) + GreasePencilLineartModifierData &first_lineart, + const bool force_compute) { using namespace bke::greasepencil; auto &lmd = reinterpret_cast(md); @@ -777,9 +783,16 @@ static void generate_strokes(ModifierData &md, return; } - LineartCache *local_lc = first_lineart.shared_cache; + const bool is_first_lineart = (&first_lineart == &lmd); + const bool use_cache = (lmd.flags & MOD_LINEART_USE_CACHE); + LineartCache *local_lc = (is_first_lineart || use_cache) ? first_lineart.shared_cache : nullptr; - if (!(lmd.flags & MOD_LINEART_USE_CACHE)) { + /* Only calculate strokes in these three conditions: + * 1. It's the very first line art modifier in the stack. + * 2. This line art modifier doesn't want to use globally cached data. + * 3. This modifier is not the first line art in stack, but it's the first that's visible (so we + * need to do a `force_compute`). */ + if (is_first_lineart || (!use_cache) || force_compute) { MOD_lineart_compute_feature_lines_v3( ctx.depsgraph, lmd, &local_lc, !(ctx.object->dtx & OB_DRAW_IN_FRONT)); MOD_lineart_destroy_render_data_v3(&lmd); @@ -825,11 +838,10 @@ static void generate_strokes(ModifierData &md, lmd.flags, lmd.calculation_flags); - if (!(lmd.flags & MOD_LINEART_USE_CACHE) && (&first_lineart != &lmd)) { - /* Clear local cache. */ - if (local_lc != first_lineart.shared_cache) { - MOD_lineart_clear_cache(&local_lc); - } + if ((!is_first_lineart) && (!use_cache)) { + /* We only clear local cache, not global cache from the first line art modifier. */ + BLI_assert(local_lc != first_lineart.shared_cache); + MOD_lineart_clear_cache(&local_lc); /* Restore the original cache pointer so the modifiers below still have access to the "global" * cache. */ lmd.cache = first_lineart.shared_cache; @@ -850,18 +862,22 @@ static void modify_geometry_set(ModifierData *md, blender::ed::greasepencil::get_first_lineart_modifier(*ctx->object); BLI_assert(first_lineart); - bool is_first_lineart = (mmd == first_lineart); - - if (is_first_lineart) { - mmd->shared_cache = MOD_lineart_init_cache(); - ed::greasepencil::get_lineart_modifier_limits(*ctx->object, mmd->shared_cache->LimitInfo); + /* Since settings for line art cached data are always in the first line art modifier, we need to + * get and set overall calculation limits on the first modifier regardless of its visibility + * state. If line art cache doesn't exist, it means line art hasn't done any calculation. */ + const bool cache_ready = (first_lineart->shared_cache != nullptr); + if (!cache_ready) { + first_lineart->shared_cache = MOD_lineart_init_cache(); + ed::greasepencil::get_lineart_modifier_limits(*ctx->object, + first_lineart->shared_cache->LimitInfo); } ed::greasepencil::set_lineart_modifier_limits( - *mmd, first_lineart->shared_cache->LimitInfo, is_first_lineart); + *mmd, first_lineart->shared_cache->LimitInfo, cache_ready); - generate_strokes(*md, *ctx, grease_pencil, *first_lineart); + generate_strokes(*md, *ctx, grease_pencil, *first_lineart, (!cache_ready)); - if (is_last_line_art(*mmd)) { + const bool use_render_params = (ctx->flag & MOD_APPLY_RENDER); + if (is_last_line_art(*mmd, use_render_params)) { MOD_lineart_clear_cache(&first_lineart->shared_cache); } From 9a4ce4a0f77364077143e00b8b5106f2df13b410 Mon Sep 17 00:00:00 2001 From: Pratik Borhade Date: Tue, 12 Nov 2024 18:56:12 +0100 Subject: [PATCH 2/2] Fix #128749: GPv3: Dopesheet Move channel to top reverses layer order The operator to move a channel to the top revered the layer order by iterating from the top to the bottom and moving selected channels to the top. Iterate from the bottom to the top instead to keep the right layer ordering. Pull Request: https://projects.blender.org/blender/blender/pulls/128793 --- .../editors/animation/anim_channels_edit.cc | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/source/blender/editors/animation/anim_channels_edit.cc b/source/blender/editors/animation/anim_channels_edit.cc index 0762aa6cc17..8a2122431b4 100644 --- a/source/blender/editors/animation/anim_channels_edit.cc +++ b/source/blender/editors/animation/anim_channels_edit.cc @@ -2064,34 +2064,39 @@ static void rearrange_grease_pencil_channels(bAnimContext *ac, eRearrangeAnimCha ANIM_animdata_filter( ac, &anim_data, eAnimFilter_Flags(filter), ac->data, eAnimCont_Types(ac->datatype)); - LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { - GreasePencil &grease_pencil = *reinterpret_cast(ale->id); - Layer *layer = static_cast(ale->data); + if (mode == REARRANGE_ANIMCHAN_TOP) { + LISTBASE_FOREACH_BACKWARD (bAnimListElem *, ale, &anim_data) { + GreasePencil &grease_pencil = *reinterpret_cast(ale->id); + Layer *layer = static_cast(ale->data); + if (layer->is_selected()) { + grease_pencil.move_node_top(layer->as_node()); + } + } + } + else { + LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { + GreasePencil &grease_pencil = *reinterpret_cast(ale->id); + Layer *layer = static_cast(ale->data); - switch (mode) { - case REARRANGE_ANIMCHAN_TOP: { - if (layer->is_selected()) { - grease_pencil.move_node_top(layer->as_node()); + switch (mode) { + case REARRANGE_ANIMCHAN_UP: { + if (layer->is_selected()) { + grease_pencil.move_node_up(layer->as_node()); + } + break; } - break; - } - case REARRANGE_ANIMCHAN_UP: { - if (layer->is_selected()) { - grease_pencil.move_node_up(layer->as_node()); + case REARRANGE_ANIMCHAN_DOWN: { + if (layer->is_selected()) { + grease_pencil.move_node_down(layer->as_node()); + } + break; } - break; - } - case REARRANGE_ANIMCHAN_DOWN: { - if (layer->is_selected()) { - grease_pencil.move_node_down(layer->as_node()); + case REARRANGE_ANIMCHAN_BOTTOM: { + if (layer->is_selected()) { + grease_pencil.move_node_bottom(layer->as_node()); + } + break; } - break; - } - case REARRANGE_ANIMCHAN_BOTTOM: { - if (layer->is_selected()) { - grease_pencil.move_node_bottom(layer->as_node()); - } - break; } } }