Revert "Remove dedicated "show editing toolbar" action from 3d windows"

[Withalion]

Fixes #60313
I would like to open discussion on this PR as @uclaros pointed out some eloquent stuff, to which I agree. I would like to come to an agreement with both sides...

The points:

• I don't think the 3d view toolbar is bloated, it only has 12 buttons, which is 44 less than the main windows' default. If you think that's too many, I'd rather remove the semi-broken on-screen navigation relic button instead.
• Point cloud editing might be niche, but the toolbar may accommodate other layer types' editing tools in the future. Vector layer 3d editing is a much requested feature.
• Main window has the editing toolbar visible by default
• Toolbar position and visibility persist in the main window
  But more importantly:
• There is no visible clue that the toolbars are dynamic and one needs to right click. In the main window, toolbars have a handle and are movable. Users (or at least many of them) expect it to be right clickable, and even if not, there is a menu entry to manipulate the toolbars. In the secondary views however, both 2d and 3d, this is not the case.
• Right click does not work on the big blank space to the right of the last item of the main toolbar

[github-actions]

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 6a50a4f)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 6a50a4f)

[nyalldawson]

Ok, here's my counter-arguments and resolution proposal:

I don't think the 3d view toolbar is bloated, it only has 12 buttons, which is 44 less than the main windows' default. If you think that's too many,
I'd rather remove the semi-broken on-screen navigation relic button instead.

I would also like to see that removed, but that's a side issue. 😆

Point cloud editing might be niche, but the toolbar may accommodate other layer types' editing tools in the future. Vector layer 3d editing is a much requested feature.

While 11 or 12 actions might not seem like much, it's still UI bloat for a feature which CURRENTLY is of niche use. Sure, there's definitely demand and a LOT more potential users for vector editing in 3d, but we don't have that yet. And when that lands, we'll need to do a comprehensive rework of this 3D editing UI anyway, since the current editing bar is only designed around point cloud editing and won't scale to vector editing well.

Even with #60357 the UX in the editing toolbar is not user friendly. There's no clear indication in the toolbar that the functionality is linked to the main window active layer and there's no clue given as to why only some point cloud layers can be edited and not others. That kind of thing is fine for V1 of a brand new feature, but I also think it's fine for V1 to require users to manually enable this toolbar to get access to these tools.

