Sculpt Hide Tool doesn't hide all adjacent faces when single vertex is selected #120761

New Issue

Sean Kim · 2024-04-18T02:33:42+02:00

Sean Kim commented

2024-04-18 02:33:42 +02:00

System Information
Operating system: macOS-14.2-x86_64-i386-64bit 64 Bits
Graphics card: Metal API AMD Radeon Pro 5300M 1.2

Blender Version
Broken: version: 4.1.1, branch: blender-v4.1-release, commit date: 2024-04-15 15:11, hash: e1743a0317bc
Worked: version: 4.0.2, branch: blender-v4.0-release, commit date: 2023-12-05 07:41, hash: 9be62e85b727

Short description of error
With a sufficiently large mesh, selecting a single vertex with the Box Hide tool only hides a subset of the connected faces

4.0.2	4.1.1

Exact steps for others to reproduce the error

Open attached hide_show_test.blend file.
Select central vertex with Box Hide Tool

In practice, I don't think this particular manifestation of the bug has a major impact, as selecting single vertices in a mesh of this size seems to be an extremely niche usecase, however this may indicate other potential problems with the tool.

**System Information** Operating system: macOS-14.2-x86_64-i386-64bit 64 Bits Graphics card: Metal API AMD Radeon Pro 5300M 1.2 **Blender Version** Broken: version: 4.1.1, branch: blender-v4.1-release, commit date: 2024-04-15 15:11, hash: `e1743a0317bc` Worked: version: 4.0.2, branch: blender-v4.0-release, commit date: 2023-12-05 07:41, hash: `9be62e85b727` **Short description of error** With a sufficiently large mesh, selecting a single vertex with the *Box Hide* tool only hides a subset of the connected faces | 4.0.2 | 4.1.1 | | ----- | ----- | | ![2024-04-17 17.23.18.gif](/attachments/dd1dda03-01f2-4f7c-8618-18469706fe68) | ![2024-04-17 17.23.46.gif](/attachments/5f1c8e3b-cd70-490d-b3d1-a45558cc089c) | **Exact steps for others to reproduce the error** 1. Open attached `hide_show_test.blend` file. 2. Select central vertex with `Box Hide` Tool --- In practice, I don't think this particular manifestation of the bug has a major impact, as selecting single vertices in a mesh of this size seems to be an extremely niche usecase, however this may indicate other potential problems with the tool.

2024-04-17 17.23.18.gif

1.1 MiB

hide_show_test.blend

1.6 MiB

2024-04-17 17.23.46.gif

928 KiB

Sean Kim added the

labels 2024-04-18 02:33:43 +02:00

YimingWu commented

2024-04-18 05:14:02 +02:00

Can confirm. Looks like the problem is only apparent on the strip of faces just above the x axis.

YimingWu added

and removed

labels 2024-04-18 05:14:11 +02:00

YimingWu commented

2024-04-18 05:17:12 +02:00

working 4.0.3 63e9cead5f
broken 4.1.0 6007838bb2

working 4.0.3 63e9cead5ff0 broken 4.1.0 6007838bb248

YimingWu added the

Module

Sculpt, Paint & Texture

label 2024-04-18 05:17:22 +02:00

Pratik Borhade commented

2024-04-18 07:21:57 +02:00

Hi, can confirm in this particular file. Unable to repro from scratch though.
Somehow forcing viewport update resolves this (changing current_frame, switching to edit mode, etc.)
Regression so raising the priority.

Hi, can confirm in this particular file. Unable to repro from scratch though. Somehow forcing viewport update resolves this (changing current_frame, switching to edit mode, etc.) Regression so raising the priority.

Pratik Borhade added

and removed

labels 2024-04-18 07:22:07 +02:00

Pratik Borhade commented

2024-04-18 08:01:36 +02:00

Broke between 09bd04697529 - 27992034f2e6

some changes on pbvh side possibly caused this.

Broke between `09bd04697529 - 27992034f2e6` some changes on pbvh side possibly caused this.

Sean Kim commented

2024-04-18 08:20:52 +02:00

I think I understand conceptually how this is happening and can probably have PR for this tomorrow. The hide tool only tags PBVH nodes for being updated when one of the node's own vertices changes visibility state. In this case, the faces below the x axis that are successfully hidden are part of the PBVH that own the vertex, but the ones above the x axis aren't.

