GPv3: Noise modifier #117057

Merged
Falk David merged 42 commits from ChengduLittleA/blender:gp3-noise-modifier into main 2024-01-29 16:49:27 +01:00
Member

Ported stroke random effect modifier to GPv3.

Internally it uses new c++ math and curves API so it's cleaner.

图片

Ported stroke random effect modifier to GPv3. Internally it uses new c++ math and curves API so it's cleaner. ![图片](/attachments/f6742393-6149-4120-b791-6881fff3e64f)
YimingWu added the
Interest
Grease Pencil
Module
Grease Pencil
labels 2024-01-12 09:42:40 +01:00
YimingWu added 5 commits 2024-01-12 09:42:45 +01:00
YimingWu changed title from gp3-noise-modifier to WIP: GPv3: Noise modifier 2024-01-12 09:43:34 +01:00
YimingWu added this to the Grease Pencil project 2024-01-12 09:51:38 +01:00
YimingWu added 2 commits 2024-01-16 02:53:05 +01:00
YimingWu changed title from WIP: GPv3: Noise modifier to GPv3: Noise modifier 2024-01-17 08:49:32 +01:00
YimingWu added 2 commits 2024-01-17 08:49:36 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
106e49a792
Noise ok
Falk David requested review from Falk David 2024-01-17 10:54:55 +01:00
Member

@blender-bot build

@blender-bot build
Member

@ChengduLittleA Please run make format. The linter is currently failing.

@ChengduLittleA Please run `make format`. The linter is currently failing.

Overall work fine, just notice you put Randomize panel inside Influence panel. It should go above in their own panel.

image

Overall work fine, just notice you put Randomize panel inside Influence panel. It should go above in their own panel. ![image](/attachments/bcfa1cec-012d-4007-a7ac-74984cc1938b)
8.8 KiB
YimingWu added 1 commit 2024-01-17 13:26:38 +01:00
Author
Member

Ah looks like it's because of this sub-panel thing being kind of different than the in-place "Influence" panel.

@LukasTonne What do you suggest we do here? A lot of modifiers will have this kind of sub panels, and they will all go to the bottom of the modifier. Having a flag property dedicated to a lot of UI panels doesn't sound that brilliant...

Ah looks like it's because of this sub-panel thing being kind of different than the in-place "Influence" panel. @LukasTonne What do you suggest we do here? A lot of modifiers will have this kind of sub panels, and they will all go to the bottom of the modifier. Having a flag property dedicated to a lot of UI panels doesn't sound that brilliant...
Member

You should just use the uiLayoutPanel function to create the Randomize subpanel, instead of the old modifier_subpanel_register. Only limitation atm is that you can't draw a header with buttons, so for the time being i think the Randomize checkbox will have to be drawn in a separate row above.

You should just use the `uiLayoutPanel` function to create the Randomize subpanel, instead of the old `modifier_subpanel_register`. Only limitation atm is that you can't draw a header with buttons, so for the time being i think the Randomize checkbox will have to be drawn in a separate row above.

Separate the checkbox from the header seems an UI regression for me. Is this something that is going to be fixed soon?

Separate the checkbox from the header seems an UI regression for me. Is this something that is going to be fixed soon?
Member

Separate the checkbox from the header seems an UI regression for me. Is this something that is going to be fixed soon?

@deadpin is looking into panel header layouts

> Separate the checkbox from the header seems an UI regression for me. Is this something that is going to be fixed soon? @deadpin is looking into panel header layouts
Author
Member

I now made it like this:

图片

I now made it like this: ![图片](/attachments/1a3afa8d-5ba4-4675-9822-c6a74ad3b33d)
YimingWu added 1 commit 2024-01-17 14:04:23 +01:00
YimingWu added 1 commit 2024-01-17 14:05:06 +01:00
Falk David requested changes 2024-01-17 14:16:01 +01:00
Falk David left a comment
Member

I added some cleanup comments in the code, but I also have some higher-level feedback:

Calling e.g. drawing->radii_for_write(); will create a copy if the array is implicitly shared. This should be avoided.
I suggest to restructure the code like so:

  • Add checks for the mmd->factor_*
    • For each check, get the data you need, e.g. the attribute array, the noise table (no need to compute a table for each stroke, just compute one for all the points).
    • Loop with filtered_strokes.foreach_index
    • Compute seed, then noise, then write to the attribute
I added some cleanup comments in the code, but I also have some higher-level feedback: Calling e.g. `drawing->radii_for_write();` will create a copy if the array is implicitly shared. This should be avoided. I suggest to restructure the code like so: * Add checks for the `mmd->factor_*` * For each check, get the data you need, e.g. the attribute array, the noise table (no need to compute a table for each stroke, just compute one for all the points). * Loop with `filtered_strokes.foreach_index` * Compute seed, then noise, then write to the attribute
@ -2486,6 +2487,7 @@ typedef enum VolumeToMeshFlag {
VOLUME_TO_MESH_USE_SMOOTH_SHADE = 1 << 0,
} VolumeToMeshFlag;
/* Enum in old dna file. */
Member

What is this comment for?

What is this comment for?
Author
Member

Seems to be a merge duplicate of Opacity modifier, will remove

