Fix T103559: Check for no-face object for particle baking
ClosedPublic
Actions

Authored by Sibo Van Gool (SiboVG) on Jan 8 2023, 4:23 PM.

Details

Reviewers

Pratik Borhade (PratikPB2123)
Campbell Barton (campbellbarton)

Maniphest Tasks

T103559: Crash on baking path particles when mesh don't have any faces

Summary

Meshes spawning particles from faces with with UV's/Vertex-colors but no faces would crash de-referencing a NULL pointer.

Resolve by adding a check for this case and an assertion if CD_MFACE is NULL when the mesh has polygons.

Diff Detail

Repository

rB Blender

Branch

TEMP-T103559-UPDATE (branched from master)

Build Status

Buildable 25374
Build 25374: arc lint + arc unit

Event Timeline

Sibo Van Gool (SiboVG) requested review of this revision.Jan 8 2023, 4:23 PM

Sibo Van Gool (SiboVG) created this revision.

Harbormaster completed remote builds in B25280: Diff 59225.Jan 8 2023, 4:24 PM

Sibo Van Gool (SiboVG) edited the summary of this revision. (Show Details)Jan 8 2023, 4:25 PM

Sibo Van Gool (SiboVG) added a reviewer: Pratik Borhade (PratikPB2123).

Sibo Van Gool (SiboVG) added projects: Nodes & Physics, BF Blender.

Sibo Van Gool (SiboVG) added a subscriber: Pratik Borhade (PratikPB2123).

Hi, thanks for the patch.
Instead check for mfaces outside the for loop and before initializing mface

Hey @Pratik Borhade (PratikPB2123), thanks for taking a look at it! Do you mean to check for a NULL face still inside particle_calculate_parent_uvs, but before MFace *mface = &mfaces[num];? Or do you mean to check for it at the initialization of

mtfaces[i] = (const MTFace *)CustomData_get_layer_n(
            &psmd->mesh_final->fdata, CD_MTFACE, i);

and

mtfaces[j] = (const MTFace *)CustomData_get_layer_n(
            &psmd->mesh_final->fdata, CD_MTFACE, j);

check for NULL in particle_calculate_parent_uvs

That CD_MFACE is missing seems like a bug, did you check on why MFace's are missing.
Further this patch should include a note explaining why this is missing.

If the MFace's cant be ensured, checking for null is better than crashing, but in that case the check should be on mfaces, before entering the loop not on mface as a null check on this value will only work if it's the first item in the array.

This revision now requires changes to proceed.Jan 9 2023, 7:09 AM

did you check on why MFace's are missing.

@Campbell Barton (campbellbarton) hi, in uploaded file, mesh don't really have any faces: T103559#1468314

T103559-simplified.blend908 KBDownload

In this case, an early return could be used in the case of a mesh without polygons/faces as there is no reason to loop over all UV layers and check for MFace's each iteration.

In this case though it would be better to assert that the mesh has no polygons, as this could silently hide the case that the CD_MFACE layer is not being generates from CD_MPOLY as it should be.

source/blender/draw/intern/draw_cache_impl_particles.c
312–323

Applied proposed changes

Harbormaster completed remote builds in B25290: Diff 59247.Jan 9 2023, 1:56 PM

Thanks for the proposed changes @Campbell Barton (campbellbarton)!

Thanks for updating. Could you also update the patch description to match with the current diff (explain why this change is done)?

Pratik Borhade (PratikPB2123) retitled this revision from [T103559] Check for no-face object for particle baking to Fix T103559: Check for no-face object for particle baking.Jan 10 2023, 4:26 PM

In D16947#462186, @Pratik Borhade (PratikPB2123) wrote:

Thanks for updating. Could you also update the patch description to match with the current diff (explain why this change is done)?

Done, hope it's good enough.

Thanks :)

Campbell Barton (campbellbarton) accepted this revision.Sun, Jan 15, 11:50 AM

This revision is now accepted and ready to land.Sun, Jan 15, 11:50 AM