(I think maybe the point cloud editing action should always be enabled, and a message bar used to give a message when editing can't be toggled for a particular layer -- eg ".ept point cloud layers cannot be edited")

(There's also the issue that the toolbar icon used here is just a reuse of the "add point cloud" layer one, which doesn't give any hint to users that it's got anything to do with editing layers!)

Main window has the editing toolbar visible by default

That's a completely different thing -- it's VECTOR editing, which is a huge component of GIS work. Point cloud editing is not 😄

Toolbar position and visibility persist in the main window

I'm +1 to having the toolbar visibility remembered for the 3d editing toolbar, that's an easy change

But more importantly:

There is no visible clue that the toolbars are dynamic and one needs to right click. In the main window, toolbars have a handle and are movable. Users (or at least many of them) expect it to be right clickable, and even if not, there is a menu entry to manipulate the toolbars. In the secondary views however, both 2d and 3d, this is not the case.
Right click does not work on the big blank space to the right of the last item of the main toolbar

Yep, that's an issue. Maybe we can solve it with an event filter on the toolbar to catch clicks on the unfilled portion of the widget?

Otherwise, how about we bring back the drop down menu for the "settings" action, with entries for "Show Point Cloud Editing Toolbar" and "Options...". That's nice and discoverable, yet doesn't contribute to UI complexity / bloat.

[nyalldawson]

Pinging @nirvn for UI feedback too

[nirvn]

Hey there; some friendly thoughts :)

• I did like the introduction of a right-click menu to enable / disable toolbars, and in the future panels; I felt it was a good way to manage the growth of functionality tied to 3D views. For e.g., we could have the on-screen navigation widget become a panel and also remove that toggle from the toolbar
• We do have a growing number of dialogs that rely on right-click menus to toggle visibility of toolbars and panels (the main window, the layout designer, the georeferencer, the processing modeler, etc.), so I also feel like the move doesn't depart from established UI/UX
• One way to go forward here would be to show the point cloud editing toolbar by default, which will inform user, and have them toggle the toolbar off via right click; alternatively we could throw a one-time message bar telling users to seek additional functionality by right-clicking on the toolbar

The PR opened by @uclaros ( #60357 ) is a nice improvement, and to me just further shouts the need for us to migrate to multiple toolbars in the 3D view panel/dialog the same way we do with other dialogs.

Also, definitively +1 to remembering 3D view toolbar (and panel) visibility across sessions, this is the way 😉

[uclaros]

Thanks @nyalldawson and @nirvn for the feedback.

Since we cannot have normal checkable/movable toolbars without a QMainWindow, my thought for this toolbar was to be some kind of a toolbar/ribbon hybrid that would also cater for vector editing in the future . Depending on the active layer we would show/hide the supported tools and depending on the selected tool we would show/hide the supported tools' widgets.

What would you say if we did the following:

• Add the "Toggle editing" button (pencil icon) to the main toolbar instead.
• Its enabled logic would be the same as in Add toggle editing and undo/redo buttons to point cloud editing toolbar #60357, following the current layer. (having it always enabled and showing an error message bar is not good ux imho, the disabled state is a great visual hint)
• Add a tooltip saying "Current layer cannot be edited in 3d views". This will hint that the ToC current layer is followed here. (I don't think we need to explicitly say why a layer cannot be edited in 3d, we don't do it for read only vector layers in 2d either. If user is in doubt, should rtfm! However we could add this info too.)
• If editing is toggled and supported for 3d, show the editing toolbar, containing undo/redo, then dynamically showing available tools for current layer (only point cloud tools for now), and then dynamically depending on the active tool, the tool's config widgets.

[nyalldawson]

@uclaros

Add the "Toggle editing" button (pencil icon) to the main toolbar instead.

+1, but I think this button should be "Show 3D Editing Tools" instead of "toggle editing", and it toggles the toolbar only.

Its enabled logic would be the same as in #60357, following the current layer. (having it always enabled and showing an error message bar is not good ux imho, the disabled state is a great visual hint)
Add a tooltip saying "Current layer cannot be edited in 3d views". This will hint that the ToC current layer is followed here. (I don't think we need to explicitly say why a layer cannot be edited in 3d, we don't do it for read only vector layers in 2d either. If user is in doubt, should rtfm! However we could add this info too.)
If editing is toggled and supported for 3d, show the editing toolbar, containing undo/redo, then dynamically showing available tools for current layer (only point cloud tools for now), and then dynamically depending on the active tool, the tool's config widgets.

Counter-proposal 😆

• Don't show toggles for enabling/disabling layer edit state in the 3d view at all, and require users to to do this via the main window. Then we have natural ways to give feedback to users (eg the message bar), and provider detailed messages when a layer cannot be made editable. Layer save operations would also be done via the main window only, with feedback presented in that window if eg the save fails.
• Add an "editable" flag for Qgis::LayerFilter, and update QgsMapLayerProxyModel so that we can get a model which only includes map layers which have edit mode currently enabled
• The first widget/action in the 3d edit toolbar would be a "target layer" one. This could be a combo box showing only editable layers (see point above), or (probably nicer) a button which shows a drop down menu of available layers populated using the filtered map layer model. (For now this could also be filtered to point cloud layers only, but later we'd also show vector/mesh/...)
• The layer selected as the "target layer" is the one ALWAYS being edited by THAT 3d map window, and we remove all links to main map window active layer. This also would permit users to open multiple 3d windows and edit side-by-side two different layers, rather then having to jump back to the main window, change the active layer, continue edits. It also ensures that the user is shown which layer is being edited DIRECTLY in the 3d window, instead of relying on them to know that this follow the main window selected layer. Much more user friendly this way.

[uclaros]

• Add an "editable" flag for Qgis::LayerFilter, and update QgsMapLayerProxyModel so that we can get a model which only includes map layers which have edit mode currently enabled

• The first widget/action in the 3d edit toolbar would be a "target layer" one. This could be a combo box showing only editable layers (see point above), or (probably nicer) a button which shows a drop down menu of available layers populated using the filtered map layer model. (For now this could also be filtered to point cloud layers only, but later we'd also show vector/mesh/...)

• The layer selected as the "target layer" is the one ALWAYS being edited by THAT 3d map window, and we remove all links to main map window active layer. This also would permit users to open multiple 3d windows and edit side-by-side two different layers, rather then having to jump back to the main window, change the active layer, continue edits. It also ensures that the user is shown which layer is being edited DIRECTLY in the 3d window, instead of relying on them to know that this follow the main window selected layer. Much more user friendly this way.

From my experience, users often have a very hard time identifying their layers just by name, outside of the ToC context.
That's why I'd prefer to select the current layer on the main ToC and not the 3d window itself.
Point cloud files very often have long filenames too which would not fit well in the 3d view's toolbars.
While per-view-editing-layer is an interesting concept (that would be a new thing for qgis, we always respect active layer) it is kind of a special case. I'd be more interested in supporting what I consider a common scenario:

• Have lots of point cloud layers
• Identify the offending points (in 2d or 3d)
• Right click the result > Activate layer
• Toggle editing
• etc

ps. my clumsy conflict resolution in #60357 brought the button back 😁

[wonder-sk]

Let's link also similar ongoing discussion in qgis/QGIS-Enhancement-Proposals#328 (comment) about how editing toolbars should be handled in the elevation profile views.

Summary of what we have so far:

• attribute table (which can be either a dock widget or a standalone top-level window) has a toolbar button to enable/disable editing, plus a couple of editing buttons (but it's clear which layer they are related to). The toolbar is fixed.
• print layout, georeferencer and some other QMainWindow objects have toolbars that can be shown/hidden
• now 3d map view and elevation profile view are yet a bit different - they are dockable widgets / standalone windows like attribute table, and they have fixed toolbar. Yet they are not linked to a particular layer - so they either need to link to the active layer, or have their own target layer picker - and neither of these options is ideal.
• elevation profile view has its own layer tree view, which could be in fact used to pick the target layer, and in theory 3d views could get the same thing - but I had the impression that users sometimes struggle a bit with the behavior of this extra layer tree

Probably the best thing would be to rethink how the top-level windows and dockable widgets work. The Blender approach seems quite good, with potentially multiple split screens, each containing potentially something else, and toolbars being contextual to the particular view (and there's also an option to have multiple top-level windows for those with multiple displays). Like this, one could have just 2D canvas - or 2D canvas with print layout next to it - or even just 3D canvas and elevation profile with no 2D canvas. This could also help to declutter the messy toolbars in the main window, but I understand, that would be a LOT of work 😄

[nyalldawson]

Ok, I'm not sure of a common way forward here, so I'll officially register my displeasure with the current UI/UX and hope that this gets fixed in a future release.

[uclaros]

@nyalldawson how about something like this:

All the actions that toggle a widget are grouped under the config button.
It's also a first step to hide the on-screen navigation thing!