Page MenuHome

Crash with "Show on Cage" for geometry nodes mesh primitives
Closed, ResolvedPublicBUG

Description

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 1070 with Max-Q Design/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 451.67

Blender Version
Broken: version: 2.93.0 Alpha, branch: master, commit date: 2021-03-23 18:34, hash: rB3ea1779365b5
Worked: never

Short description of error
If you go into edit mode for an object with the "Show on cage" option selected on the Geometry Node modifier then, as soon as you connect a Geometry nodes primitive to the output, blender will crash.

Exact steps for others to reproduce the error

  • Load attached file
    • Contains an object already in Edit mode with a Geometry node modifier already attached
  • Connect the Cylinder output to the Group Output node
  • Observe crash
ExceptionCode         : EXCEPTION_ACCESS_VIOLATION
Exception Address     : 0x00007FF6FD88A308
Exception Module      : blender.exe
Exception Flags       : 0x00000000
Exception Parameters  : 0x2
	Parameters[0] : 0x0000000000000000
	Parameters[1] : 0xFFFFFFFFFFFFFFFF


Stack trace:
blender.exe         :0x00007FF6FD88A1E0  extract_edit_data_iter_poly_mesh
blender.exe         :0x00007FF6FD88F0C0  extract_run
blender.exe         :0x00007FF6FD88F3B0  extract_single_threaded_task_node_exec
tbb.dll             :0x00007FFBC3A454A0  tbb::interface7::internal::isolate_within_arena
blender.exe         :0x00007FF701CD4F10  tbb::flow::interface11::internal::function_body_leaf<tbb::flow::interface11::continue_msg,tbb::flow
blender.exe         :0x00007FF701CD5320  tbb::flow::interface11::internal::apply_body_task_bypass<tbb::flow::interface11::internal::continue
tbb.dll             :0x00007FFBC3A554D0  tbb::recursive_mutex::scoped_lock::internal_try_acquire
tbb.dll             :0x00007FFBC3A554D0  tbb::recursive_mutex::scoped_lock::internal_try_acquire
tbb.dll             :0x00007FFBC3A443E0  tbb::interface7::internal::task_arena_base::internal_execute
blender.exe         :0x00007FF701CD5A30  tbb::flow::interface10::graph::wait_for_all
blender.exe         :0x00007FF6FD85E7A0  DRW_mesh_batch_cache_create_requested
blender.exe         :0x00007FF6FD855640  drw_batch_cache_generate_requested
blender.exe         :0x00007FF6FD8348B0  drw_engines_cache_populate
blender.exe         :0x00007FF6FD831400  DRW_draw_render_loop_ex
blender.exe         :0x00007FF6FD832460  DRW_draw_view
blender.exe         :0x00007FF6FDFDBC00  view3d_main_region_draw
blender.exe         :0x00007FF6FDBA7250  ED_region_do_draw

Event Timeline

Dalai Felinto (dfelinto) changed the task status from Needs Triage to Confirmed.Mar 24 2021, 10:53 AM
Dalai Felinto (dfelinto) triaged this task as High priority.
Dalai Felinto (dfelinto) changed the subtype of this task from "Report" to "Bug".

Confirmed. Also marking as high priority since it is a crash.

It crashes for two reasons:

  • The CD_ORIGINDEX layer on the mesh is incorrect.
  • Drawing code does not check if indices in the CD_ORIGINDEX are in a valid range.

I'm not sure if CD_ORIGINDEX is a "best effort" structure or if it must be maintained correctly. Depending on that the drawing code has to be updated or not.

The CD_ORIGINDEX layer exists on the mesh, because it has been added by BM_mesh_bm_to_me_for_eval in create_cube_mesh. We can add an option to BM_mesh_bm_to_me_for_eval to disable creating original indices or use a different function like below.

diff --git a/source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cube.cc b/source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cube.cc
index 1803a13f651..f8a9bfd2ed1 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cube.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cube.cc
@@ -50,8 +50,10 @@ static Mesh *create_cube_mesh(const float size)
                size,
                true);
 
+  BMeshToMeshParams params{};
+  params.calc_object_remap = false;
   Mesh *mesh = (Mesh *)BKE_id_new_nomain(ID_ME, nullptr);
-  BM_mesh_bm_to_me_for_eval(bm, mesh, nullptr);
+  BM_mesh_bm_to_me(nullptr, bm, mesh, &params);
   BM_mesh_free(bm);
 
   return mesh;

Will leave the fix to @Hans Goudey (HooglyBoogly) because maybe this will also be fixed by D10730 when we don't do the bmesh round trip.

Thanks for the investigation @Jacques Lucke (JacquesLucke).

I'm not sure if CD_ORIGINDEX even makes sense for primitives, since there is no original geometry to back it up, right?

Since D10730 doesn't replace the cube primitive, I think we'll need to do something like you suggest anyway.
I mostly picked BM_mesh_bm_to_me_for_eval to avoid passing the params, so using BM_mesh_bm_to_me should work fine!

Hans Goudey (HooglyBoogly) renamed this task from Crash when attempting to enable "Show on cage" option for Geometry nodes containing primitives to Crash with "Show on Cage" for geometry nodes mesh primitives.Mar 24 2021, 3:10 PM
Hans Goudey (HooglyBoogly) claimed this task.