Fix #116262: Modal knife tool can crash if used on hidden object #119383

Philipp Oeser · 2024-03-12T15:28:02+01:00

Philipp Oeser commented

2024-03-12 15:28:02 +01:00

This could happen if objects were hidden after going to editmode and
knife was used on them.

In that case, raycasting would (rightfully) fail, but there is a
fallback in place using the back-buffer selection method (in which the
hidden object is still present). So a face would be found, but this
makes all following code confusing/wrong since we are working with
coordinates / faces under the assumption there would be a valid/
corresponding object to it in KnifeTool_OpData > objects -- which is
not the case...

So to resolve, just check if the object is visible before calling
EDBM_face_find_nearest in knife code.

Alternativeliy, we could also add a check for Base viewport visibility
to all the bmesh find_nearest functions (a bit hesitant though since i
am not sure this would be desired in all cases)

This could happen if objects were hidden after going to editmode and knife was used on them. In that case, raycasting would (rightfully) fail, but there is a fallback in place using the back-buffer selection method (in which the hidden object is still present). So a face would be found, but this makes all following code confusing/wrong since we are working with coordinates / faces under the assumption there would be a valid/ corresponding object to it in `KnifeTool_OpData` > `objects` -- which is not the case... So to resolve, just check if the object is visible before calling `EDBM_face_find_nearest` in knife code. Alternativeliy, we could also add a check for Base viewport visibility to all the bmesh `find_nearest` functions (a bit hesitant though since i am not sure this would be desired in all cases)

Philipp Oeser added 1 commit 2024-03-12 15:28:11 +01:00

e0e36ed5a7 Fix #116262 : Modal knife tool can crash if used on hidden object

This could happen if objects were hidden after going to editmode and
knife was used on them.

In that case, raycasting would (rightfully) fail, but there is a
fallback in place using the back-buffer selection method (in which the
hidden object is still present). So a face would be found, but this
makes all following code confusing/wrong since we are working with
coordinates / faces under the assumption there would be a valid/
corresponding object to it in `KnifeTool_OpData` > `objects` -- which is
not the case...

So to resolve, just check if the object is visible before calling
`EDBM_face_find_nearest` in knife code.

Alternativeliy, we could also add a check for Base viewport visibility
to all the bmesh `find_nearest` functions (a bit hesitant though since i
am not sure this would be desired in all cases)

Philipp Oeser added this to the Modeling project 2024-03-12 15:30:47 +01:00

Philipp Oeser requested review from Germano Cavalcante 2024-03-12 15:30:54 +01:00

Philipp Oeser requested review from Campbell Barton 2024-03-12 15:31:00 +01:00

Germano Cavalcante approved these changes 2024-03-12 16:05:47 +01:00

source/blender/editors/mesh/editmesh_knife.cc

						
				@ -3214,3 +3214,3 @@

				      vc.mval[1] = int(kcd->curr.mval[1]);

				      f = EDBM_face_find_nearest(&vc, &dist);

				      if (BKE_object_is_visible_in_viewport(vc.v3d, vc.obact)) {

Germano Cavalcante commented

2024-03-12 16:02:55 +01:00

It looks like the ViewContext and comment can be moved inside this block.

Ideally all objects in kcd->objects should be tested for "nearest".

And for further improvements, the back-buffer could not even be used if we used the BVH nearest function instead.

But these changes are more involved. For bug fixing, it looks fine.

It looks like the `ViewContext` and comment can be moved inside this block. Ideally all objects in `kcd->objects` should be tested for "nearest". And for further improvements, the back-buffer could not even be used if we used the BVH nearest function instead. But these changes are more involved. For bug fixing, it looks fine.

Philipp Oeser commented

2024-03-12 16:36:44 +01:00

Yep, yep and yep.

Will do separately.

Yep, yep and yep. Will do separately.