GPv3: Add Catmull-Clark option in subdivide modifier #117382

YimingWu · 2024-01-21T14:35:32+01:00

YimingWu commented

2024-01-21 14:35:32 +01:00

The subdivide modifier of Grease Pencil v3 was lacking catmull smoothing option when it
was merged at first due to no apparent API for doing that directly. Now re-implemented
using CURVE_TYPE_CATMULL_ROM and curves.evaluated_positions().

The subdivide modifier of Grease Pencil v3 was lacking catmull smoothing option when it was merged at first due to no apparent API for doing that directly. Now re-implemented using `CURVE_TYPE_CATMULL_ROM` and `curves.evaluated_positions()`.

YimingWu added the

Module

Grease Pencil

label 2024-01-21 14:35:32 +01:00

YimingWu added 1 commit 2024-01-21 14:35:44 +01:00

c4fb186604 Catmull smoothing of the curve

Falk David added this to the Grease Pencil project 2024-01-22 12:56:18 +01:00

Falk David commented

2024-01-23 11:25:12 +01:00

@ChengduLittleA Unfortunately, this does not seem to work like in GPv2 :/ I'll think about this some more.

Falk David commented

2024-01-23 11:30:18 +01:00

This is a direct port of the old implementation, but it has some issues

if (use_catmull_clark) {
  bke::CurvesGeometry subdivided_curves = drawing.strokes();
  for ([[maybe_unused]] const int level_i : IndexRange(mmd.level)) {
    VArray<int> one_cut = VArray<int>::ForSingle(1, subdivided_curves.points_num());
    subdivided_curves = geometry::subdivide_curves(
        subdivided_curves, strokes, std::move(one_cut), {});

    offset_indices::OffsetIndices<int> points_by_curve = subdivided_curves.points_by_curve();
    MutableSpan<float3> positions = subdivided_curves.positions_for_write();
    threading::parallel_for(subdivided_curves.curves_range(), 1024, [&](const IndexRange range) {
      for (const int curve_i : range) {
        const IndexRange points = points_by_curve[curve_i];
        for (const int point_i : points.drop_front(1).drop_back(1)) {
          positions[point_i] = math::interpolate(
              positions[point_i],
              math::interpolate(positions[point_i - 1], positions[point_i + 1], 0.5f),
              0.5f);
        }
      }
    });
  }
  drawing.strokes_for_write() = subdivided_curves;
}
else {
  VArray<int> cuts = VArray<int>::ForSingle(mmd.level, drawing.strokes().points_num());
  drawing.strokes_for_write() = geometry::subdivide_curves(
      drawing.strokes(), strokes, std::move(cuts), {});
}

It means that mmd.level is treated as a power of 2 again so anything higher than 5 gets very slow.
It's an iterative algorithm that has to subidivide once per iteration.

We could add a seperate level input for the Catmull-Clark mode and change the limits for it, but that doesn't seem like the best idea.

@HooglyBoogly Anything you can think of maybe? I'm not sure if there is a good way to compute the "final" catmull-clark positions. That would be ideal because then we can just do the normal geometry::subdivide_curves and call something like geometry::smooth_curves_catmull_clark after.

This is a direct port of the old implementation, but it has some issues ```cpp if (use_catmull_clark) { bke::CurvesGeometry subdivided_curves = drawing.strokes(); for ([[maybe_unused]] const int level_i : IndexRange(mmd.level)) { VArray<int> one_cut = VArray<int>::ForSingle(1, subdivided_curves.points_num()); subdivided_curves = geometry::subdivide_curves( subdivided_curves, strokes, std::move(one_cut), {}); offset_indices::OffsetIndices<int> points_by_curve = subdivided_curves.points_by_curve(); MutableSpan<float3> positions = subdivided_curves.positions_for_write(); threading::parallel_for(subdivided_curves.curves_range(), 1024, [&](const IndexRange range) { for (const int curve_i : range) { const IndexRange points = points_by_curve[curve_i]; for (const int point_i : points.drop_front(1).drop_back(1)) { positions[point_i] = math::interpolate( positions[point_i], math::interpolate(positions[point_i - 1], positions[point_i + 1], 0.5f), 0.5f); } } }); } drawing.strokes_for_write() = subdivided_curves; } else { VArray<int> cuts = VArray<int>::ForSingle(mmd.level, drawing.strokes().points_num()); drawing.strokes_for_write() = geometry::subdivide_curves( drawing.strokes(), strokes, std::move(cuts), {}); } ``` 1. It means that `mmd.level` is treated as a power of 2 again so anything higher than 5 gets very slow. 2. It's an iterative algorithm that has to subidivide once per iteration. We could add a seperate `level` input for the `Catmull-Clark` mode and change the limits for it, but that doesn't seem like the best idea. @HooglyBoogly Anything you can think of maybe? I'm not sure if there is a good way to compute the "final" catmull-clark positions. That would be ideal because then we can just do the normal `geometry::subdivide_curves` and call something like `geometry::smooth_curves_catmull_clark` after.

