Fix #108117: Crash in uv editor with hidden geometry #108130

Chris Blackbourn · 2023-05-22T03:48:27+02:00

Chris Blackbourn commented

2023-05-22 03:48:27 +02:00

Add NULL pointer protection for pointers returned from #BM_uv_element_get

Affects most UV transform tools, smart-stitch, sculptuv etc.

Add NULL pointer protection for pointers returned from #BM_uv_element_get Affects most UV transform tools, smart-stitch, sculptuv etc.

Chris Blackbourn added 1 commit 2023-05-22 03:48:38 +02:00

d217ad846e Fix #108117 : Crash in uv editor with hidden geometry

Add NULL pointer protection for pointers returned from #BM_uv_element_get

Affects most UV transform tools, smart-stitch, sculptuv etc.

Chris Blackbourn requested review from Campbell Barton 2023-05-22 03:48:50 +02:00

Campbell Barton commented

2023-05-22 04:22:35 +02:00

While this change is OK, I miss the wider context.

Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash?

Also, it would be good to update the doc-string for BM_uv_element_get noting under which conditions it will return NULL, since the caller might not be checking for hidden faces (assume this only happens when use_select & respecthide are set).

While this change is OK, I miss the wider context. Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash? Also, it would be good to update the doc-string for `BM_uv_element_get` noting under which conditions it will return NULL, since the caller might not be checking for hidden faces (assume this only happens when `use_select` & `respecthide` are set).

Chris Blackbourn added 1 commit 2023-05-22 07:04:35 +02:00

d7a6663ec2 UV: Improve correctness during #UvElementMap operations

Chris Blackbourn commented

2023-05-22 07:10:13 +02:00

Will create a 'Squash' commit:

Fix #108117: Crash in uv editor with hidden geometry

Many "UV island" style operations internally use #UvElementMap, including:

Transform tools
Smart-Stitch
UV Pinch, UV Grab and UV Relax sculpt tools.

Normally, every UV in the mesh is included in the #UvElementMap.
However, with hidden geometry, only the visible geometry is included.
This change enforces stricter usage, reducing the chance of crashes in other areas.

Will create a 'Squash' commit: ================== Fix #108117: Crash in uv editor with hidden geometry Many "UV island" style operations internally use #UvElementMap, including: * Transform tools * Smart-Stitch * UV Pinch, UV Grab and UV Relax sculpt tools. Normally, every UV in the mesh is included in the #UvElementMap. However, with hidden geometry, only the visible geometry is included. This change enforces stricter usage, reducing the chance of crashes in other areas.

Campbell Barton approved these changes 2023-05-22 11:27:47 +02:00

Philipp Oeser commented

2023-05-22 11:41:46 +02:00

check from 58c54b5859 Might possibly be redundant then? (havent checked though)

Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash?

For context: 8f543a73ab was the cause for that issue

check from 58c54b5859 Might possibly be redundant then? (havent checked though) > Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash? For context: 8f543a73abc42843fb924fc6d849e3055e3ae011 was the cause for that issue

Chris Blackbourn commented

2023-05-22 12:21:17 +02:00

check from 58c54b5859 Might possibly be redundant then? (havent checked though)

Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash?

For context: 8f543a73ab was the cause for that issue

I didn't fully sanitize the smart-stitch code as I don't have good test procedures for that area.
Best to keep both fixes...

> check from 58c54b5859 Might possibly be redundant then? (havent checked though) > > > Surely this would have caused crashes long ago if the functions were returning NULL, what changed recently that caused the crash? > > For context: 8f543a73abc42843fb924fc6d849e3055e3ae011 was the cause for that issue I didn't fully sanitize the smart-stitch code as I don't have good test procedures for that area. Best to keep both fixes...

Campbell Barton commented

2023-05-22 12:22:59 +02:00

In this case, it's worth mentioning the crash is a regression caused by 8f543a73ab (fix for T99659).

In this case, it's worth mentioning the crash is a regression caused by 8f543a73abc42843fb924fc6d849e3055e3ae011 (fix for T99659).

Chris Blackbourn referenced this issue from a commit

2023-05-22 23:23:29 +02:00

Fix #108117: Crash in uv editor with hidden geometry

Chris Blackbourn merged commit ae9ac99d2b into blender-v3.6-release

2023-05-22 23:23:30 +02:00

Chris Blackbourn deleted branch uv-fix-108117-2

2023-05-22 23:23:33 +02:00

Howard Trickey referenced this issue from a commit