static void vert_hide_update(Object &object,
                             const Span<PBVHNode *> nodes,
                             const FunctionRef<void(Span<int>, MutableSpan<bool>)> calc_hide)
{
  Mesh &mesh = *static_cast<Mesh *>(object.data);
  bke::MutableAttributeAccessor attributes = mesh.attributes_for_write();
  bke::SpanAttributeWriter<bool> hide_vert = attributes.lookup_or_add_for_write_span<bool>(
      ".hide_vert", bke::AttrDomain::Point);

  bool any_changed = false;
  threading::EnumerableThreadSpecific<Vector<bool>> all_new_hide;
  threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) {
    Vector<bool> &new_hide = all_new_hide.local();
    for (PBVHNode *node : nodes.slice(range)) {
      const Span<int> verts = BKE_pbvh_node_get_unique_vert_indices(node);

      new_hide.reinitialize(verts.size());
      array_utils::gather(hide_vert.span.as_span(), verts, new_hide.as_mutable_span());
      calc_hide(verts, new_hide);
      if (!array_utils::indexed_data_equal<bool>(hide_vert.span, verts, new_hide)) {
        continue;
      }

      any_changed = true;
      undo::push_node(&object, node, undo::Type::HideVert);
      array_utils::scatter(new_hide.as_span(), verts, hide_vert.span);

      BKE_pbvh_node_mark_update_visibility(node);
      bke::pbvh::node_update_visibility_mesh(hide_vert.span, *node);
    }
  });

  hide_vert.finish();
  if (any_changed) {
    bke::mesh_hide_vert_flush(mesh);
  }
}

I think I understand conceptually how this is happening and can probably have PR for this tomorrow. The hide tool only tags PBVH nodes for being updated when one of the node's own vertices changes visibility state. In this case, the faces below the x axis that are successfully hidden are part of the PBVH that own the vertex, but the ones above the x axis aren't. ```Cpp static void vert_hide_update(Object &object, const Span<PBVHNode *> nodes, const FunctionRef<void(Span<int>, MutableSpan<bool>)> calc_hide) { Mesh &mesh = *static_cast<Mesh *>(object.data); bke::MutableAttributeAccessor attributes = mesh.attributes_for_write(); bke::SpanAttributeWriter<bool> hide_vert = attributes.lookup_or_add_for_write_span<bool>( ".hide_vert", bke::AttrDomain::Point); bool any_changed = false; threading::EnumerableThreadSpecific<Vector<bool>> all_new_hide; threading::parallel_for(nodes.index_range(), 1, [&](const IndexRange range) { Vector<bool> &new_hide = all_new_hide.local(); for (PBVHNode *node : nodes.slice(range)) { const Span<int> verts = BKE_pbvh_node_get_unique_vert_indices(node); new_hide.reinitialize(verts.size()); array_utils::gather(hide_vert.span.as_span(), verts, new_hide.as_mutable_span()); calc_hide(verts, new_hide); if (!array_utils::indexed_data_equal<bool>(hide_vert.span, verts, new_hide)) { continue; } any_changed = true; undo::push_node(&object, node, undo::Type::HideVert); array_utils::scatter(new_hide.as_span(), verts, hide_vert.span); BKE_pbvh_node_mark_update_visibility(node); bke::pbvh::node_update_visibility_mesh(hide_vert.span, *node); } }); hide_vert.finish(); if (any_changed) { bke::mesh_hide_vert_flush(mesh); } } ```

Pratik Borhade commented

2024-04-18 08:57:13 +02:00

The hide tool only tags PBVH nodes for being updated when one of the node's own vertices changes visibility state

Yes, looks correct.

> The hide tool only tags PBVH nodes for being updated when one of the node's own vertices changes visibility state Yes, looks correct.

Sean Kim referenced a pull request that will close this issue

2024-04-18 23:22:31 +02:00

Fix #120761: Tag all PBVH nodes for needing a visibility update when using hide tools #120798

Hans Goudey added

and removed

labels 2024-04-22 23:53:31 +02:00

Sean Kim commented

2024-05-07 06:11:36 +02:00

As mentioned in the recent meeting notes, dropping the severity of this bug for now.

As mentioned in the [recent meeting notes](https://devtalk.blender.org/t/2024-04-30-sculpt-paint-texture-module-meeting/34415), dropping the severity of this bug for now.

Sean Kim removed the

Severity

High

label 2024-05-07 06:12:07 +02:00

Sean Kim self-assigned this 2024-05-08 22:47:17 +02:00

Sean Kim referenced a pull request that will close this issue

2024-05-11 01:38:54 +02:00

Fix #120761: Handle vert to face flushing in vert_hide_update #121678

Sean Kim referenced this issue from a commit

2024-05-14 21:25:09 +02:00

Fix #120761: Handle vert to face flushing in vert_hide_update

Sean Kim closed this issue