YimingWu commented

2024-01-23 11:56:40 +01:00

I think it would be hard to use the linear level for Catmull-Clark, since to do that perfectly you'll then need to have a "limit curve" and sample the points along it, like in OpenSubdiv you can get a limit surface without actually having to subdivide the geometry.

I think it would be hard to use the linear `level` for Catmull-Clark, since to do that perfectly you'll then need to have a "limit curve" and sample the points along it, like in OpenSubdiv you can get a limit surface without actually having to subdivide the geometry.

Hans Goudey commented

2024-01-23 19:02:11 +01:00

I would recommend just porting the old modifier's logic. It seems important to frame our efforts with the modifiers in the right context. The main goal of adding the modifiers is compatibility. If we get better performance that's nice, but long term these things should be implemented with nodes anyway. So I'd suggest not overthinking this and just doing the simplest thing.

YimingWu commented

2024-01-24 02:23:28 +01:00

@filedescriptor does the "direct port" of the algorithm work exactly the same way as the old one? If so I think we could use 3 modes: Simple, Subdiv Smooth (the linear subdiv mode that's current, with mid-point smooth), and Catmull-Clark (for the old behaviour). Thoughts?

@filedescriptor does the "direct port" of the algorithm work exactly the same way as the old one? If so I think we could use 3 modes: `Simple`, `Subdiv Smooth` (the linear subdiv mode that's current, with mid-point smooth), and `Catmull-Clark` (for the old behaviour). Thoughts?

Iliya Katushenock reviewed 2024-01-24 09:32:58 +01:00

source/blender/modifiers/intern/MOD_grease_pencil_subdiv.cc Outdated

						
				@ -90,2 +97,4 @@

				  /* Simple subdiv first, then we can catmull smooth positions. */

				  drawing.strokes_for_write() = geometry::subdivide_curves(

				      drawing.strokes(), strokes, std::move(cuts), {});

Iliya Katushenock commented

2024-01-24 09:32:58 +01:00

Argument cuts is taken by const reference, you can delete std::move and back const to variable declaration.

Argument `cuts` is taken by const reference, you can delete `std::move` and back `const` to variable declaration.

ChengduLittleA marked this conversation as resolved

Falk David commented

2024-01-24 10:39:46 +01:00

@ChengduLittleA Yes, the code above will give the same results as the old one.

I'm not sure what you mean with Subdiv Smooth. Seems like that would be the same thing as Catmull-Clark.

I think we should stick to having the two modes as before Simple and Catmull-Clark. But because Catmull-Clark subdivides exponetially, we have to do the same in the Simple mode. Basically restoring the old behavior exactly.

So to summarize:

Add the code path for Catmull-Clark.
Use math::pow(2, mmd.level) for geometry::subdivide_curves in Simple mode.
Restore the old limits in RNA

@ChengduLittleA Yes, the code above will give the same results as the old one. I'm not sure what you mean with `Subdiv Smooth`. Seems like that would be the same thing as `Catmull-Clark`. I think we should stick to having the two modes as before `Simple` and `Catmull-Clark`. But because `Catmull-Clark` subdivides exponetially, we have to do the same in the `Simple` mode. Basically restoring the old behavior exactly. So to summarize: * Add the code path for `Catmull-Clark`. * Use `math::pow(2, mmd.level)` for `geometry::subdivide_curves` in `Simple` mode. * Restore the old limits in RNA

👍 1

YimingWu added 1 commit 2024-01-24 11:48:25 +01:00

ee69b6053b Use iterative catmull smooth.

Falk David approved these changes 2024-01-24 11:57:12 +01:00

Falk David left a comment

Thanks for updating. Just one cleanup comment.

source/blender/makesrna/intern/rna_modifier.cc Outdated

						
				@ -7766,6 +7766,12 @@ static void rna_def_modifier_grease_pencil_subdiv(BlenderRNA *brna)

				  StructRNA *srna;

				  PropertyRNA *prop;

				  static const EnumPropertyItem gpencil_subdivision_type_items[] = {

Falk David commented

2024-01-24 11:56:30 +01:00

Can just be subdivision_type_items

Can just be `subdivision_type_items`

ChengduLittleA marked this conversation as resolved

YimingWu added 1 commit 2024-01-24 12:34:56 +01:00

bbc8118333 cleanups

Falk David changed title from ~~GPv3: Catmull smoothing in Subdiv modifier~~ to GPv3: Add Catmull-Clark option in subdivide modifier

2024-01-24 13:11:36 +01:00

Falk David referenced this issue from a commit

2024-01-24 13:14:16 +01:00

GPv3: Add Catmull-Clark option in subdivide modifier

Falk David merged commit 756f7cf1e8 into main

2024-01-24 13:14:16 +01:00

Jonas Dichelle referenced this issue from a commit

2024-02-08 17:29:05 +01:00

GPv3: Add Catmull-Clark option in subdivide modifier

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

GPv3: Add Catmull-Clark option in subdivide modifier #117382