Page MenuHome

UI: support persistent state during number/slider interaction
ClosedPublic

Authored by Campbell Barton (campbellbarton) on Jul 9 2021, 6:38 AM.
Subscribers
None
Tokens
"Love" token, awarded by gilberto_rodrigues."Love" token, awarded by Rusculleda."Love" token, awarded by mano-wii.

Details

Summary

Support for begin/update/end callbacks allowing state to be cached
and reused while dragging a number button or slider.

This is done using UI_block_interaction_set to set callbacks.

  • Dragging multiple buttons at once is supported, passing multiple unique events into the update function.
  • Update is only called once even when multiple buttons are edited.
  • The update callback can detect the difference between click & drag actions so situations to support skipping cache creation and freeing for situations where it's not beneficial.

NOTE: this includes changes from view3d_buttons.c which would be split into a separate commit.

Motivation:

The initial motivation for this functionality was the speed up edit-mesh updates when dragging number buttons by caching information about the normals and face-triangulation that needs to be performed while dragging a button (as is currently done in transform code).

This can also be used to resolve T67442: Modifier re-evaluation makes editing dimensions unreliable, see reply noting that a persistent state during a drag action could resolve the problem.

Details:

Implementing this functionality ended up not being so straight-forward since multiple buttons can be edited at once and cancelling number buttons relies on uiAfterFunc, so supporting updating and freeing the cached data after the button it's self was freed needed to be supported.

Initializing the cache also needs to occur after multiple buttons have been selected (dragging vertically flags UI_BUT_DRAG_MULTI). So it's possible for the begin function to know which button events are being edited.

It's also important for this to run last, so there are checks to ensure this isn't added to uiAfterFunc multiple times.

Further Work:

There are larger refactors that I believe would be better handled as a separate patch.

  • Merge uiBlockInteraction_CallbackData.update_fn into the existing uiBlock.handle_func callback.

    This would involve the following changes.

    Add custom_data and event array arguments to and handle_func.

    Ensure handle_func is not called multiple times.
  • Move block level updates into a separate list to UIAfterFuncs, so multiple buttons may be edited at once, running the block handle afterwards without the need for the logic this patch introduces that moves the handler to the last uiAfterFunc.

Diff Detail

Repository
rB Blender
Branch
TEMP-UI-MODAL (branched from master)
Build Status
Buildable 15709
Build 15709: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) requested review of this revision.Jul 9 2021, 6:38 AM
Campbell Barton (campbellbarton) created this revision.
Campbell Barton (campbellbarton) retitled this revision from Modal UI Block Custom Data to UI: support persistant state during interaction.Jul 9 2021, 7:22 AM
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
  • Fix error building event array.
  • Fix error running update handler when editing a number as text.
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
  • Reduce the number of calls to ui_block_interaction_begin_ensure
Campbell Barton (campbellbarton) retitled this revision from UI: support persistant state during interaction to UI: support persistant state during number/slider button interaction.Jul 9 2021, 1:34 PM
Campbell Barton (campbellbarton) retitled this revision from UI: support persistant state during number/slider button interaction to UI: support persistant state during number/slider interaction.
  • remove useless comment

I'm of course a bit reluctant to adding this kind of complexity, but of course it may well be worth it here. Off-hand I don't see better ways to do it, multi-button editing makes this tricky indeed. Do you have any numbers to see how much of a performance improvement this gives?

I was also reluctant to add this although in principle being able to cache state used for the block handler is reasonable thing to do IMHO.

I tried to support this in the least intrusive way, but found it wasn't possible without a larger refactor. We could support this more cleanly, by postponing BUTTON_STATE_NUM_EDITING state until multiple buttons have been selected.
This way the begin function could be called in one place when editing the number button begins.
Although the end result might be similar to this patch, moving the calls to initialize data to an extra state (BUTTON_STATE_NUM_EDITING_PREPARE & BUTTON_STATE_NUM_EDITING for example).


The speedup allows for avoiding all tessellation & normal recalculation (in the best case), so the overall speedup boils down to how much of the time spent updating is taken by these operations.

From a quick test on a highly subdivided cube (1.5 million faces) this gives an overall 1.44x speedup when dragging number buttons.

(Tested after committing rB0f201049b4b2: Edit Mesh: tag the object data for updating instead of the object, as there was a bottleneck re-copying the evaluated mesh).

Campbell Barton (campbellbarton) retitled this revision from UI: support persistant state during number/slider interaction to UI: support persistent state during number/slider interaction.Jul 12 2021, 12:59 PM
source/blender/editors/include/UI_interface.h
536–537

The comment says this is two different things, is that right?

  • Improve & correct comments
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/include/UI_interface.h
536–537

Yes, updated comment for this and is_click too.

source/blender/editors/include/UI_interface.h
539

I find the term "events" a bit too ambiguous, at first I thought you'd pass actual WM events in there. Besides the 3D View code, do we use "events" for this elsewhere? To me it seems more like a button ID for manual applying where there is no better way to identify the button.

Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/include/UI_interface.h
539

Agree it's not great, I only named them this way since it's called int event in the callback, it could also be called but_retval_values for e.g.

Rename events to unique_retval_ids based on discussion with @Julian Eisel (Severin)

I didn't spend much time on this. But overall things seem reasonable, so fine with me.

This revision is now accepted and ready to land.Jul 13 2021, 11:56 AM
  • Remove view3d_buttons.c changes (these will be part of a separate commit)