blender/blender
154
489
Fork 727
Code Issues 6.7k Pull Requests 885 Projects 18 Wiki Activity

Fix #124297: GPv3: Build modifier natural drawing speed fix #124350

Merged
Falk David merged 6 commits from ChengduLittleA/blender:fix-124297 into main 2024-07-16 10:26:50 +02:00
Conversation 13 Commits 6 Files Changed 3 +15 -4
3 changed files with 15 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 257ae19737 - Show all commits

4
source/blender/blenkernel/intern/grease_pencil_convert_legacy.cc
View File

@ -796,7 +796,9 @@ static Drawing legacy_gpencil_frame_to_grease_pencil_drawing(const bGPDframe &gp
stroke_cyclic.span[stroke_i] = (gps->flag & GP_STROKE_CYCLIC) != 0;
/* TODO: This should be a `double` attribute. */
ChengduLittleA marked this conversation as resolved Outdated
Falk David commented 2024-07-08 15:55:38 +02:00
Outdated
Review
Copy Link

We can remove this TODO I think

We can remove this TODO I think
stroke_init_times.span[stroke_i] = float(gps->inittime);
/* Truncating time in ms to uint32 then we don't lose precision in lower bits. */
stroke_init_times.span[stroke_i] = float(uint32_t(uint64_t(gps->inittime * (double)1e3))) /
ChengduLittleA marked this conversation as resolved Outdated
Falk David commented 2024-07-08 15:56:36 +02:00
Outdated
Review
Copy Link

Use double(1e3).
No need to cast to uint64_t before casting to uint32_t.

Use `double(1e3)`. No need to cast to `uint64_t` before casting to `uint32_t`.
float(1e3);
stroke_start_caps.span[stroke_i] = int8_t(gps->caps[0]);
stroke_end_caps.span[stroke_i] = int8_t(gps->caps[1]);
stroke_softness.span[stroke_i] = 1.0f - gps->hardness;

4
source/blender/editors/sculpt_paint/grease_pencil_paint.cc
View File

@ -590,7 +590,9 @@ struct PaintOperationExecutor {
bke::SpanAttributeWriter<float> init_times = attributes.lookup_or_add_for_write_span<float>(
"init_time", bke::AttrDomain::Curve);
init_times.span[active_curve] = self.start_time_;
/* Truncating time in ms to uint32 then we don't lose precision in lower bits. */
init_times.span[active_curve] = float(uint32_t(uint64_t(self.start_time_ * double(1e3)))) /
ChengduLittleA marked this conversation as resolved Outdated
Falk David commented 2024-07-08 15:58:33 +02:00
Outdated
Review
Copy Link

Same as above, no need to cast to uint64_t before casting to uint32_t.

Same as above, no need to cast to `uint64_t` before casting to `uint32_t`.
float(1e3);
curve_attributes_to_skip.add("init_time");
init_times.finish();

11
source/blender/modifiers/intern/MOD_grease_pencil_build.cc
View File

@ -473,16 +473,23 @@ static float get_factor_from_draw_speed(const bke::CurvesGeometry &curves,
"delta_time", bke::AttrDomain::Point, 0.0f);
Array<float> times(curves.points_num());
filedescriptor marked this conversation as resolved Outdated
Falk David commented 2024-07-15 11:31:46 +02:00
Outdated
Review
Copy Link

I think it's better if we pre-compute the start times (accounting for the maximum gap) for each curve first, and then go over the points.

Array<float> start_times(curves.curves_num());
start_times[0] = init_times[0];
float accumulated_shift_delta_time = 0.0f;
for (const int curve : curves.curves_range().drop_front(1)) {
    const float previous_start_time = start_times[curve - 1];
    const float previous_delta_time = delta_times[points_by_curve[curve - 1].last()];
    const float previous_end_time = previous_start_time + previous_delta_time;

    const float shifted_start_time = init_times[curve] - accumulated_shift_delta_time;
    const float gap_delta_time = math::min(shifted_start_time - previous_end_time, max_gap);

    start_times[curve] = previous_end_time + gap_delta_time;
    accumulated_shift_delta_time += math::max(shifted_start_time - start_times[curve], 0.0f);
}

The second loop then becomes much simpler. We can even return from there directly:

const float limit = time_elapsed * speed_fac;
for (const int curve : curves.curves_range()) {
    const float start_time = start_times[curve];
	for (const int point : points_by_curve[curve]) {
        if (start_time + delta_times[point] >= limit) {
            return math::clamp(float(point) / float(curves.points_num()), 0.0f, 1.0f);
        }
    }
}
I think it's better if we pre-compute the start times (accounting for the maximum gap) for each curve first, and then go over the points. ```cpp Array<float> start_times(curves.curves_num()); start_times[0] = init_times[0]; float accumulated_shift_delta_time = 0.0f; for (const int curve : curves.curves_range().drop_front(1)) { const float previous_start_time = start_times[curve - 1]; const float previous_delta_time = delta_times[points_by_curve[curve - 1].last()]; const float previous_end_time = previous_start_time + previous_delta_time; const float shifted_start_time = init_times[curve] - accumulated_shift_delta_time; const float gap_delta_time = math::min(shifted_start_time - previous_end_time, max_gap); start_times[curve] = previous_end_time + gap_delta_time; accumulated_shift_delta_time += math::max(shifted_start_time - start_times[curve], 0.0f); } ``` The second loop then becomes much simpler. We can even return from there directly: ```cpp const float limit = time_elapsed * speed_fac; for (const int curve : curves.curves_range()) { const float start_time = start_times[curve]; for (const int point : points_by_curve[curve]) { if (start_time + delta_times[point] >= limit) { return math::clamp(float(point) / float(curves.points_num()), 0.0f, 1.0f); } } } ```
👀 1
YimingWu commented 2024-07-15 13:47:45 +02:00
Outdated
Review
Copy Link

Thanks for the suggestion. Let me see if I could make this work :D

Thanks for the suggestion. Let me see if I could make this work :D
YimingWu commented 2024-07-15 14:07:15 +02:00
Outdated
Review
Copy Link

start_times[0] needs to be 0 obviously and otherwise it seems to be working fine taking into account of gap limit. Thanks!

`start_times[0]` needs to be 0 obviously and otherwise it seems to be working fine taking into account of gap limit. Thanks!
Falk David commented 2024-07-15 14:19:15 +02:00
Outdated
Review
Copy Link

Hm if start_times[0] = 0 then you need to set float accumulated_shift_delta_time = init_times[0]; so that all the strokes after are also shifted relative to the first stroke start time.

Hm if `start_times[0] = 0` then you need to set `float accumulated_shift_delta_time = init_times[0];` so that all the strokes after are also shifted relative to the first stroke start time.
YimingWu commented 2024-07-15 15:20:58 +02:00
Outdated
Review
Copy Link

Ah yes, this way the first stroke won't have a gap time

Ah yes, this way the first stroke won't have a gap time
float previous_end_time = init_times[0];
float accumulated_delta_time = 0;
/* For the first stroke, the start time is 0. */
for (const int point : points_by_curve[0]) {
times[point] = delta_times[point];
}
accumulated_delta_time += times[points_by_curve[0].last()];
for (const int curve : curves.curves_range().drop_front(1)) {
const float previous_init_time = init_times[curve - 1];
const float start_time = math::min(init_times[curve] - previous_init_time, max_gap);
const float start_time = math::min(init_times[curve] - previous_end_time, max_gap) +
accumulated_delta_time;
for (const int point : points_by_curve[curve]) {
times[point] = start_time + delta_times[point];
}
const float this_delta_time = times[points_by_curve[curve].last()];
previous_end_time = init_times[curve] + this_delta_time;
accumulated_delta_time += this_delta_time;
}
for (const int point : curves.points_range()) {
const float limit = time_elapsed * speed_fac;