Fix paint cursor crash
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Sep 17 2019, 11:28 AM.

Details

Reviewers

Pablo Dobarro (pablodp606)
Jeroen Bakker (jbakker)

Maniphest Tasks

T69957: Blender crash on 2d painting without active object (also drawing asserts)

Commits

rB41d6d1463f8e: Fix paint cursor crash

Summary

The new paint cursor (introduced in rBe0c792135adf) could crash for 2d painting without an active object.

Note there are still drawing asserts (because we are mixing 2d and 3d drawing in 'paint_draw_cursor'), but these will be handled in a seperate commit.

part of T69957

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Sep 17 2019, 11:28 AM

Harbormaster completed remote builds in B4983: Diff 18277.Sep 17 2019, 11:28 AM

also fixes T69948 afaict

@Philipp Oeser (lichtwerk) You mentioned that there were some drawing asserts. I haven't been able to reproduce them, not sure that the drawing code changes are needed.
The obact stuff is fine and solves the issue.

Do I miss something?

In D5820#133743, @Jeroen Bakker (jbakker) wrote:

@Philipp Oeser (lichtwerk) You mentioned that there were some drawing asserts. I haven't been able to reproduce them, not sure that the drawing code changes are needed.
Do I miss something?

@Jeroen Bakker (jbakker): you'll get these e.g. if you enable

blender: /blender/gpu/intern/gpu_immediate.c:457: immAttr2f: Assertion attr->comp_len == 2' failed.` (or attr->comp_len == 3, depending on where you are drawing)
It is just that code atm always adds 3D GPU_vertformat_attr_add, then we are drawing once with imm_draw_circle_wire_2d, once with imm_draw_circle_wire_3d...
Note that I think you get away with drawing imm_draw_circle_wire_3d even in 2D, but should be consistent here, no?
So we either just use always use imm_draw_circle_wire_3d, or be precise and do like the diff suggested...

Perhaps we should only use imm_draw_circle_wire_3d in this part of the code, as far as I see it we just need to replace one line. This reduces the complexity of the code and makes it more readable.

There are otherwise more places in this function that are fixed to 3d circle drawing, and they assumed that the 3d painting is required.

Jeroen Bakker (jbakker) requested changes to this revision.Sep 17 2019, 1:42 PM

Jeroen Bakker (jbakker) added inline comments.

source/blender/editors/sculpt_paint/paint_cursor.c
1306	Just fix to 3d cursor drawing. There are more places in this method that sticks to 3d drawing, that aren't touched by this patch that might lead to confusion when looking at the code in the future.

This revision now requires changes to proceed.Sep 17 2019, 1:42 PM

Philipp Oeser (lichtwerk) added inline comments.Sep 17 2019, 1:59 PM

source/blender/editors/sculpt_paint/paint_cursor.c
1306	There are more places in this method that sticks to 3d drawing that aren't touched by this patch Not sure what these would be? If you are talking about everything in `if ((mode == PAINT_MODE_SCULPT)`, that is implicitly not `is_2d_painting`... (could also put that in the condition as well, but like I said, it is implicit...) I mean, I dont have a strong opinion here, but sticking to 3D drawing in 2D is kinda confusing as well, right? I can try reshuffling the code a bit, so it is more clear when we are in 2D and when in 3D, maybe that makes sense? Even split out into 2 functions?