Page MenuHome

DRW concurrency: Avoid change to global theme state
Needs ReviewPublic

Authored by Julian Eisel (Severin) on Aug 5 2019, 11:55 AM.

Details

Summary

Changing the global state would obviously cause issues for async
execution. This is the simplest solution for a simple problem.
We could also make the global theme access thread safe, e.g. using
thread-local storage for g_theme_state or by making state changes
(i.e. UI_SetTheme) scoped, so we can maintain a mutex. Figured it's
better to keep it simple though.

Diff Detail

Repository
rB Blender
Branch
temp-concurrent-viewport-theme (branched from master)
Build Status
Buildable 4297
Build 4297: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) retitled this revision from Avoid change to global theme state in draw-manager to DRW concurrency: Avoid change to global theme state.Aug 5 2019, 12:06 PM
Julian Eisel (Severin) edited the summary of this revision. (Show Details)
Brecht Van Lommel (brecht) requested changes to this revision.Aug 5 2019, 4:33 PM

Fine with me in principle to make this more thread safe.

I see lots of other calls to UI_GetTheme* in the draw module though. It is not obvious to me that this covers all the cases, is there some rule about which code gets called where that guarantees this?

source/blender/editors/interface/resources.c
1385

Call this in UI_GetThemeColorShade4fv to deduplicate code?

1427

Same comment.

This revision now requires changes to proceed.Aug 5 2019, 4:33 PM

Yikes, indeed there are quite a few more calls to be covered. Wrote this while traveling so wasn't too attentive...

  • Cover all UI_GetTheme calls in draw manager
  • Share code of new functions in resources.c
Julian Eisel (Severin) marked 2 inline comments as done.Aug 5 2019, 6:58 PM
  • Cleanup: Remove unnecessary call & var

Are we certain that none of the drawing code in editors/space_view3d gets called here? Like gizmo drawing code?

If so then this seems fine, it's just not obvious to me still.