Seems to be a merge duplicate of Opacity modifier, will remove
ChengduLittleA marked this conversation as resolved
@ -7615,3 +7621,3 @@
PropertyRNA *prop;
static const EnumPropertyItem color_mode_items[] = {
static const EnumPropertyItem color_mode_items[] = {
Member

Formatting issue

Formatting issue
ChengduLittleA marked this conversation as resolved
@ -0,0 +58,4 @@
#include "DEG_depsgraph.hh"
#include "DEG_depsgraph_query.hh"
using namespace blender;
Member

All these functions should be in the namesspace blender

All these functions should be in the namesspace `blender`
ChengduLittleA marked this conversation as resolved
@ -0,0 +106,4 @@
return (mmd->flag & GP_NOISE_USE_RANDOM) != 0;
}
static float *noise_table(int len, int offset, int seed)
Member

Instead of using MEM_callocN and returning a pointer, this should build an Array<float>.

Instead of using `MEM_callocN` and returning a pointer, this should build an `Array<float>`.
ChengduLittleA marked this conversation as resolved
@ -0,0 +126,4 @@
static void deform_stroke(ModifierData *md,
Depsgraph *depsgraph,
Object *ob,
bke::greasepencil::Drawing *drawing)
Member

Pass these parameters by reference e.g. bke::greasepencil::Drawing &drawing. When the function expects these to be not nullptr, it's better to use references to ensure they exist.

Pass these parameters by reference e.g. `bke::greasepencil::Drawing &drawing`. When the function expects these to be not `nullptr`, it's better to use references to ensure they exist.
ChengduLittleA marked this conversation as resolved
@ -0,0 +136,4 @@
GreasePencilNoiseModifierData *mmd = (GreasePencilNoiseModifierData *)md;
/* Noise value in range [-1..1] */
float3 vec1, vec2;
const bool use_curve = (mmd->influence.flag & GREASE_PENCIL_INFLUENCE_USE_CUSTOM_CURVE);
Member

(mmd->influence.flag & GREASE_PENCIL_INFLUENCE_USE_CUSTOM_CURVE) != 0

`(mmd->influence.flag & GREASE_PENCIL_INFLUENCE_USE_CUSTOM_CURVE) != 0`
ChengduLittleA marked this conversation as resolved
@ -0,0 +141,4 @@
const bool is_keyframe = (mmd->noise_mode == GP_NOISE_RANDOM_KEYFRAME);
/* Sanitize as it can create out of bound reads. */
float noise_scale = clamp_f(mmd->noise_scale, 0.0f, 1.0f);
Member

use math::clamp

use `math::clamp`
ChengduLittleA marked this conversation as resolved
@ -0,0 +148,4 @@
MutableSpan<float3> positions = strokes.positions_for_write();
MutableSpan<float> radiis = drawing->radii_for_write();
MutableSpan<float> opacities = drawing->opacities_for_write();
OffsetIndices<int> curves = strokes.points_by_curve();
Member

curves -> points_by_curve

`curves` -> `points_by_curve`
ChengduLittleA marked this conversation as resolved
@ -0,0 +221,4 @@
}
/* Vector orthogonal to normal. */
vec2 = math::cross(vec1, normals[point]);
Member

This whole calculation of vec2 should be replaced with

const float3 up_vector = math::normalize(math::cross(tangents[point], normals[point])); 

where tangents and normals are retrieved like so:

const Span<float3> tangents = strokes.evaluated_tangents();
const Span<float3> normals = drawing.curve_plane_normals();

and then up_vector is used when writing to positions.

