GPv3: Line Art modifier migration #117028
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#117028
Loading…
Reference in New Issue
No description provided.
Delete Branch "ChengduLittleA/blender:gp3-new-lineart"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is a working migration of the old Line Art modifier. All the features stays the same for the moment until we get everything ported to GPv3.
Note:
ModifierData
part should stay exactly the same until we do proper versioning of the modifier data and completely remove the GPv2 support.f89943385e
to426523746c
525df0ae74
tod7cbbe3f6c
Did a fist pass on this. Haven't touch everything yet.
Most concerning part to me right now is the
LineartCache
pointer that you store in the object-data runtime and how that interacts with the modifier stack. I'm pretty sure that this breaks some design principles.Would be good if you could explain why this is needed and how it impacts performance.
@ -721,6 +721,8 @@ class GreasePencilRuntime {
/* The frame on which the object was evaluated (only valid for evaluated object). */
int eval_frame = 0;
struct ::LineartCache *lineart_cache;
What's the use of this cache? Why does it need to be stored on the object-data?
So I think this pointer should be stored on
ModifierEvalContext
instead.The
ModifierEvalContext
is created ingrease_pencil_evaluate_modifiers
. This means that we can free the cache at the end of the function.This makes the lifetime of this cache very clear: It only exists during modifier stack evaluation.
That does sound good. Let me try putting it there instead. Thanks!
@ -891,0 +938,4 @@
if (md->type != eModifierType_GreasePencilLineart) {
return false;
}
LISTBASE_FOREACH (ModifierData *, gmd, &ob->modifiers) {
gmd
->md
@ -891,0 +947,4 @@
}
}
/* If we reach here it means md is not in ob's modifier stack. */
BLI_assert(false);
This assert doesn't do anything.
@ -5423,0 +5681,4 @@
curves[writer.index_range().last() + 1] = up_to_point;
blender::float4x4 mat(gp_obmat_inverse);
drawing.strokes_for_write() = blender::ed::greasepencil::create_drawing_data(
Modifiers shouldn't use code from the editors. You can define a
static
function in this file.@ -5491,2 +5761,4 @@
modifier_calculation_flags);
}
void MOD_lineart_gpencil_generate_v3(LineartCache *cache,
Most of the parameters should be const if they are just passed as parameters.
@ -5492,1 +5762,4 @@
}
void MOD_lineart_gpencil_generate_v3(LineartCache *cache,
Object *ob,
const Object &
@ -5493,0 +5763,4 @@
void MOD_lineart_gpencil_generate_v3(LineartCache *cache,
Object *ob,
struct GreasePencilDrawing &drawing,
bke::greasepencil::Drawing &
@ -5493,0 +5773,4 @@
uchar mask_switches,
uchar material_mask_bits,
uchar intersection_mask,
int16_t thickness,
I think it would be worth is to switch this to a
const float radius
.I'll keep DNA thickness as
int16_t
to keep modifier data layout exactly the same as the old one.Alright, I can agree to not change the behavior/UI.
@ -5493,0 +5803,4 @@
float gp_obmat_inverse[4][4];
invert_m4_m4(gp_obmat_inverse, ob->object_to_world);
lineart_gpencil_generate_v3(cache,
I'm not sure why it's necessary to nest this further.
@ -0,0 +706,4 @@
modifier_subpanel_register(region_type, "bake", "Bake", nullptr, bake_panel_draw, panel_type);
}
static void generate_strokes_actual(const ModifierData &md,
There's no need for this to be a function.
@ -0,0 +738,4 @@
lmd.calculation_flags);
}
static void generate_strokes(ModifierData &md, Depsgraph *depsgraph, Object &ob, GreasePencil &gpd)
gpd
->grease_pencil
@ -0,0 +740,4 @@
static void generate_strokes(ModifierData &md, Depsgraph *depsgraph, Object &ob, GreasePencil &gpd)
{
GreasePencilLineartModifierData &lmd = (GreasePencilLineartModifierData &)md;
Use c++ casts
@ -0,0 +743,4 @@
GreasePencilLineartModifierData &lmd = (GreasePencilLineartModifierData &)md;
/* This will check whether layer/materials are non-null. */
if (is_disabled(nullptr, &md, false)) {
Why is this call needed here?
@ -0,0 +759,4 @@
MOD_lineart_destroy_render_data_v3(&lmd);
}
else {
if (!(lmd.flags & LRT_GPENCIL_USE_CACHE)) {
Looks like you're using the old flags here. This should be
LINEART_GPENCIL_USE_CACHE
. Same for other places.That does make a good point. I'm not so sure about the approach of this, since for now the modifier data part stays exactly the same for v3 and v2 modifiers, I thought maybe they should use the exact same flags for the moment (as defined in the old DNA) to avoid accidental changes causing flags to mismatch etc.
I pass the new modifier data to the calculation function by memcpy-ing it to the old modifier data, so everything has to stay the same for now, we can have a more complex wrapper that manually assign all the flags if we changed the layout of the modifier data of v3 but it's not necessary for what we are doing right now.
Do you suggest that we keep v2 and v3 symbols as separated as possible? (Althougth I did copy all the flags and changed their names to
LINEART_
fromLRT_
tho, just not decided yet)@ -0,0 +769,4 @@
}
/* Ensure we have a frame in the selected layer to put line art result in. */
bke::greasepencil::Layer &got_layer = node->as_layer();
got_layer
->layer
@ -0,0 +770,4 @@
/* Ensure we have a frame in the selected layer to put line art result in. */
bke::greasepencil::Layer &got_layer = node->as_layer();
got_layer.add_frame(0, 0, 0);
This code seems a bit sketchy. I think you should first try and get a drawing and only insert a new one if there is not a drawing already. Something like
Thanks for the suggestion, however should we clear the drawing if the drawing at that frame already exists?
Seems like the rest of the code will overwrite the geometry anyway.
@ -0,0 +826,4 @@
/*struct_name*/ "GreasePencilLineartModifierData",
/*struct_size*/ sizeof(GreasePencilLineartModifierData),
/*srna*/ &RNA_GreasePencilLineartModifier,
/*type*/ ModifierTypeType::Nonconstructive,
ModifierTypeType::Constructive
But it will delete old geometry?
Right, might be fine to keep
Nonconstructive
then.@filedescriptor
The idea of using
LineartCache
in object runtime is that line art only needs to run once (if theuse_cache
options are enabled) for multiple line art modifiers, and those modifiers only selectively generate strokes from 1 line art result, it's the same as the GPv2 line art implementation which is also ingpd->runtime
. Of course after GPv2 has been removed we can have new designs like nodes and stuff but at the moment it's just to move it directly to v3 without much changes otherwise it's gonna be confusing.I'll first clean up these notes.
@ChengduLittleA See #117028 (comment)
Did a pass on the eval context changes.
@ -162,1 +162,4 @@
ModifierApplyFlag flag;
/* Line Art modifier cache will exist until the end of modifier stack evaluation. */
struct ::LineartCache *lineart_cache;
Would be good to default this to
nullptr
.struct ::LineartCache *lineart_cache = nullptr;
@ -1264,6 +1268,17 @@ GreasePencil *BKE_grease_pencil_copy_for_eval(const GreasePencil *grease_pencil_
return grease_pencil;
}
static struct ::LineartCache *init_lineart_cache()
This function is not needed.
@ -1267,0 +1273,4 @@
return MOD_lineart_init_cache();
}
static void clear_lineart_cache(struct ::LineartCache *lc)
This function is also not needed.
@ -1277,3 +1292,3 @@
}
ModifierApplyFlag apply_flag = use_render ? MOD_APPLY_RENDER : MOD_APPLY_USECACHE;
const ModifierEvalContext mectx = {depsgraph, object, apply_flag};
LineartCache *lineart_cache = init_lineart_cache();
I don't think we should initialize the lineart cache in all cases.
Yeah it should only initialize when there's line art modifier present. Originally the cache is written into the grease pencil data structure but now in
ModifierEvalContext
, which isconst
so line art function can't change the pointer in there. That's why I put a separate init call before evaluation. I'll check modifier first then.Ah it's ok for
ModifierEvalContext
to not be declared const here.@ -1278,2 +1293,3 @@
ModifierApplyFlag apply_flag = use_render ? MOD_APPLY_RENDER : MOD_APPLY_USECACHE;
const ModifierEvalContext mectx = {depsgraph, object, apply_flag};
LineartCache *lineart_cache = init_lineart_cache();
const ModifierEvalContext mectx = {depsgraph, object, apply_flag, lineart_cache};
I'd prefer something like
@ -1298,2 +1322,4 @@
}
}
clear_lineart_cache(lineart_cache);
Can't do it exactly like this since mectx is const.
Right,
mectx
can be declared non-const :)The problem is that the evaluation call function is also
const
, we probably don't want to change this in all the places?No. There is a couple of ways to deal with this. I propose the following:
std::unique_ptr<LineartCache> lineart_cache;
to theModifierEvalContext
.std::unique_ptr
means that we need to add a destructor toLineartCache
. I guess that would just beMOD_lineart_clear_cache
basically.get
toLineartCache
that computes the cache lazily. E.g. the first time the function is called, the cache is populated and any subuquent calls will just return the cache directly.ModifierEvalContext
can still be const, because we're modifing something inside that pointer that is not const.I see... But then this looks largely the same as:
ModifierEvalContext
.I don't think that works if the
ModifierEvalContext
is passed asconst
. You wouldn't be able to change the pointer.Hummm so currently I just pre-create that cache and set the pointer inside
ModifierEvalContext
, and they can modify stuff inside. Do we still need to make it into a class and have constructor and destructor? 🤔@ -911,6 +924,8 @@ LineartBoundingArea *MOD_lineart_get_parent_bounding_area(LineartData *ld, doubl
*/
LineartBoundingArea *MOD_lineart_get_bounding_area(LineartData *ld, double x, double y);
struct GreasePencilDrawing;
Is this needed?
Looks like it isn't.
@ -7962,0 +8328,4 @@
"Match the beginning of vertex group names from mesh objects, match all when left empty");
RNA_def_property_update(prop, 0, "rna_Modifier_update");
// prop = RNA_def_property(srna, "vertex_group", PROP_STRING, PROP_NONE);
Even though it might not do anything right now, this should be uncommented.
Right this is for vertex weight transferring. I guess I'll keep the property there for now.
@ -17,6 +17,9 @@ set(INC
../render
../windowmanager
../../../intern/eigen
../gpencil_modifiers_legacy
Are these needed?
Those are needed for the calls to shared legacy line art functions.
@ -554,1 +557,4 @@
/* Stores the maximum calculation range in the whole modifier stack for line art so the cache can
* cover everything that will be visible. */
typedef struct GreasePencilLineartLimitInfo {
typedef
is unnecessary in C++ code@ -555,0 +558,4 @@
/* Stores the maximum calculation range in the whole modifier stack for line art so the cache can
* cover everything that will be visible. */
typedef struct GreasePencilLineartLimitInfo {
short edge_types;
Use integer types with specific widths,
int16_t
orint8_t
for example@ -555,0 +568,4 @@
bool BKE_grease_pencil_has_lineart_modifier(const Object *ob);
GreasePencilLineartLimitInfo BKE_grease_pencil_get_lineart_modifier_limits(
const struct Object *ob);
struct
is unnecessary in C++@ -891,0 +905,4 @@
bool is_first = true;
LISTBASE_FOREACH (ModifierData *, md, &ob->modifiers) {
if (md->type == eModifierType_GreasePencilLineart) {
auto *lmd = reinterpret_cast<GreasePencilLineartModifierData *>(md);
const ModifierData *,
@ -891,0 +948,4 @@
if (md->type != eModifierType_GreasePencilLineart) {
return false;
}
LISTBASE_FOREACH (ModifierData *, i_md, &ob->modifiers) {
const ModifierData *
@ -3547,0 +3554,4 @@
void MOD_lineart_wrap_modifier_v3(LineartGpencilModifierData *lmd_legacy,
GreasePencilLineartModifierData *lmd)
{
memcpy((reinterpret_cast<char *>(lmd)) + sizeof(ModifierData) +
Personally I'd just copy the fields manually, this is just unnecessarily fragile and weird
@ -5015,3 +5051,1 @@
if (lmd->calculation_flags & LRT_USE_CUSTOM_CAMERA) {
if (!lmd->source_camera ||
(lineart_camera = DEG_get_evaluated_object(depsgraph, lmd->source_camera))->type !=
if (lmd.calculation_flags & LRT_USE_CUSTOM_CAMERA) {
+1 for the pointer to reference change! But let's leave that sort of cleanup for separate cleanup commits
@ -5493,0 +5786,4 @@
return;
}
blender::Array<blender::float3> positions(total_point_count);
You can just create the
bke::CurvesGeometry
here and fill in the attributes directly belowWIP: GPv3: Line Art modifier migrationto GPv3: Line Art modifier migrationUpdated to include vertex weight transferring feature, note that it doesn't work exactly the same way due to the change of attribute API. See PR description.
OMG why there are so many commits here after rebasing...
69151b2f1d
to9b0a390a42
Fixed the patch with a force push but not sure how to get rid of the erroneous commit message on this page...
@ChengduLittleA Don't worry, that's a gitea thing..
@blender-bot build
@ -162,1 +162,4 @@
ModifierApplyFlag flag;
/* Line Art modifier cache will exist until the end of modifier stack evaluation. */
struct ::LineartCache *lineart_cache = nullptr;
Adding the cache for a single modifier type doesn't really make sense here. This is a more general struct that shouldn't know about any one modifier type in particular.
Generally runtime data can be saved in a modifier data struct directly. For example:
NodesModifierData::runtime
. It would probably make sense to do the same thing here: When the first line art modifier executes, store the cache there, and access the first modifier's runtime data in the next modifiers.Didn't know about the runtime for modifiers. Indeed, it makes more sense to put the cache there.
I now moved it to
GreasePencilLineartModifierData
.@blender-bot build
The PR has lots of conflicts, could you update it to latest main?
@blender-bot build
@ -1371,0 +1378,4 @@
GreasePencilLineartModifierData *first_lineart = BKE_grease_pencil_get_first_lineart_modifier(
object);
if (first_lineart) {
first_lineart->shared_cache = MOD_lineart_init_cache();
Don't you only want to add this cache if there's a second line art modifier that will use the data?
Eh yeah but the line art code path expect the cache to be always available because the final result is in there even if there's only one line art modifier in the stack. And here it adds the cache for the first one, later if more line art modifiers exists, it will refer to this cache internally.
It feels like all of this logic could just be in the
modify_geometry_set
function of the lineart modifier.Since this function (
grease_pencil_evaluate_modifiers
) is really only supposed to run the modifiers, having special handling for lineart in here is not ideal.I propose the following:
modifiy_geometry_set
callGreasePencilLineartModifierData *first_lineart = BKE_grease_pencil_get_first_lineart_modifier(ctx.object);
to get the first lineart modifier.const bool is_first_lineart = (first_lineart == lmd);
BKE_grease_pencil_get_lineart_modifier_limits(ctx.object);
BKE_grease_pencil_set_lineart_modifier_limits(md, &info, is_first_lineart);
lmd
is the last lineart modifier, callMOD_lineart_clear_cache(&first_lineart->shared_cache);
@ -35,6 +35,7 @@
#include "BLI_linklist.h"
#include "BLI_listbase.h"
#include "BLI_math_base.hh"
Same here,
std::min
andstd::max
instead@ -13,6 +13,8 @@
#include "BLI_math_vector.h"
#include "BLI_threads.h"
#include "BKE_grease_pencil.hh"
Better to use forward declarations and avoid indirect header includes
@ -3540,0 +3558,4 @@
LMD_WRAP(source_object);
LMD_WRAP(source_collection);
LMD_WRAP(target_material);
strcpy(lmd->source_vertex_group, lmd_legacy->source_vertex_group);
STRNCPY
https://developer.blender.org/docs/handbook/guidelines/best_practice_c_cpp/#avoid-unsafe-string-c-apis
@ -5486,0 +5674,4 @@
bool inverse_silhouette = modifier_flags & LRT_GPENCIL_INVERT_SILHOUETTE_FILTER;
blender::Vector<LineartChainWriteInfo> writer = {};
= {}
is unnecessary, Vector has a default constructor@ -5486,0 +5803,4 @@
}
bke::CurvesGeometry new_curves;
new_curves.resize(total_point_count, stroke_count);
bke::CurvesGeometry new_curves(total_point_count, stroke_count);
@ -5486,0 +5876,4 @@
point_opacities.finish();
stroke_materials.finish();
drawing.strokes_for_write() = new_curves;
std::move(new_curves)
@ -0,0 +157,4 @@
* 2) Shadow render buffer if 3rd stage reprojection is need for silhouette/lit/shaded region
* selection. Otherwise the shadow render buffer is deleted before this function returns.
*/
bool lineart_main_try_generate_shadow(struct Depsgraph *depsgraph,
Better to do this file rename in a separate commit afterwards
@ -10,9 +10,12 @@
#include <climits>
#include <cstdlib>
#include "BLI_math_base.hh"
Maybe someone has told you otherwise, but in this case it's better to just use
std::max
than include the full math base header.That is too sade the fact that a lot of pretty common bli headers (like
BLI_index_range.hh
) have includes of many std headers. 90% of std functions can be added in any.cc
file on whole blender even without check for required header and this will compile...Humm so does this mean it would be better for me to include std headers explicitly? Or leave it as-is?
In not really near future, modules would fix includes-leak issue, but for now you can just use std::max everywhere. I'd still would like to use internal blender wrappers.. but if this is another one additional header, and there is no any other such math functions to use...
I'll rename
MOD_lineart.h
andlineart_intern.h
into.hh
afterwards.Renamed all old
LRT_
flags intoLINEART_
and put them all insideDNA_modifier_types.h
so that the old gpencil dna file only includes the old modifier structure.Added some more comments.
@ -871,6 +871,76 @@ void BKE_modifier_path_init(char *path, int path_maxncpy, const char *name)
BLI_path_join(path, path_maxncpy, blendfile_path[0] ? "//" : BKE_tempdir_session(), name);
}
GreasePencilLineartLimitInfo BKE_grease_pencil_get_lineart_modifier_limits(const Object *ob)
Should be a reference here, since the function expects it to be
!= nullptr
:const Object &object
@ -874,0 +893,4 @@
return info;
}
void BKE_grease_pencil_set_lineart_modifier_limits(ModifierData *md,
Seems like this function should just get a
GreasePencilLineartModifierData &lmd
directly.@ -874,0 +894,4 @@
}
void BKE_grease_pencil_set_lineart_modifier_limits(ModifierData *md,
const GreasePencilLineartLimitInfo *info,
const GreasePencilLineartLimitInfo &info
@ -874,0 +931,4 @@
return false;
}
GreasePencilLineartModifierData *BKE_grease_pencil_get_first_lineart_modifier(const Object *ob)
const Object &object
@ -5486,0 +5796,4 @@
stroke_count++;
/* TODO: Debug random color feature not implemented for v3 yet. */
Is this still true?
Yes... But I haven't been really needing that for debug purposes... Also I'm not sure if GPv3 support vertex colors?
We do have vertex colors.
Do we have a equivalent of
BKE_gpencil_stroke_set_random_color
for v3? Or should I make a shared one somewhere? Looks like the original function is only needed inside old line art, so I think it's okay we don't include this thing since I don't really use it for debug anyway.@ -0,0 +733,4 @@
/* Ensure we have a frame in the selected layer to put line art result in. */
bke::greasepencil::Layer &layer = node->as_layer();
layer.add_frame(0, 0, 0);
This line shouldn't be necessary. The logic below will ensure a keyframe exists.
@ -0,0 +759,4 @@
lmd.mask_switches,
lmd.material_mask_bits,
lmd.intersection_mask,
(float)lmd.thickness / 1000,
float(lmd.thickness) / 1000.0f
@ -11,6 +11,7 @@ set(INC
../draw
../functions
../gpencil_modifiers_legacy
../gpencil_modifiers_legacy/intern/lineart
These headers should be moved to a more visible place instead of adding internal folders as include paths.
@ -869,1 +870,3 @@
void MOD_lineart_destroy_render_data(struct LineartGpencilModifierData *lmd);
void MOD_lineart_wrap_modifier_v3(const LineartGpencilModifierData *lmd_legacy,
GreasePencilLineartModifierData *lmd);
void MOD_lineart_unwrap_modifier_v3(LineartGpencilModifierData *lmd_legacy,
Why have GPv3 functions in a legacy header, wouldn't it make more sense to move these into a new header?
Because the line art calculation code is shared between the two atm and those wrapping function is needed to pass data between them.
Ok, should be easy to clean up later.
@ -2990,0 +3057,4 @@
typedef struct GreasePencilLineartModifierData {
ModifierData modifier;
GreasePencilModifierInfluenceData influence;
I think the line art modifier shouldn't be using this influence struct. The only thing it uses is the layer name, which can easily be a regular property. It doesn't utilize any of the other utility functions that make using this struct convenient for other modifiers.
@ -0,0 +793,4 @@
if (is_first_lineart) {
mmd->shared_cache = MOD_lineart_init_cache();
info = BKE_grease_pencil_get_lineart_modifier_limits(ctx->object);
LISTBASE_FOREACH (ModifierData *, imd, &ctx->object->modifiers) {
There is no need for this loop. You can just put this above the
if
I'm not sure tho, this needs to set limit info for all line art modifiers.
Yes, that's what it will do. When each lineart modifier executes, it sets the info first.
Wouldn't it be kinda unnecessary since the idea is that it gets the limits once and set for all modifiers, and this way each modifier had to get the limit one more time 😅.
I now put that in the cache, it should work fine now.
@ -58,6 +58,10 @@
#include "MEM_guardedalloc.h"
/* For Line Art modifier cache calls. */
These are no longer needed. in fact the whole file can just be reverted
git checkout main source/blender/blenkernel/intern/grease_pencil.cc
@ -874,0 +879,4 @@
if (md->type == eModifierType_GreasePencilLineart) {
const auto *lmd = reinterpret_cast<const GreasePencilLineartModifierData *>(md);
if (is_first || (lmd->flags & MOD_LINEART_USE_CACHE)) {
info.min_level = std::min(int(info.min_level), int(lmd->level_start));
std::min(int(info.min_level), int(lmd->level_start));
->
std::min<int>(info.min_level, lmd->level_start);
@ -0,0 +398,4 @@
const int level_start = RNA_int_get(ptr, "level_start");
const int level_end = RNA_int_get(ptr, "level_end");
if (use_multiple_levels) {
return (std::max(level_start, level_end) > 0);
return (std::max(level_start, level_end) > 0);
->
return std::max(level_start, level_end) > 0;
@blender-bot build
Some more cleanup. I think this is getting close.
@ -56,3 +65,4 @@
#include <algorithm> /* For `min/max`. */
using blender::int3;
using namespace blender;
Where is this needed?
The curve generation part, it has
bke
and stuff inblender
.I guess I'll modify that to not use blender namespace globally.
@ -8184,0 +8270,4 @@
RNA_define_lib_overridable(true);
prop = RNA_def_property(srna, "use_custom_camera", PROP_BOOLEAN, PROP_NONE);
RNA_def_property_boolean_sdna(prop, nullptr, "calculation_flags", LRT_USE_CUSTOM_CAMERA);
Shouldn't the flags here be also renamed?
LINEART_USE_CUSTOM_CAMERA
?@ -0,0 +85,4 @@
return false;
}
ModifierData *imd = md.modifier.prev;
while (imd) {
imd != nullptr
@ -0,0 +100,4 @@
return false;
}
ModifierData *imd = md.modifier.next;
while (imd) {
imd != nullptr
@ -0,0 +158,4 @@
const ModifierUpdateDepsgraphContext *ctx,
const int mode)
{
if (!c) {
Seems like the argument should be
Collection &collection
and the caller should do this check.@blender-bot build
Only got 2 comments, but this doesn't need another review round from me. Accepting.
@ -1811,0 +1819,4 @@
{
GreasePencilLineartModifierData *lmd = (GreasePencilLineartModifierData *)ptr->data;
CLAMP(value, 0, 128);
math::clamp
(orstd::clamp
) should be used here@ -1811,0 +1828,4 @@
{
GreasePencilLineartModifierData *lmd = (GreasePencilLineartModifierData *)ptr->data;
CLAMP(value, 0, 128);
Same as above
Found a bug while testing.
@ -5486,0 +5854,4 @@
int point_i = i + up_to_point;
float *point = (float *)&point_positions[point_i];
copy_v3_v3(point, eci->gpos);
point_radii.span[point_i] = thickness;
This is not correct. Since we're writing to the radii, this needs to be
thickness / 2.0f
.Huh so now it's actually treating it as
2r
instead ofr
? Interesting...thickness
was2 * radius
before. It's just that we're storing the radius instead of the diameter now.@ -940,0 +968,4 @@
const int8_t source_type,
Object *source_object,
struct Collection *source_collection,
const int level_start,
const int level_start
->int level_start
This and the same for all other by-value trivial small types in forward declaration.