Fix T101062: sculpt curves crash using a paintcurve brush
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Sep 29 2022, 3:34 PM.

Details

Reviewers

Jacques Lucke (JacquesLucke)

Maniphest Tasks

T101062: Curves sculptmode: crash when using curve stroke in any tool

Commits

rB6810deb5215d: Fix T101062: sculpt curves crash using a paintcurve brush
rB3f91540cef7e: Fix T101062: sculpt curves crash using a paintcurve brush

Summary

For one, paintcurves were not considered in curves sculpt mode at all
(so you couldnt draw them). This is now enabled.

And the second issue was that since curves sculpt mode uses the reguar
paint_stroke_modal() [which handles paintcurves], this was actually
excuted, freeing the PaintStroke from SculptCurvesBrushStrokeData (but
not the CurvesSculptStrokeOperation) and immediately return
OPERATOR_FINISHED from modal (resulting in a double MEM_delete of
SculptCurvesBrushStrokeData -- in both invoke and modal).

There might be better ways to handle the memory free, for now the double
freeing is prevented by setting the operator customdata to NULL (and
check for that later).

Diff Detail

Repository

rB Blender

Branch

T101062 (branched from master)

Build Status

Buildable 24026
Build 24026: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Sep 29 2022, 3:34 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B24026: Diff 56290.Sep 29 2022, 3:34 PM

Philipp Oeser (lichtwerk) added a project: Nodes & Physics.Sep 29 2022, 3:34 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

Not sure I fully understand right now, does paint_stroke_modal call MEM_freeN on the customdata? Or is the customdata always freed with MEM_delete currently (but twice sometimes)?

In D16099#438524, @Jacques Lucke (JacquesLucke) wrote:

Not sure I fully understand right now, does paint_stroke_modal call MEM_freeN on the customdata? Or is the customdata always freed with MEM_delete currently (but twice sometimes)?

paint_stroke_modal calls
- paint_stroke_curve_end which calls
  - stroke_done which calls
    - PaintStroke> done (this is defined empty in curves_sculpt_ops.cc) but also
    - paint_stroke_free which calls
      - MEM_SAFE_FREE(stroke)

So half of the customdata is already freed in the case of using paint stroke curve, then from modal we get back to invoke (where freeing happens a second time).
Does this help?

Philipp Oeser (lichtwerk) mentioned this in T101062: Curves sculptmode: crash when using curve stroke in any tool.Oct 11 2022, 3:05 PM

Thanks. As you mention, only half of the customdata is freed with the current patch afaik. Would be better if everything is freed of course..
It would be good if the call to paint_stroke_modal sets op_data->stroke to null when the stroke is freed. This way it can't be freed twice here.

Jacques Lucke (JacquesLucke) accepted this revision.Oct 26 2022, 4:41 PM

This revision is now accepted and ready to land.Oct 26 2022, 4:41 PM

Closed by commit rB3f91540cef7e: Fix T101062: sculpt curves crash using a paintcurve brush (authored by Philipp Oeser (lichtwerk)). · Explain WhyNov 1 2022, 12:29 PM

This revision was automatically updated to reflect the committed changes.

Philipp Oeser (lichtwerk) added a commit: rB3f91540cef7e: Fix T101062: sculpt curves crash using a paintcurve brush.

Philipp Oeser (lichtwerk) added a commit: rB6810deb5215d: Fix T101062: sculpt curves crash using a paintcurve brush.Nov 28 2022, 2:22 PM