blender
/
blender
134
398
Fork 579
Code Issues 6.3k Pull Requests 750 Projects 19 Wiki Activity

Initial Grease Pencil 3.0 stage #106848

Merged
Falk David merged 224 commits from filedescriptor/blender:grease-pencil-v3 into main 2023-05-30 11:14:22 +02:00
Conversation 8 Commits 224 Files Changed 94 +43 -22
1 changed files with 43 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 47ab3d91ce - Show all commits

65
source/blender/draw/intern/draw_cache_impl_grease_pencil.cc
View File

@ -186,6 +186,29 @@ static int count_cyclic_curves(bke::CurvesGeometry &curves)
[&](const int a, const int b) { return a + b; });
}
BLI_INLINE int32_t pack_rotation_aspect_hardness(float rot, float asp, float hard)
{
int32_t packed = 0;
/* Aspect uses 9 bits */
float asp_normalized = (asp > 1.0f) ? (1.0f / asp) : asp;
packed |= int32_t(unit_float_to_uchar_clamp(asp_normalized));
/* Store if inversed in the 9th bit. */
if (asp > 1.0f) {
packed |= 1 << 8;
}
Hans Goudey commented 2023-04-25 04:44:09 +02:00
Outdated
Review
Copy Link

Looks like this can be Vector<Array<int>>, which is better since they don't need to be resized anyway

Looks like this can be `Vector<Array<int>>`, which is better since they don't need to be resized anyway
/* Rotation uses 9 bits */
/* Rotation are in [-90°..90°] range, so we can encode the sign of the angle + the cosine
Hans Goudey commented 2023-04-25 05:12:29 +02:00
Outdated
Review
Copy Link

What about building a vector of the drawings once, to avoid putting most of the code inside the lambda here and the need to call "foreach_visible_drawing" twice:

  Vector<const GreasePencilDrawing *> drawings;
  grease_pencil.foreach_visible_drawing(
      cfra, [&](GreasePencilDrawing &drawing) { drawings.append(&drawing); });

...

  Array<Array<int>> verts_start_offsets_per_visible_drawing;
  Array<Array<int>> tris_start_offsets_per_visible_drawing;

  for (const int drawing_i : drawings.index_range()) {
    const GreasePencilDrawing &drawing = *drawings[drawing_i];

...

  /* Fill buffers with data. */
  for (const int drawing_i : drawings.index_range()) {
    const GreasePencilDrawing &drawing = *drawings[drawing_i];
What about building a vector of the drawings once, to avoid putting most of the code inside the lambda here and the need to call "foreach_visible_drawing" twice: ``` Vector<const GreasePencilDrawing *> drawings; grease_pencil.foreach_visible_drawing( cfra, [&](GreasePencilDrawing &drawing) { drawings.append(&drawing); }); ... Array<Array<int>> verts_start_offsets_per_visible_drawing; Array<Array<int>> tris_start_offsets_per_visible_drawing; for (const int drawing_i : drawings.index_range()) { const GreasePencilDrawing &drawing = *drawings[drawing_i]; ... /* Fill buffers with data. */ for (const int drawing_i : drawings.index_range()) { const GreasePencilDrawing &drawing = *drawings[drawing_i]; ```
* because the cosine will always be positive. */
packed |= int32_t(unit_float_to_uchar_clamp(cosf(rot))) << 9;
/* Store sine sign in 9th bit. */
if (rot < 0.0f) {
packed |= 1 << 17;
}
/* Hardness uses 8 bits */
packed |= int32_t(unit_float_to_uchar_clamp(hard)) << 18;
Hans Goudey commented 2023-04-25 04:43:13 +02:00
Outdated
Review
Copy Link

More specific/helpful variable names than t and v would be nice here

More specific/helpful variable names than `t` and `v` would be nice here
return packed;
}
static void grease_pencil_batches_ensure(GreasePencil &grease_pencil, int cfra)
{
BLI_assert(grease_pencil.runtime != nullptr);
@ -249,45 +272,43 @@ static void grease_pencil_batches_ensure(GreasePencil &grease_pencil, int cfra)
VArray<int> materials = attributes.lookup_or_default<int>(
"material_index", ATTR_DOMAIN_CURVE, -1);
Hans Goudey commented 2023-04-25 04:13:57 +02:00
Outdated
Review
Copy Link

Use the * overload rather than adding .varray at the end

Use the `*` overload rather than adding `.varray` at the end
auto populate_point = [&](int curve_i, int point_i, int v_start, int v) {
copy_v3_v3(verts[v].pos, positions[point_i]);
verts[v].radius = radii[point_i];
verts[v].opacity = opacities[point_i];
verts[v].point_id = v;
verts[v].stroke_id = v_start;
verts[v].mat = materials[curve_i] % GPENCIL_MATERIAL_BUFFER_LEN;
verts[v].packed_asp_hard_rot = pack_rotation_aspect_hardness(0.0f, 1.0f, 1.0f);
verts[v].u_stroke = 0;
verts[v].uv_fill[0] = verts[v].uv_fill[1] = 0;
int v_mat = (v << GP_VERTEX_ID_SHIFT) | GP_IS_STROKE_VERTEX_BIT;
Hans Goudey commented 2023-04-25 04:16:22 +02:00
Outdated
Review
Copy Link

These .as_span() calls shouldn't be necessary

These `.as_span()` calls shouldn't be necessary
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 0, v_mat + 1, v_mat + 2);
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 2, v_mat + 1, v_mat + 3);
};
for (const int curve_i : curves.curves_range()) {
IndexRange points = points_by_curve[curve_i];
const bool is_cyclic = cyclic[curve_i];
const int v_start = v;
/* First point is not drawn. */
verts[v].mat = -1;
v++;
for (const int point_i : points) {
copy_v3_v3(verts[v].pos, positions[point_i]);
verts[v].radius = radii[point_i];
verts[v].opacity = opacities[point_i];
verts[v].point_id = v;
verts[v].stroke_id = v_start;
verts[v].mat = materials[curve_i] % GPENCIL_MATERIAL_BUFFER_LEN;
int v_mat = (v << GP_VERTEX_ID_SHIFT) | GP_IS_STROKE_VERTEX_BIT;
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 0, v_mat + 1, v_mat + 2);
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 2, v_mat + 1, v_mat + 3);
populate_point(curve_i, point_i, v_start, v);
v++;
}
if (is_cyclic) {
copy_v3_v3(verts[v].pos, positions[points.first()]);
verts[v].radius = radii[points.first()];
verts[v].opacity = opacities[points.first()];
verts[v].point_id = v;
verts[v].stroke_id = v_start;
verts[v].mat = materials[curve_i] % GPENCIL_MATERIAL_BUFFER_LEN;
int v_mat = (v << GP_VERTEX_ID_SHIFT) | GP_IS_STROKE_VERTEX_BIT;
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 0, v_mat + 1, v_mat + 2);
GPU_indexbuf_add_tri_verts(&ibo, v_mat + 2, v_mat + 1, v_mat + 3);
populate_point(curve_i, points.first(), v_start, v);
Sergey Sharybin commented 2023-04-25 10:01:46 +02:00
Outdated
Review
Copy Link

Add details about TODO. Something that helps others to pick up a task, or to understand that specific case is not expected to be yet working.

Add details about TODO. Something that helps others to pick up a task, or to understand that specific case is not expected to be yet working.
v++;
}
/* Last point is not drawn. */
verts[v].mat = -1;
v++;
}
});