Cleanup: Paint Cursor Refactor
ClosedPublic
Actions

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

Details

Reviewers

Sergey Sharybin (sergey)

Commits

rB5f5143818591: Cleanup: Paint Cursor Refactor

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

Branch

paint-cursor-refactor (branched from master)

Build Status

Buildable 8843
Build 8843: arc lint + arc unit

Event Timeline

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

Harbormaster completed remote builds in B8843: Diff 26527.Jul 4 2020, 5:41 PM

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

Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)Jul 4 2020, 5:42 PM

Pablo Dobarro (pablodp606) added a reviewer: Sergey Sharybin (sergey).

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

Rebase

Harbormaster completed remote builds in B9049: Diff 26879.Jul 16 2020, 6:13 PM

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 a subscriber: Hans Goudey (HooglyBoogly).Jul 16 2020, 6:57 PM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/editors/sculpt_paint/paint_cursor.c
1214–1216	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.

Pablo Dobarro (pablodp606) added inline comments.Jul 16 2020, 7:04 PM

source/blender/editors/sculpt_paint/paint_cursor.c
1214–1216	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