This whole calculation of `vec2` should be replaced with ``` const float3 up_vector = math::normalize(math::cross(tangents[point], normals[point])); ``` where `tangents` and `normals` are retrieved like so: ``` const Span<float3> tangents = strokes.evaluated_tangents(); const Span<float3> normals = drawing.curve_plane_normals(); ``` and then `up_vector` is used when writing to `positions`.
ChengduLittleA marked this conversation as resolved
@ -0,0 +232,4 @@
if (mmd->factor_thickness > 0.0f) {
float noise = table_sample(noise_table_thickness,
local_point * noise_scale + fractf(mmd->noise_offset));
radiis[point] *= max_ff(1.0f + (noise * 2.0f - 1.0f) * weight * mmd->factor_thickness, 0.0f);
Member

Use math::max. Same below.

Use `math::max`. Same below.
ChengduLittleA marked this conversation as resolved
@ -0,0 +233,4 @@
float noise = table_sample(noise_table_thickness,
local_point * noise_scale + fractf(mmd->noise_offset));
radiis[point] *= max_ff(1.0f + (noise * 2.0f - 1.0f) * weight * mmd->factor_thickness, 0.0f);
CLAMP_MIN(radiis[point], GPENCIL_STRENGTH_MIN);
Member

This clamp is no longer needed

This clamp is no longer needed
ChengduLittleA marked this conversation as resolved
@ -0,0 +240,4 @@
float noise = table_sample(noise_table_strength,
local_point * noise_scale + fractf(mmd->noise_offset));
opacities[point] *= max_ff(1.0f - noise * weight * mmd->factor_strength, 0.0f);
CLAMP(radiis[point], GPENCIL_STRENGTH_MIN, 1.0f);
Member

This clamp should not be needed (and it's clamping the radii atm.)

This clamp should not be needed (and it's clamping the radii atm.)
ChengduLittleA marked this conversation as resolved
Falk David reviewed 2024-01-17 14:40:39 +01:00
@ -0,0 +147,4 @@
/* Calculate or get stroke normals. */
Span<float3> normals = strokes.evaluated_normals();
MutableSpan<float3> positions = strokes.positions_for_write();
MutableSpan<float> radiis = drawing->radii_for_write();
Member

radii (it's already the plural of radius)

`radii` (it's already the plural of `radius`)
ChengduLittleA marked this conversation as resolved
Author
Member

Now i generate a noise table for all strokes. The side effect of this is that the noise table is continuous along separated strokes, and depending on how you are animating, the scrolling effect may or may not be visible.

Now i generate a noise table for all strokes. The side effect of this is that the noise table is continuous along separated strokes, and depending on how you are animating, the scrolling effect may or may not be visible.
YimingWu added 1 commit 2024-01-17 14:55:31 +01:00
YimingWu added 1 commit 2024-01-17 14:56:28 +01:00
Falk David requested changes 2024-01-17 15:04:44 +01:00
Falk David left a comment
Member

Please only mark things as resolved when they are actually resolved 😅 Otherwise I'll have no idea what was changed.

Please only mark things as resolved when they are actually resolved 😅 Otherwise I'll have no idea what was changed.
@ -0,0 +243,4 @@
blender::bke::GeometrySet *geometry_set)
{
GreasePencilNoiseModifierData *mmd = (GreasePencilNoiseModifierData *)md;
GreasePencil *gp = geometry_set->get_grease_pencil_for_write();
Member

Looks like you didn't make the changes here, but marked my request as resolved.

Looks like you didn't make the changes here, but marked my request as resolved.
Member

Ah it seems like I got confused with the review of the other modifier. Same feedback applies here though.

Ah it seems like I got confused with the review of the other modifier. Same feedback applies here though.
Author
Member

Ah that was from the subdiv PR. I saw it later 😅

Ah that was from the subdiv PR. I saw it later 😅
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-17 15:14:45 +01:00
YimingWu force-pushed gp3-noise-modifier from c0cc12d33c to 196f1f4524 2024-01-17 15:22:47 +01:00 Compare
YimingWu force-pushed gp3-noise-modifier from 196f1f4524 to e1332951a9 2024-01-17 15:24:36 +01:00 Compare
Falk David requested changes 2024-01-17 15:25:04 +01:00
@ -1135,3 +1132,1 @@
const Span<int> offsets,
const Span<int> materials,
const float4x4 &matrix)
bke::CurvesGeometry create_drawing_data(const Span<float3> positions,
Member

This still needs to be removed.

This still needs to be removed.
ChengduLittleA marked this conversation as resolved
@ -200,6 +200,13 @@ IndexMask retrieve_editable_and_selected_elements(Object &object,
bke::AttrDomain selection_domain,
IndexMaskMemory &memory);
bke::CurvesGeometry create_drawing_data(const Span<float3> positions,
Member

Same as above.

Same as above.
ChengduLittleA marked this conversation as resolved
@ -17,6 +17,7 @@ set(INC
../render
../windowmanager
../../../intern/eigen
../gpencil_modifiers_legacy
Member

Not sure why this is needed 🤔

Not sure why this is needed 🤔
ChengduLittleA marked this conversation as resolved
@ -0,0 +63,4 @@
static void init_data(ModifierData *md)
{
GreasePencilNoiseModifierData *gpmd = (GreasePencilNoiseModifierData *)md;
Member

C++ style casts, same for the rest of the file.

C++ style casts, same for the rest of the file.
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-17 15:32:43 +01:00
YimingWu added 2 commits 2024-01-18 06:51:33 +01:00
YimingWu added 1 commit 2024-01-18 08:52:16 +01:00
Falk David requested changes 2024-01-18 10:33:26 +01:00
Falk David left a comment
Member

I see that you went a little into the direction that I was thinking, but let me just write it out o make it more clear.
This is what I had in mind:

if (mmd.factor_thickness > 0.0f) {
  MutableSpan<float> radii = drawing.radii_for_write();
  const Array<float> noise_table_thickness = noise_table(noise_len, int(floor(mmd.noise_offset)), seed);
  filtered_strokes.foreach_index([&](const int stroke_i) {
    const IndexRange points = points_by_curve[stroke_i];
    for (const int point_i : points) {
      float weight = 1.0f;
      if (use_curve) {
        float value = float(point_i - points.first()) / (points.size() - 1);
        weight *= BKE_curvemapping_evaluateF(mmd.influence.custom_curve, 0, value);
      }
      const float noise = table_sample(noise_table_thickness,
                                  point_i * noise_scale + fractf(mmd.noise_offset));
      radii[point_i] *= math::max(1.0f + (noise * 2.0f - 1.0f) * weight * mmd.factor_thickness,
                                0.0f);
    }
  });
}

And you would do position, opacity, etc. each in their own loop. This means that we only ever change the attributes that we need to.

I see that you went a little into the direction that I was thinking, but let me just write it out o make it more clear. This is what I had in mind: ``` if (mmd.factor_thickness > 0.0f) { MutableSpan<float> radii = drawing.radii_for_write(); const Array<float> noise_table_thickness = noise_table(noise_len, int(floor(mmd.noise_offset)), seed); filtered_strokes.foreach_index([&](const int stroke_i) { const IndexRange points = points_by_curve[stroke_i]; for (const int point_i : points) { float weight = 1.0f; if (use_curve) { float value = float(point_i - points.first()) / (points.size() - 1); weight *= BKE_curvemapping_evaluateF(mmd.influence.custom_curve, 0, value); } const float noise = table_sample(noise_table_thickness, point_i * noise_scale + fractf(mmd.noise_offset)); radii[point_i] *= math::max(1.0f + (noise * 2.0f - 1.0f) * weight * mmd.factor_thickness, 0.0f); } }); } ``` And you would do position, opacity, etc. each in their own loop. This means that we only ever change the attributes that we need to.
Author
Member

Huh I think I get it now. That way you can reduce the number of branches so the performance can be a bit better I guess? :D

Huh I think I get it now. That way you can reduce the number of branches so the performance can be a bit better I guess? :D
Member

Right, and it makes it clear what branch is modifiying what attribute :)

Right, and it makes it clear what branch is modifiying what attribute :)
YimingWu added 1 commit 2024-01-18 10:50:47 +01:00
Falk David requested changes 2024-01-18 11:22:09 +01:00
Falk David left a comment
Member

Some more comments

Some more comments
@ -0,0 +136,4 @@
&ob, strokes, mmd.influence, memory);
/* Noise value in range [-1..1] */
float3 up_vector;
Member

No need for this declaration.

No need for this declaration.
ChengduLittleA marked this conversation as resolved
@ -0,0 +148,4 @@
return;
}
OffsetIndices<int> points_by_curve = strokes.points_by_curve();
Member

Use const

Use `const`
ChengduLittleA marked this conversation as resolved
@ -0,0 +166,4 @@
}
int noise_len = ceilf(strokes.points_num() * noise_scale) + 2;
if (mmd.factor) {
Member

Since these are floats, better to write as mmd.factor > 0.0f (otherwise it looks like they should be booleans when reading the code). Same for the others.

Since these are floats, better to write as `mmd.factor > 0.0f` (otherwise it looks like they should be booleans when reading the code). Same for the others.
ChengduLittleA marked this conversation as resolved
@ -0,0 +174,4 @@
noise_len, int(floor(mmd.noise_offset)), seed + 2);
filtered_strokes.foreach_index([&](const int stroke_i) {
int point_count = points_by_curve[stroke_i].size();
Member

Usually we iterate over the actual point indices and calculate the local point as needed. This makes it a bit easier to read (and with my suggested change below, local_point is not needed).

const IndexRange points = points_by_curve[stroke_i];
for (const int point_i : points) {

Same for the other loops below.

Usually we iterate over the actual point indices and calculate the local point as needed. This makes it a bit easier to read (and with my suggested change below, `local_point` is not needed). ``` const IndexRange points = points_by_curve[stroke_i]; for (const int point_i : points) { ``` Same for the other loops below.
ChengduLittleA marked this conversation as resolved
@ -0,0 +177,4 @@
int point_count = points_by_curve[stroke_i].size();
for (const int local_point : points_by_curve[stroke_i].index_range()) {
int point = local_point + points_by_curve[stroke_i].start();
float weight = 1.0f;
Member

Seems like a good idea to put this weight calculation in a lambda (so it can be reused in the other loops as well):

auto get_weight = [&](const IndexRange points, const int point_i) {
    if (!use_curve) {
        return 1.0f;
    }
    const float value = float(point_i - points.start()) / float(points.size() - 1);
    return BKE_curvemapping_evaluateF(mmd.influence.custom_curve, 0, value);
};

You can put this above if (mmd.factor > 0.0f) { and then call it like so:

const float weight = get_weight(points, point_i);

Use it in the other loops below too

Seems like a good idea to put this weight calculation in a lambda (so it can be reused in the other loops as well): ``` auto get_weight = [&](const IndexRange points, const int point_i) { if (!use_curve) { return 1.0f; } const float value = float(point_i - points.start()) / float(points.size() - 1); return BKE_curvemapping_evaluateF(mmd.influence.custom_curve, 0, value); }; ``` You can put this above `if (mmd.factor > 0.0f) {` and then call it like so: ``` const float weight = get_weight(points, point_i); ``` Use it in the other loops below too
ChengduLittleA marked this conversation as resolved
@ -0,0 +183,4 @@
weight *= BKE_curvemapping_evaluateF(mmd.influence.custom_curve, 0, value);
}
/* Vector orthogonal to normal. */
up_vector = math::normalize(math::cross(tangents[point], normals[point]));
Member

This should be const float3 bi_normal

This should be `const float3 bi_normal`
ChengduLittleA marked this conversation as resolved
@ -0,0 +184,4 @@
}
/* Vector orthogonal to normal. */
up_vector = math::normalize(math::cross(tangents[point], normals[point]));
float noise = table_sample(noise_table_position,
Member

Use const (same for the other loops)

Use `const` (same for the other loops)
ChengduLittleA marked this conversation as resolved
@ -0,0 +188,4 @@
point * noise_scale + fractf(mmd.noise_offset));
positions[point] += up_vector * (noise * 2.0f - 1.0f) * weight * mmd.factor * 0.1f;
}
});
Member

We should call drawing.tag_positions_changed(); after this line.

We should call `drawing.tag_positions_changed();` after this line.
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-18 11:57:36 +01:00
Falk David requested changes 2024-01-18 12:12:54 +01:00
Falk David left a comment
Member

We're almost there with this one too! I'll test this locally as well. Some more cleanup comments.

We're almost there with this one too! I'll test this locally as well. Some more cleanup comments.
@ -0,0 +181,4 @@
filtered_strokes.foreach_index([&](const int stroke_i) {
const IndexRange points = points_by_curve[stroke_i];
for (const int point : points) {
Member

Since we're using stroke_i, let's use point_i to be consistent. Same for the other loops.

Since we're using `stroke_i`, let's use `point_i` to be consistent. Same for the other loops.
ChengduLittleA marked this conversation as resolved
@ -0,0 +187,4 @@
const float3 bi_normal = math::normalize(math::cross(tangents[point], normals[point]));
const float noise = table_sample(noise_table_position,
point * noise_scale + fractf(mmd.noise_offset));
positions[point] += up_vector * (noise * 2.0f - 1.0f) * weight * mmd.factor * 0.1f;
Member

up_vector -> bi_normal

`up_vector` -> `bi_normal`
ChengduLittleA marked this conversation as resolved
@ -0,0 +248,4 @@
int current_frame = DEG_get_evaluated_scene(ctx->depsgraph)->r.cfra;
IndexMaskMemory mask_memory;
IndexMask layer_mask = modifier::greasepencil::get_filtered_layer_mask(
Member

Can be const

Can be `const`
ChengduLittleA marked this conversation as resolved
Falk David reviewed 2024-01-18 12:20:24 +01:00
@ -0,0 +199,4 @@
noise_len, int(floor(mmd.noise_offset)), seed);
filtered_strokes.foreach_index([&](const int stroke_i) {
int point_count = points_by_curve[stroke_i].size();
Member

Unused variable

Unused variable
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-18 12:51:28 +01:00
YimingWu added 2 commits 2024-01-18 13:33:23 +01:00
Falk David requested changes 2024-01-18 14:32:13 +01:00
@ -0,0 +111,4 @@
return table;
}
BLI_INLINE float table_sample(Array<float> &table, float x)
Member

No need for the BLI_INLINE, the compiler should do it anyway.

No need for the `BLI_INLINE`, the compiler should do it anyway.
ChengduLittleA marked this conversation as resolved
@ -0,0 +158,4 @@
}
else {
/* If change every keyframe, use the last keyframe. */
/* TODO: not correct. */
Member

Looks like this is still to be done? I think we need to just pass the frame number into this function.

Looks like this is still to be done? I think we need to just pass the frame number into this function.
Author
Member

This used to be seed += gpf->framenum;, but now we are operating on a drawing which doesn't have frame info... How should we approach this?

This used to be `seed += gpf->framenum;`, but now we are operating on a `drawing` which doesn't have frame info... How should we approach this?
Member

Hm we can pass the frame number from the keyframe that the drawing came from. It would mean that we need another variant of get_drawings_for_write. I can take care of that.

Hm we can pass the frame number from the keyframe that the drawing came from. It would mean that we need another variant of `get_drawings_for_write`. I can take care of that.
ChengduLittleA marked this conversation as resolved
Falk David requested changes 2024-01-19 15:02:15 +01:00
Falk David left a comment
Member

There is a function get_drawing_infos_for_write now so you can get the frame numbers.

There is a function `get_drawing_infos_for_write` now so you can get the frame numbers.
@ -0,0 +122,4 @@
static void deform_drawing(ModifierData &md,
Depsgraph *depsgraph,
Object &ob,
bke::greasepencil::Drawing &drawing)
Member

Add const int start_frame_number

Add `const int start_frame_number`
ChengduLittleA marked this conversation as resolved
@ -0,0 +250,4 @@
const IndexMask layer_mask = modifier::greasepencil::get_filtered_layer_mask(
grease_pencil, mmd->influence, mask_memory);
const Vector<bke::greasepencil::Drawing *> drawings =
modifier::greasepencil::get_drawings_for_write(grease_pencil, layer_mask, current_frame);
Member

Use get_drawing_infos_for_write here.

Use `get_drawing_infos_for_write` here.
ChengduLittleA marked this conversation as resolved
@ -0,0 +252,4 @@
const Vector<bke::greasepencil::Drawing *> drawings =
modifier::greasepencil::get_drawings_for_write(grease_pencil, layer_mask, current_frame);
threading::parallel_for_each(drawings, [&](bke::greasepencil::Drawing *drawing) {
Member

This becomes

threading::parallel_for_each(drawing_infos, [&](DrawingInfo &info) {
  deform_drawing(*md, ctx->depsgraph, *ctx->object, *info.drawing, info.start_frame_number);
});
This becomes ``` threading::parallel_for_each(drawing_infos, [&](DrawingInfo &info) { deform_drawing(*md, ctx->depsgraph, *ctx->object, *info.drawing, info.start_frame_number); }); ```
ChengduLittleA marked this conversation as resolved
YimingWu added 3 commits 2024-01-21 14:50:27 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
1f5e2dbe26
Use DrawingInfo for handling start frame in randomize.
Member

@blender-bot build

@blender-bot build
Pratik Borhade reviewed 2024-01-22 13:22:37 +01:00
Pratik Borhade left a comment
Member

Hi, I also tested the PR and works great

Hi, I also tested the PR and works great
@ -7789,6 +7890,7 @@ static void rna_def_modifier_grease_pencil_subdiv(BlenderRNA *brna)
RNA_def_property_range(prop, 0, 65536);
RNA_def_property_ui_range(prop, 0, 32, 1, 0);
RNA_def_property_ui_text(prop, "Level", "Number of subdivisions");
Member

accidental change :)

accidental change :)
ChengduLittleA marked this conversation as resolved
@ -0,0 +215,4 @@
Array<float> noise_table_strength = noise_table(
noise_len, int(floor(mmd.noise_offset)), seed + 3);
filtered_strokes.foreach_index([&](const int stroke_i) {
Member

Guess calling foreach_index separately for writing each attribute would affect performance for big curves?
I'm not expert though

Current code looks clean BTW.

Guess calling `foreach_index` separately for writing each attribute would affect performance for big curves? I'm not expert though Current code looks clean BTW.
Member

It's mostly so that we can call the accessor for e.g. drawing.opacities_for_write(); seperatly and avoid copying the data (when it's shared). So this means that if you just use one of the factors, only that attribute would be touched.

It's mostly so that we can call the accessor for e.g. `drawing.opacities_for_write();` seperatly and avoid copying the data (when it's shared). So this means that if you just use one of the factors, only that attribute would be touched.
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-22 15:26:31 +01:00
Falk David approved these changes 2024-01-22 17:15:28 +01:00
Falk David left a comment
Member

This looks good to me. I added just a few cleanup comments for the math functions.

This looks good to me. I added just a few cleanup comments for the math functions.
@ -0,0 +111,4 @@
return table;
}
static float table_sample(Array<float> &table, float x)
Member

const float

`const float`
ChengduLittleA marked this conversation as resolved
@ -0,0 +113,4 @@
static float table_sample(Array<float> &table, float x)
{
return math::interpolate(table[int(ceilf(x))], table[int(floor(x))], fractf(x));
Member

should use math::ceil,math::floor and math::fract

should use `math::ceil`,`math::floor` and `math::fract`
ChengduLittleA marked this conversation as resolved
@ -0,0 +155,4 @@
seed += BLI_hash_string(md.name);
if (mmd.flag & GP_NOISE_USE_RANDOM) {
if (!is_keyframe) {
seed += cfra / mmd.step;
Member

math::floor(float(cfra) / float(mmd.step))

`math::floor(float(cfra) / float(mmd.step))`
Author
Member

This needs to be a integer division int(cfra) / mmd.step otherwise the seed is not correctly.

This needs to be a integer division `int(cfra) / mmd.step` otherwise the seed is not correctly.
ChengduLittleA marked this conversation as resolved
@ -0,0 +162,4 @@
seed += start_frame_number;
}
}
int noise_len = ceilf(strokes.points_num() * noise_scale) + 2;
Member

math:ceil

`math:ceil`
ChengduLittleA marked this conversation as resolved
@ -0,0 +186,4 @@
/* Vector orthogonal to normal. */
const float3 bi_normal = math::normalize(math::cross(tangents[point], normals[point]));
const float noise = table_sample(noise_table_position,
point * noise_scale + fractf(mmd.noise_offset));
Member

math::fract

`math::fract`
ChengduLittleA marked this conversation as resolved
@ -0,0 +203,4 @@
for (const int point : points) {
const float weight = get_weight(points, point);
const float noise = table_sample(noise_table_thickness,
point * noise_scale + fractf(mmd.noise_offset));
Member

math::fract

`math::fract`
ChengduLittleA marked this conversation as resolved
@ -0,0 +220,4 @@
for (const int point : points) {
const float weight = get_weight(points, point);
const float noise = table_sample(noise_table_strength,
point * noise_scale + fractf(mmd.noise_offset));
Member

math::fract

`math::fract`
ChengduLittleA marked this conversation as resolved
Hans Goudey requested changes 2024-01-22 17:49:06 +01:00
@ -0,0 +6,4 @@
* \ingroup modifiers
*/
#include "BLI_hash.h"
Member

Next time, please check for unused includes. I can comment out all of these and the file still compiles:

#include "BLI_hash.h"
// #include "BLI_math_matrix.hh"
#include "BLI_math_vector.hh"
// #include "BLI_task.h"
// #include "BLI_utildefines.h"

#include "BLT_translation.h"

#include "BLO_read_write.hh"

#include "DNA_defaults.h"
// #include "DNA_gpencil_legacy_types.h"
#include "DNA_gpencil_modifier_types.h"
// #include "DNA_material_types.h"
// #include "DNA_meshdata_types.h"
// #include "DNA_object_types.h"
// #include "DNA_scene_types.h"
// #include "DNA_screen_types.h"

#include "BKE_colortools.hh"
// #include "BKE_context.hh"
#include "BKE_curves.hh"
// #include "BKE_curves_utils.hh"
#include "BKE_geometry_set.hh"
#include "BKE_grease_pencil.hh"
// #include "BKE_lib_query.hh"
// #include "BKE_modifier.hh"
// #include "BKE_screen.hh"

#include "UI_interface.hh"
#include "UI_resources.hh"

// #include "ED_grease_pencil.hh"

#include "MOD_grease_pencil_util.hh"
#include "MOD_modifiertypes.hh"
#include "MOD_ui_common.hh"

// #include "MEM_guardedalloc.h"

#include "RNA_access.hh"
#include "RNA_prototypes.h"

// #include "DEG_depsgraph.hh"
// #include "DEG_depsgraph_query.hh"
Next time, please check for unused includes. I can comment out all of these and the file still compiles: ```cpp #include "BLI_hash.h" // #include "BLI_math_matrix.hh" #include "BLI_math_vector.hh" // #include "BLI_task.h" // #include "BLI_utildefines.h" #include "BLT_translation.h" #include "BLO_read_write.hh" #include "DNA_defaults.h" // #include "DNA_gpencil_legacy_types.h" #include "DNA_gpencil_modifier_types.h" // #include "DNA_material_types.h" // #include "DNA_meshdata_types.h" // #include "DNA_object_types.h" // #include "DNA_scene_types.h" // #include "DNA_screen_types.h" #include "BKE_colortools.hh" // #include "BKE_context.hh" #include "BKE_curves.hh" // #include "BKE_curves_utils.hh" #include "BKE_geometry_set.hh" #include "BKE_grease_pencil.hh" // #include "BKE_lib_query.hh" // #include "BKE_modifier.hh" // #include "BKE_screen.hh" #include "UI_interface.hh" #include "UI_resources.hh" // #include "ED_grease_pencil.hh" #include "MOD_grease_pencil_util.hh" #include "MOD_modifiertypes.hh" #include "MOD_ui_common.hh" // #include "MEM_guardedalloc.h" #include "RNA_access.hh" #include "RNA_prototypes.h" // #include "DEG_depsgraph.hh" // #include "DEG_depsgraph_query.hh" ```
Author
Member

For some reason I forgot to do this on this modifier...

The DEG_depsgraph_query.hh is needed for DEG_get_ctime(), which is needed beside drawing key frame for the input of randomization. I'll keep it there.

For some reason I forgot to do this on this modifier... The `DEG_depsgraph_query.hh` is needed for `DEG_get_ctime()`, which is needed beside drawing key frame for the input of randomization. I'll keep it there.
Member

Right, for that I thought it could be accessed with the current_frame variable instead.

Right, for that I thought it could be accessed with the `current_frame` variable instead.
Author
Member

Right... I thought it was two different thing. After testing it seems to be matching the time from depsgraph.

Right... I thought it was two different thing. After testing it seems to be matching the time from depsgraph.
ChengduLittleA marked this conversation as resolved
@ -0,0 +120,4 @@
* Apply noise effect based on stroke direction.
*/
static void deform_drawing(ModifierData &md,
Depsgraph *depsgraph,
Member

I think you can pass current_frame from the caller instead of the depsgraph here

I think you can pass `current_frame` from the caller instead of the depsgraph here
ChengduLittleA marked this conversation as resolved
@ -0,0 +122,4 @@
static void deform_drawing(ModifierData &md,
Depsgraph *depsgraph,
const int start_frame_number,
Object &ob,
Member

Use const and references where possible

Use const and references where possible
ChengduLittleA marked this conversation as resolved
@ -0,0 +164,4 @@
}
int noise_len = ceilf(strokes.points_num() * noise_scale) + 2;
auto get_weight = [&](const IndexRange points, const int point_i) {
Member

int noise_len -> const int noise_len

`int noise_len` -> `const int noise_len`
ChengduLittleA marked this conversation as resolved
@ -0,0 +173,4 @@
};
if (mmd.factor > 0.0f) {
Span<float3> normals = strokes.evaluated_normals();
Member

Span -> const Span

`Span` -> `const Span`
ChengduLittleA marked this conversation as resolved
@ -0,0 +179,4 @@
Array<float> noise_table_position = noise_table(
noise_len, int(floor(mmd.noise_offset)), seed + 2);
filtered_strokes.foreach_index([&](const int stroke_i) {
Member

Might as well parallelize these loops with GrainSize(512) or so

Might as well parallelize these loops with `GrainSize(512)` or so
ChengduLittleA marked this conversation as resolved
@ -0,0 +182,4 @@
filtered_strokes.foreach_index([&](const int stroke_i) {
const IndexRange points = points_by_curve[stroke_i];
for (const int point : points) {
float weight = get_weight(points, point);
Member

Rather than iterating over points directly and subtracting point - points.start() to get the index in the curve, iterate over const int i : points.index_range() and then const int point = points[i]

That avoids the indirection and unnecessary step of subtraction, and it generally done elsewhere too

Rather than iterating over `points` directly and subtracting `point - points.start()` to get the index in the curve, iterate over `const int i : points.index_range()` and then `const int point = points[i]` That avoids the indirection and unnecessary step of subtraction, and it generally done elsewhere too
Member

Heh, that's what @ChengduLittleA had there before, my bad..

Heh, that's what @ChengduLittleA had there before, my bad..
Member

Haha, oops. Good that we're on the same page now anyway I guess!

Haha, oops. Good that we're on the same page now anyway I guess!
Author
Member

Wait I'm not sure I understand... Here in the loop we don't seem to need that point - points.start() value at all, we seem to just need point in the global index...?

If I'm understanding this correctly then we should do:

for(const int i : points.index_range()){
    const int point = points[i];
    // but apparently we don't need the `i` in the loop?
}

Modified into this way atm, but if we want it to be simpler I can revert it back as well 😅
Wait I'm not sure I understand... Here in the loop we don't seem to need that `point - points.start()` value at all, we seem to just need `point` in the global index...? If I'm understanding this correctly then we should do: ``` for(const int i : points.index_range()){ const int point = points[i]; // but apparently we don't need the `i` in the loop? } Modified into this way atm, but if we want it to be simpler I can revert it back as well 😅
Member

get_weight needs i in this case, though currently it's hidden inside the lambda.

`get_weight` needs `i` in this case, though currently it's hidden inside the lambda.
Author
Member

Ah I see. Nice catch

Ah I see. Nice catch
ChengduLittleA marked this conversation as resolved
@ -0,0 +231,4 @@
static void modify_geometry_set(ModifierData *md,
const ModifierEvalContext *ctx,
blender::bke::GeometrySet *geometry_set)
Member

blender:: is unnecessary, we're already in the blender namespace

`blender::` is unnecessary, we're already in the blender namespace
ChengduLittleA marked this conversation as resolved
YimingWu force-pushed gp3-noise-modifier from 7988805e15 to f08750e917 2024-01-23 03:29:36 +01:00 Compare
YimingWu added 1 commit 2024-01-23 03:34:30 +01:00
YimingWu added 1 commit 2024-01-23 03:40:14 +01:00
YimingWu added 1 commit 2024-01-23 09:26:44 +01:00
Hans Goudey requested changes 2024-01-23 16:56:38 +01:00
Hans Goudey left a comment
Member

Maybe I'm doing it wrong? The modifier isn't doing anything here.

Maybe I'm doing it wrong? The modifier isn't doing anything here.
@ -298,1 +298,4 @@
"Deform volume based on noise or other vector fields"}, /* TODO: Use correct icon. */
{eModifierType_GreasePencilNoise,
"GREASEPENCIL_NOISE",
ICON_GREASEPENCIL,
Member

This isn't the right icon

This isn't the right icon
ChengduLittleA marked this conversation as resolved
Member

@HooglyBoogly Looks like the "Noise Scale" is zero.

@HooglyBoogly Looks like the "Noise Scale" is zero.
Member

Hmm, right. Should that be the default value?

Anyway, the position noise seems not to work either way?

Hmm, right. Should that be the default value? Anyway, the position noise seems not to work either way?
Member

@HooglyBoogly Working for me. Maybe you forgot the experimental option?

@HooglyBoogly Working for me. Maybe you forgot the experimental option?
Falk David requested changes 2024-01-23 17:44:32 +01:00
Falk David left a comment
Member

I noticed that it doesn't use the right normals

diff --git a/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc b/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc
index 6b0bbf76ad2..9dda5e094cd 100644
--- a/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc
+++ b/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc
@@ -152,7 +152,7 @@ static void deform_drawing(const ModifierData &md,
   };
 
   if (mmd.factor > 0.0f) {
-    const Span<float3> normals = strokes.evaluated_normals();
+    const Span<float3> curve_plane_normals = drawing.curve_plane_normals();
     const Span<float3> tangents = strokes.evaluated_tangents();
     MutableSpan<float3> positions = strokes.positions_for_write();
     const Array<float> noise_table_position = noise_table(
@@ -164,7 +164,8 @@ static void deform_drawing(const ModifierData &md,
         const int point = points[i];
         float weight = get_weight(points, i);
         /* Vector orthogonal to normal. */
-        const float3 bi_normal = math::normalize(math::cross(tangents[point], normals[point]));
+        const float3 bi_normal = math::normalize(
+            math::cross(tangents[point], curve_plane_normals[stroke_i]));
         const float noise = table_sample(noise_table_position,
                                          point * noise_scale + math::fract(mmd.noise_offset));
         positions[point] += bi_normal * (noise * 2.0f - 1.0f) * weight * mmd.factor * 0.1f;
I noticed that it doesn't use the right normals ```diff diff --git a/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc b/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc index 6b0bbf76ad2..9dda5e094cd 100644 --- a/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc +++ b/source/blender/modifiers/intern/MOD_grease_pencil_noise.cc @@ -152,7 +152,7 @@ static void deform_drawing(const ModifierData &md, }; if (mmd.factor > 0.0f) { - const Span<float3> normals = strokes.evaluated_normals(); + const Span<float3> curve_plane_normals = drawing.curve_plane_normals(); const Span<float3> tangents = strokes.evaluated_tangents(); MutableSpan<float3> positions = strokes.positions_for_write(); const Array<float> noise_table_position = noise_table( @@ -164,7 +164,8 @@ static void deform_drawing(const ModifierData &md, const int point = points[i]; float weight = get_weight(points, i); /* Vector orthogonal to normal. */ - const float3 bi_normal = math::normalize(math::cross(tangents[point], normals[point])); + const float3 bi_normal = math::normalize( + math::cross(tangents[point], curve_plane_normals[stroke_i])); const float noise = table_sample(noise_table_position, point * noise_scale + math::fract(mmd.noise_offset)); positions[point] += bi_normal * (noise * 2.0f - 1.0f) * weight * mmd.factor * 0.1f; ```
Author
Member

@filedescriptor Thanks! Note that the plane normal implementation also isn't the same as the old one. The old one only uses 3 points, the two end points and a point at a index of 0.75*totpoints (BKE_gpencil_stroke_normal)... So the behavior is gonna be different anyway, and it looks to me that using individual normals feels better, and this way you can have natural direction changes over 3d. Any thoughts?

Of course we'll need to document the difference in here too.

@filedescriptor Thanks! Note that the plane normal implementation also isn't the same as the old one. The old one only uses 3 points, the two end points and a point at a index of `0.75*totpoints` (`BKE_gpencil_stroke_normal`)... So the behavior is gonna be different anyway, and it looks to me that using individual normals feels better, and this way you can have natural direction changes over 3d. Any thoughts? Of course we'll need to document the difference in here too.
YimingWu added 1 commit 2024-01-24 02:27:14 +01:00
YimingWu added 1 commit 2024-01-24 05:05:35 +01:00
Member

@ChengduLittleA Right the old method was a crude way of getting a triangle with the plane normal. The new method is a bit more robust, and will yield the same result for flat strokes (which is the common case). The normal from the curves is very different from what we need. We need one normal per stroke, not per point.

@ChengduLittleA Right the old method was a crude way of getting a triangle with the plane normal. The new method is a bit more robust, and will yield the same result for flat strokes (which is the common case). The normal from the curves is very different from what we need. We need one normal per stroke, not per point.
YimingWu added 1 commit 2024-01-24 11:42:14 +01:00
YimingWu added 2 commits 2024-01-25 04:40:28 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
fadb0ff7fe
Use `FrameDrawingInfo`
Hans Goudey requested changes 2024-01-25 14:26:58 +01:00
Hans Goudey left a comment
Member

The behavior in the video while drawing doesn't happen in the legacy version of the modifier.

The behavior in the video while drawing doesn't happen in the legacy version of the modifier.
@ -0,0 +128,4 @@
const OffsetIndices<int> points_by_curve = strokes.points_by_curve();
int seed = mmd.seed + strokes.points_num();
Member

I noticed the seed = mmd.seed + strokes.points_num(); is different than the legacy version of the noise modifier which uses seed = mmd->seed + BLI_findindex(&gpf->strokes, gps);

On chat we talked about making sure the results are exactly the same, so probably better to stick with the old seed?

I noticed the `seed = mmd.seed + strokes.points_num()`; is different than the legacy version of the noise modifier which uses `seed = mmd->seed + BLI_findindex(&gpf->strokes, gps);` On chat we talked about making sure the results are exactly the same, so probably better to stick with the old seed?
@ -0,0 +233,4 @@
const IndexMask layer_mask = modifier::greasepencil::get_filtered_layer_mask(
grease_pencil, mmd->influence, mask_memory);
const Vector<modifier::greasepencil::FrameDrawingInfo> drawing_infos =
modifier::greasepencil::get_drawing_infos_by_frame(
Member

Clang format needed here

Clang format needed here
Member

@blender-bot build

@blender-bot build
First-time contributor

small questions, is it possible to have where the noise is cyclic (like we see in cartoon 3steps ...ect), if yes can we know a what is the range
or it 's a feature that i'm questioning about ? if so will it be a future target ?

small questions, is it possible to have where the noise is cyclic (like we see in cartoon 3steps ...ect), if yes can we know a what is the range or it 's a feature that i'm questioning about ? if so will it be a future target ?
Member

@hamza-el-barmaki I'm not exactly sure what you're asking about, but it's not a question that belongs in this PR.

@hamza-el-barmaki I'm not exactly sure what you're asking about, but it's not a question that belongs in this PR.
Member

@ChengduLittleA Looks like there are some formatting issues still.

@ChengduLittleA Looks like there are some formatting issues still.
YimingWu added 2 commits 2024-01-29 04:37:01 +01:00
Falk David added 1 commit 2024-01-29 13:10:09 +01:00
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
5e88e75829
Use per stroke noise
Author
Member

Update: After making sure that we use the same stroke order (by using Convert To -> Grease Pencil v3) we do get consistent result in both v2 and v3 with per-stroke table.

Hummm the result is still not exactly the same. The stroke on top seems flipped in noise direction but other strokes suggests otherwise... Not sure what's wrong here, probably seed is hashed differently

old new
image image
Update: After making sure that we use the same stroke order (by using `Convert To -> Grease Pencil v3`) we do get consistent result in both v2 and v3 with per-stroke table. ~~Hummm the result is still not exactly the same. The stroke on top seems flipped in noise direction but other strokes suggests otherwise... Not sure what's wrong here, probably seed is hashed differently~~ | old | new | |---|---| | ![image](/attachments/6fc12c58-d139-447d-88ef-dc82989ebebd) | ![image](/attachments/3f02cbaa-add8-4a08-b6d1-8ad737cfe32a) |
Falk David approved these changes 2024-01-29 13:55:17 +01:00
Member

@blender-bot build

@blender-bot build
Hans Goudey requested changes 2024-01-29 14:47:44 +01:00
Hans Goudey left a comment
Member

Obviously not ideal to allocate an array for every curve, but this is still probably way faster than the old modifier so it should be good enough. Just a few comments inline.

Obviously not ideal to allocate an array for every curve, but this is still probably way faster than the old modifier so it should be good enough. Just a few comments inline.
@ -125,6 +125,7 @@ typedef enum eNoiseGpencil_Flag {
GP_NOISE_MOD_UV = (1 << 9), /* Deprecated (only for versioning). */
GP_NOISE_INVERT_LAYERPASS = (1 << 10),
GP_NOISE_INVERT_MATERIAL = (1 << 11),
MOD_GREASE_PENCIL_NOISE_OPEN_INFLUENCE_PANEL = (1 << 12),
Member

This looks unused and can be removed

This looks unused and can be removed
ChengduLittleA marked this conversation as resolved
@ -0,0 +159,4 @@
filtered_strokes.foreach_index(GrainSize(512), [&](const int stroke_i) {
const IndexRange points = points_by_curve[stroke_i];
const int noise_len = math::ceil(points.size() * noise_scale) + 2;
const Array<float> noise_table_positions = noise_table(
Member

This whole loop is about the positions, so this could have a simpler name like noise_table here

This whole loop is about the positions, so this could have a simpler name like `noise_table` here
ChengduLittleA marked this conversation as resolved
@ -0,0 +219,4 @@
const ModifierEvalContext *ctx,
bke::GeometrySet *geometry_set)
{
GreasePencilNoiseModifierData *mmd = reinterpret_cast<GreasePencilNoiseModifierData *>(md);
Member

Add const (with const auto &) here. Then pass mmd rather than md to the deform_drawing function

Add const (with `const auto &`) here. Then pass `mmd` rather than `md` to the `deform_drawing` function
ChengduLittleA marked this conversation as resolved
YimingWu added 1 commit 2024-01-29 15:34:26 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
daa6984962
Cleanups
Falk David requested review from Hans Goudey 2024-01-29 15:35:59 +01:00
Hans Goudey approved these changes 2024-01-29 15:49:26 +01:00
Member

@blender-bot build

@blender-bot build
Falk David referenced this issue from a commit 2024-01-29 16:49:26 +01:00
Falk David merged commit 56439d88f5 into main 2024-01-29 16:49:26 +01:00
Jonas Dichelle referenced this issue from a commit 2024-02-08 17:29:06 +01:00
Sign in to join this conversation.
No reviewers
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
7 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#117057
No description provided.