Refactor: clarify logic for 3D view dolly, improve docs

The dolly operators poll function was set to `view3d_rotation_poll`
instead of `view3d_zoom_or_dolly_poll` which reads like a mistake.

As it happens this didn't cause any user visible problems because
RV3D_LOCK_ZOOM_AND_DOLLY is only set when all other locks are set.

Nevertheless, logically the dolly operator should check that dolly
is not locked. Updated the poll function for dolly to check neither
rotation or zoom/dolly is locked with comments noting why both are
needed.

Also expand on code-comments for why dolly enforces perspective view.

source/blender/editors/space_view3d/view3d_navigate.cc

@@ -593,6 +593,13 @@ bool view3d_zoom_or_dolly_poll(bContext *C)
   return view3d_navigation_poll_impl(C, RV3D_LOCK_ZOOM_AND_DOLLY);
 }
 
+bool view3d_zoom_or_dolly_or_rotation_poll(bContext *C)
+{
+  /* This combination of flags is needed for the dolly operator,
+   * see code-comments there for details. */
+  return view3d_navigation_poll_impl(C, RV3D_LOCK_ZOOM_AND_DOLLY | RV3D_LOCK_ROTATION);
+}
+
 int view3d_navigate_modal_fn(bContext *C, wmOperator *op, const wmEvent *event)
 {
   ViewOpsData *vod = static_cast<ViewOpsData *>(op->customdata);

source/blender/editors/space_view3d/view3d_navigate.hh

@@ -204,6 +204,7 @@ struct ViewOpsData {
 bool view3d_location_poll(bContext *C);
 bool view3d_rotation_poll(bContext *C);
 bool view3d_zoom_or_dolly_poll(bContext *C);
+bool view3d_zoom_or_dolly_or_rotation_poll(bContext *C);
 
 int view3d_navigate_invoke_impl(bContext *C,
                                 wmOperator *op,

source/blender/editors/space_view3d/view3d_navigate_view_dolly.cc

@@ -234,9 +234,10 @@ static int viewdolly_exec(bContext *C, wmOperator *op)
   return OPERATOR_FINISHED;
 }
 
-/* copied from viewzoom_invoke(), changes here may apply there */
 static int viewdolly_invoke(bContext *C, wmOperator *op, const wmEvent *event)
 {
+  /* Near duplicate logic in #viewzoom_invoke(), changes here may apply there too. */
+
   ViewOpsData *vod;
 
   if (viewdolly_offset_lock_check(C, op)) {
@@ -250,8 +251,25 @@ static int viewdolly_invoke(bContext *C, wmOperator *op, const wmEvent *event)
 
   ED_view3d_smooth_view_force_finish(C, vod->v3d, vod->region);
 
-  /* needs to run before 'viewops_data_create' so the backup 'rv3d->ofs' is correct */
-  /* switch from camera view when: */
+  /* Rationale for enforcing a perspective projection:
+   *
+   * While translating the view center (the #RegionView3D::ofs) is possible,
+   * in most cases there is no user feedback that anything is changing,
+   * because only "panning" the view is shown in orthographic projections.
+   *
+   * From a user perspective it seems like a bug when interactive operators appear to do nothing,
+   * so force a perspective view.
+   *
+   * There are some exceptions where users would notice (mentioning for completeness),
+   * but they're obscure enough for the logic to stay as-is.
+   *
+   * - With a small far-clip plane "dolly" may move contents in/out of the visible clipping range.
+   * - With quad-view and "Sync Zoom/Pan" enabled, "dolly" will be visible other views.
+   *   We could even make an exception for this and allow dolly however even in this case
+   *   the user might as well pan the other views directly.
+   *
+   * NOTE: needs to run before #viewops_data_create so the backup `rv3d->ofs` is correct.
+   */
   if (vod->rv3d->persp != RV3D_PERSP) {
     if (vod->rv3d->persp == RV3D_CAMOB) {
       /* ignore rv3d->lpersp because dolly only makes sense in perspective mode */
@@ -316,7 +334,9 @@ void VIEW3D_OT_dolly(wmOperatorType *ot)
   ot->invoke = viewdolly_invoke;
   ot->exec = viewdolly_exec;
   ot->modal = viewdolly_modal;
-  ot->poll = view3d_rotation_poll;
+  /* Check rotation because this operator switches from orthographic to perspective view.
+   * See inline code-comments for details. */
+  ot->poll = view3d_zoom_or_dolly_or_rotation_poll;
   ot->cancel = view3d_navigate_cancel_fn;
 
   /* flags */

source/blender/editors/space_view3d/view3d_navigate_view_zoom.cc

@@ -519,9 +519,9 @@ static int viewzoom_invoke_impl(bContext *C,
   return OPERATOR_RUNNING_MODAL;
 }
 
-/* viewdolly_invoke() copied this function, changes here may apply there */
 static int viewzoom_invoke(bContext *C, wmOperator *op, const wmEvent *event)
 {
+  /* Near duplicate logic in #viewdolly_invoke(), changes here may apply there too. */
   return view3d_navigate_invoke_impl(C, op, event, &ViewOpsType_zoom);
 }
 

source/blender/makesdna/DNA_view3d_types.h

@@ -436,6 +436,12 @@ enum {
 
 /** #RegionView3D.viewlock */
 enum {
+  /**
+   * Used to lock axis views when quad-view is enabled.
+   *
+   * \note this implies locking the perspective as these views
+   * should use an orthographic projection.
+   */
   RV3D_LOCK_ROTATION = (1 << 0),
   RV3D_BOXVIEW = (1 << 1),
   RV3D_BOXCLIP = (1 << 2),