Page MenuHome

Cleanup: Paint Cursor Refactor
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Jul 4 2020, 5:41 PM.

Details

Summary

The paint_draw_cursor function was handling the cursor drawing for 2D
and 3D views of all paint modes, calculating the brush radius, updating
the SculptSession data and updating and drawing all sculpt cursor
overlays for different tools. It was almost impossible to understand when
and what was being drawn and in which state the GPU matrix was.

Now everyting is organized into different functions, with clear
separation between modes, sculpt tool overlays and different drawing
setups. Update and drawing functions are also separated (this allows to
skip one PBVH query on each cursor drawing).

Diff Detail

Repository
rB Blender

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Jul 4 2020, 5:41 PM

From quick look seems much better. Are there any expected functional changes?
If you gave a good test, just commit?

This should not have any functional changes. I tested it again and it seems to be working fine, so I'll commit it tomorrow

@Jeroen Bakker (jbakker) This splits the drawing and update logic intro different functions, so now it should be easier to work on the proposal you did in D7174. I can also organize the functions in a different way if needed to make the drawing faster.

Hans Goudey (HooglyBoogly) added inline comments.
source/blender/editors/sculpt_paint/paint_cursor.c
1209–1211

I might be missing something and excuse me if I am. But is it necessary to store these outside of bContext above? I don't see their values being overridden from what's already in C in this patch.

source/blender/editors/sculpt_paint/paint_cursor.c
1209–1211

My intention was to not add bContext to the struct so everything is initialized in paint_cursor_context_init(), but it is needed because it is an argument of SCULPT_cursor_geometry_info_update(). It can probably be removed if I also include the refactor of that function as part of this patch, but I think it that would include too many changes for a single commit

source/blender/editors/sculpt_paint/paint_cursor.c
1209–1211

Makes sense, thanks for indulging my curiosity.

source/blender/editors/sculpt_paint/paint_cursor.c
1812

static void paint_cursor_restore_drawing_state(void)

Otherwise it is not a strict prototype (-Werror=strict-prototypes)

Pablo Dobarro (pablodp606) marked an inline comment as done.
  • Rebase
  • Review Update

Think is really better to just commit this. Can't spot anything obviously wrong, there are no compiler warnings.

This revision is now accepted and ready to land.Thu, Aug 6, 9:39 AM
This revision was automatically updated to reflect the committed changes.