Page MenuHome

Gizmos: Keep navigation gizmo visible during modals
AbandonedPublic

Authored by Asher (ThatAsherGuy) on Nov 14 2020, 8:17 PM.
Tokens
"Love" token, awarded by jc4d."Love" token, awarded by juizoi."Like" token, awarded by Harti."Love" token, awarded by HooglyBoogly."Love" token, awarded by pablovazquez."Love" token, awarded by Regnas."100" token, awarded by NAS."Love" token, awarded by DotBow.

Details

Summary

The basic idea with this patch is to setup a clean way to keep the navigation gizmo visible during modals without changing if/when other gizmos are hidden.

To do that, I added another value to the View3d.gizmo_flag enum that's specifically for hiding gizmos during modals, swapped out V3D_GIZMO_HIDE for V3D_GIZMO_HIDE_MODAL in those relevant modals, and added new poll functions to a handful of gizmos that were previously using ED_gizmo_poll_or_unlink_delayed_from_tool() as a generic poll function. The navigation gizmo stays visible, since it doesn't look for V3D_GIZMO_HIDE_MODAL in its poll function, while the rest of the gizmos do their usual things.

My thought process behind approaching it this way is that I didn't want to change the meaning of V3D_GIZMO_HIDE, and I wanted avoid any secondary effects that would come from not setting a hide flag at all during modals. Doing it this way lets me lay some groundwork for T63743, as I don't see a way to solve that particular problem without adding a similar flag.

Diff Detail

Repository
rB Blender

Event Timeline

Asher (ThatAsherGuy) requested review of this revision.Nov 14 2020, 8:17 PM
Asher (ThatAsherGuy) created this revision.

This is more a design task, we should have a clear design before reviewing code.

I assumed that, because tasks like T73684 were flagged as duplicates of T63743, the design side of this was more or less settled. But I can drill down into the specifics in a design task; is there a specific facet of this that I should focus on?

Hans Goudey (HooglyBoogly) requested changes to this revision.Nov 26 2020, 12:16 AM

I ran into troubles building this:

/home/hans/Documents/Blender-Git/blender/source/blender/editors/mesh/editmesh_extrude_spin_gizmo.c: In function ‘gizmo_mesh_spin_poll’:
/home/hans/Documents/Blender-Git/blender/source/blender/editors/mesh/editmesh_extrude_spin_gizmo.c:121:1: error: control reaches end of non-void function [-Werror=return-type]

Same with other poll functions. Does this depend on another patch?

Just from my imagination, this change sounds so great!

This revision now requires changes to proceed.Nov 26 2020, 12:16 AM
Asher (ThatAsherGuy) added a comment.EditedNov 26 2020, 5:45 PM

Ah, looks like I need to stick else{return true;} on the ends of those. I'll get that done after the holiday.

Edit: Can't reproduce the error but I do get a related warning. Convention-wise, is it better for poll functions to return false by default and return true in conditionals, or vice-versa?

Fixed control path issue in gizmo poll functions

Convention-wise, is it better for poll functions to return false by default and return true in conditionals, or vice-versa?

I'd lean towards the latter, so how it is now.

The poll function is also duplicated a lot now, it would probably be better if it was extracted somewhere?

Also, I'm probably missing something but this isn't working for me.

Unified poll functions, set the flags on a few operators I'd missed previously, tweaked the draw prepare function for the bisect gizmo in order to fix a negative behavior that arose from enabling WM_GIZMOGROUPTYPE_DRAW_MODAL_ALL for it.

Also, I'm probably missing something but this isn't working for me.

I'm a bit confused here — the navigation gizmo is visible during the modals in your video. This patch doesn't make all gizmos visible during modals, just the navigation gizmo.

I'm a bit confused here — the navigation gizmo is visible during the modals in your video. This patch doesn't make all gizmos visible during modals, just the navigation gizmo.

Aha, duh! That's what I was missing. Sorry for the misunderstanding.

In that case my video is great proof that the change is working well! What a great step to reduce the flashing when using modal operators, this is awesome.

Code-wise this looks good to me. Campbell will probably want to check it too though.

source/blender/makesdna/DNA_view3d_types.h
603–604

A comment here about when this should be used and its relation to V3D_GIZMO_HIDE_TOOL would be helpful.

I really appreciate it when enums have this kind of documentation, and it's good practice anyway.

This revision is now accepted and ready to land.Jan 8 2021, 9:53 PM

This patch has an approved status however it was not committed yet. So I'm assuming the other reviewers are blocking. Updating the reviewers list to reflect this. This way the patch status still show as Need Review.

This revision now requires review to proceed.Mar 26 2021, 5:42 PM

Thanks for working on this!

It's been a while since the last update, could you please rebase this in master to make sure there are no conflicts? Then if @Campbell Barton (campbellbarton) gives it the final approval it can go in :D

Campbell Barton (campbellbarton) abandoned this revision.EditedSep 14 2021, 4:56 AM

This patch seems overly verbose, adding a new hide flag that is only set and checked while transforming, causing new poll functions & gizmo-group-type flags to be necessary.

Note that this patch is looking to lay the groundwork for T63743 (which is appreciated), and it may be this direction is ultimately necessary to properly support. However I'd rather not add this complexity preemptively when a fairly simple solution is sufficient to handle navigation gizmos.

Committed an alternate patch that simply hides tool & context gizmos while transforming (leaving navigation gizmos as-is) rB917a972b56af103aee406dfffe1f42745b5ad360, closing.

Might want to double-check that patch you committed. It doesn't cover scaling or extruding from a gizmo, or using the light gizmo, and it's introduced some weirdness with the shear gizmo. You need to add WM_GIZMOGROUPTYPE_DRAW_MODAL_ALL to gzgt->flag in all of the places that I did, if you want to prevent the gizmos themselves from hiding the navigation gizmo group.

@Asher (ThatAsherGuy) thanks for the feedback, these issues shouldn't be difficult to resolve.

@Asher (ThatAsherGuy) these issues should be resolved fb27a9bb983ce74b8d8f5f871cf0706dd1e25051

@Campbell Barton (campbellbarton) Don't forget to add your new WM_GIZMOGROUPTYPE_DRAW_MODAL_EXCLUDE flag to the bisect gizmo. That one breaks pretty hard if you let it update during modals.