griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

View 3D: Refactor code to split functions and remove duplication

Browse Source 
No functional changes.

Previously, each navigation operation had its own method for restoring
the 3D View's state upon cancellation.

However, this approach did not provide significant advantages.

The number of affected variables was too small to justify considering
it for performance optimization, and the cancellation code was
duplicated across different operations.

To address this, the present commit unifies all restore code into a
single function:

- `void viewops_data_state_restore(ViewOpsData *vod);`

As a result, the `viewmove_apply_reset` function has been removed.

Moreover, since the state now has a dedicated restore function, it
seemed appropriate to create a function for capturing the state as
well.

Therefore, the code responsible for capturing the state (previously
scattered within `viewops_data_init_navigation`) has been extracted to
create the function:

- `void viewops_data_state_capture(ViewOpsData *vod);`

During this separation process, a potential error in the code was
identified, where the state may not be fully restored.

Although this issue persists, a comment has been added to clarify this
situation.

Pull Request: https://projects.blender.org/blender/blender/pulls/109760
This commit is contained in:
Germano Cavalcante
2023-07-06 18:21:36 +02:00
committed by Germano Cavalcante
parent c9fbbea261
commit d9829c2f87
7 changed files with 134 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

145
source/blender/editors/space_view3d/view3d_navigate.cc
View File

@@ -530,6 +530,99 @@ static void viewops_data_init_context(bContext *C, ViewOpsData *vod)
vod->rv3d = static_cast<RegionView3D *>(vod->region->regiondata);
}
static void viewops_data_state_capture(ViewOpsData *vod)
{
Depsgraph *depsgraph = vod->depsgraph;
View3D *v3d = vod->v3d;
RegionView3D *rv3d = vod->rv3d;
/* Set the view from the camera, if view locking is enabled.
* we may want to make this optional but for now its needed always.
* NOTE: This changes the value of `rv3d->dist`. */
ED_view3d_camera_lock_init(depsgraph, v3d, rv3d);
copy_v3_v3(vod->init.ofs, rv3d->ofs);
copy_v3_v3(vod->init.ofs_lock, rv3d->ofs_lock);
vod->init.camdx = rv3d->camdx;
vod->init.camdy = rv3d->camdy;
vod->init.camzoom = rv3d->camzoom;
vod->init.dist = rv3d->dist;
copy_qt_qt(vod->init.quat, rv3d->viewquat);
vod->init.persp = rv3d->persp;
vod->init.view = rv3d->view;
vod->init.view_axis_roll = rv3d->view_axis_roll;
}
void viewops_data_state_restore(ViewOpsData *vod)
{
/* DOLLY, MOVE, ROTATE and ZOOM. */
{
/* For Move this only changes when offset is not locked. */
/* For Rotate this only changes when rotating around objects or last-brush. */
/* For Zoom this only changes when zooming to mouse position. */
/* Note this does not remove auto-keys on locked cameras. */
copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
}
/* MOVE and ZOOM. */
{
/* For Move this only changes when offset is not locked. */
/* For Zoom this only changes when zooming to mouse position in camera view. */
vod->rv3d->camdx = vod->init.camdx;
vod->rv3d->camdy = vod->init.camdy;
}
/* MOVE. */
{
if ((vod->rv3d->persp == RV3D_CAMOB) && !ED_view3d_camera_lock_check(vod->v3d, vod->rv3d)) {
// vod->rv3d->camdx = vod->init.camdx;
// vod->rv3d->camdy = vod->init.camdy;
}
else if (ED_view3d_offset_lock_check(vod->v3d, vod->rv3d)) {
copy_v2_v2(vod->rv3d->ofs_lock, vod->init.ofs_lock);
}
else {
// copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
if (RV3D_LOCK_FLAGS(vod->rv3d) & RV3D_BOXVIEW) {
view3d_boxview_sync(vod->area, vod->region);
}
}
}
/* ZOOM. */
{
vod->rv3d->camzoom = vod->init.camzoom;
}
/* ROTATE and ZOOM. */
{
/**
* For Rotate this only changes when orbiting from a camera view.
* In this case the `dist` is calculated based on the camera relative to the `ofs`.
*/
/* Note this does not remove auto-keys on locked cameras. */
vod->rv3d->dist = vod->init.dist;
}
/* ROLL and ROTATE. */
{
/* Note this does not remove auto-keys on locked cameras. */
copy_qt_qt(vod->rv3d->viewquat, vod->init.quat);
}
/* ROTATE. */
{
vod->rv3d->persp = vod->init.persp;
vod->rv3d->view = vod->init.view;
vod->rv3d->view_axis_roll = vod->init.view_axis_roll;
}
/* NOTE: there is no need to restore "last" values (as set by #ED_view3d_lastview_store). */
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
}
static void viewops_data_init_navigation(bContext *C,
const wmEvent *event,
const eV3D_OpMode nav_type,
@@ -537,6 +630,8 @@ static void viewops_data_init_navigation(bContext *C,
ViewOpsData *vod)
{
Depsgraph *depsgraph = vod->depsgraph;
ARegion *region = vod->region;
View3D *v3d = vod->v3d;
RegionView3D *rv3d = vod->rv3d;
eViewOpsFlag viewops_flag = viewops_flag_from_prefs();
@@ -597,7 +692,8 @@ static void viewops_data_init_navigation(bContext *C,
vod->use_dyn_ofs = false;
}
vod->init.persp = rv3d->persp;
viewops_data_state_capture(vod);
if (viewops_flag & VIEWOPS_FLAG_PERSP_ENSURE) {
if (ED_view3d_persp_ensure(depsgraph, vod->v3d, vod->region)) {
@@ -607,16 +703,7 @@ static void viewops_data_init_navigation(bContext *C,
}
}
/* set the view from the camera, if view locking is enabled.
* we may want to make this optional but for now its needed always */
ED_view3d_camera_lock_init(depsgraph, vod->v3d, vod->rv3d);
vod->init.persp_with_auto_persp_applied = rv3d->persp;
vod->init.view = rv3d->view;
vod->init.view_axis_roll = rv3d->view_axis_roll;
vod->init.dist = rv3d->dist;
vod->init.camzoom = rv3d->camzoom;
copy_qt_qt(vod->init.quat, rv3d->viewquat);
copy_v2_v2_int(vod->init.event_xy, event->xy);
copy_v2_v2_int(vod->prev.event_xy, event->xy);
@@ -630,14 +717,8 @@ static void viewops_data_init_navigation(bContext *C,
}
vod->init.event_type = event->type;
copy_v3_v3(vod->init.ofs, rv3d->ofs);
copy_qt_qt(vod->curr.viewquat, rv3d->viewquat);
copy_v3_v3(vod->init.ofs_lock, rv3d->ofs_lock);
vod->init.camdx = rv3d->camdx;
vod->init.camdy = rv3d->camdy;
if (viewops_flag & VIEWOPS_FLAG_ORBIT_SELECT) {
float ofs[3];
if (view3d_orbit_calc_center(C, ofs) || (vod->use_dyn_ofs == false)) {
@@ -675,32 +756,35 @@ static void viewops_data_init_navigation(bContext *C,
/* find a new ofs value that is along the view axis
* (rather than the mouse location) */
closest_to_line_v3(dvec, vod->dyn_ofs, my_pivot, my_origin);
vod->init.dist = rv3d->dist = len_v3v3(my_pivot, dvec);
rv3d->dist = len_v3v3(my_pivot, dvec);
negate_v3_v3(rv3d->ofs, dvec);
}
else {
const float mval_region_mid[2] = {float(vod->region->winx) / 2.0f,
float(vod->region->winy) / 2.0f};
const float mval_region_mid[2] = {float(region->winx) / 2.0f, float(region->winy) / 2.0f};
ED_view3d_win_to_3d(vod->v3d, vod->region, vod->dyn_ofs, mval_region_mid, rv3d->ofs);
ED_view3d_win_to_3d(v3d, region, vod->dyn_ofs, mval_region_mid, rv3d->ofs);
negate_v3(rv3d->ofs);
}
negate_v3(vod->dyn_ofs);
/* XXX: The initial state captured by `viewops_data_state_capture` is being modified here.
* This causes the state when canceling a navigation operation to not be fully restored. */
vod->init.dist = rv3d->dist;
copy_v3_v3(vod->init.ofs, rv3d->ofs);
}
}
/* For dolly */
const float mval[2] = {float(event->mval[0]), float(event->mval[1])};
ED_view3d_win_to_vector(vod->region, mval, vod->init.mousevec);
ED_view3d_win_to_vector(region, mval, vod->init.mousevec);
{
int event_xy_offset[2];
add_v2_v2v2_int(event_xy_offset, event->xy, vod->init.event_xy_offset);
/* For rotation with trackball rotation. */
calctrackballvec(&vod->region->winrct, event_xy_offset, vod->init.trackvec);
calctrackballvec(&region->winrct, event_xy_offset, vod->init.trackvec);
}
{
@@ -904,23 +988,6 @@ void viewmove_apply(ViewOpsData *vod, int x, int y)
ED_region_tag_redraw(vod->region);
}
void viewmove_apply_reset(ViewOpsData *vod)
{
if ((vod->rv3d->persp == RV3D_CAMOB) && !ED_view3d_camera_lock_check(vod->v3d, vod->rv3d)) {
vod->rv3d->camdx = vod->init.camdx;
vod->rv3d->camdy = vod->init.camdy;
}
else if (ED_view3d_offset_lock_check(vod->v3d, vod->rv3d)) {
copy_v2_v2(vod->rv3d->ofs_lock, vod->init.ofs_lock);
}
else {
copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
if (RV3D_LOCK_FLAGS(vod->rv3d) & RV3D_BOXVIEW) {
view3d_boxview_sync(vod->area, vod->region);
}
}
}
/** \} */
/* -------------------------------------------------------------------- */

45
source/blender/editors/space_view3d/view3d_navigate.h
View File

@@ -113,9 +113,27 @@ typedef struct ViewOpsData {
/** Viewport state on initialization, don't change afterwards. */
struct {
float dist;
float camzoom;
float quat[4];
/** These variables reflect the same in #RegionView3D. */
float ofs[3]; /* DOLLY, MOVE, ROTATE and ZOOM. */
float ofs_lock[2]; /* MOVE. */
float camdx, camdy; /* MOVE and ZOOM. */
float camzoom; /* ZOOM. */
float dist; /* ROTATE and ZOOM. */
float quat[4]; /* ROLL and ROTATE. */
char persp; /* ROTATE. */
char view; /* ROTATE. */
char view_axis_roll; /* ROTATE. */
/**
* #RegionView3D.persp set after auto-perspective is applied.
* If we want the value before running the operator, add a separate member.
*/
char persp_with_auto_persp_applied;
/** The ones below are unrelated to the state of the 3D view. */
/** #wmEvent.xy. */
int event_xy[2];
/** Offset to use when #VIEWOPS_FLAG_USE_MOUSE_INIT is not set.
@@ -123,32 +141,15 @@ typedef struct ViewOpsData {
int event_xy_offset[2];
/** #wmEvent.type that triggered the operator. */
int event_type;
float ofs[3];
/** #RegionView3D.ofs_lock */
float ofs_lock[2];
/** Initial distance to 'ofs'. */
float zfac;
/** Camera offset. */
float camdx, camdy;
/** Trackball rotation only. */
float trackvec[3];
/** Dolly only. */
float mousevec[3];
/**
* #RegionView3D.persp set after auto-perspective is applied.
* If we want the value before running the operator, add a separate member.
*/
char persp_with_auto_persp_applied;
/** #RegionView3D.persp set after before auto-perspective is applied. */
char persp;
/** #RegionView3D.view */
char view;
/** #RegionView3D.view_axis_roll */
char view_axis_roll;
/** Used for roll */
struct Dial *dial;
} init;
@@ -212,7 +213,6 @@ void view3d_navigate_cancel_fn(struct bContext *C, struct wmOperator *op);
void calctrackballvec(const struct rcti *rect, const int event_xy[2], float r_dir[3]);
void viewmove_apply(ViewOpsData *vod, int x, int y);
void viewmove_apply_reset(ViewOpsData *vod);
void view3d_orbit_apply_dyn_ofs(float r_ofs[3],
const float ofs_old[3],
const float viewquat_old[4],
@@ -235,6 +235,7 @@ ViewOpsData *viewops_data_create(bContext *C,
const wmEvent *event,
const eV3D_OpMode nav_type,
const bool use_cursor_init);
void viewops_data_state_restore(ViewOpsData *vod);
void VIEW3D_OT_view_all(struct wmOperatorType *ot);
void VIEW3D_OT_view_selected(struct wmOperatorType *ot);

4
source/blender/editors/space_view3d/view3d_navigate_dolly.c
View File

@@ -163,9 +163,7 @@ static int viewdolly_modal(bContext *C, wmOperator *op, const wmEvent *event)
break;
}
case VIEW_CANCEL: {
/* Note this does not remove auto-keys on locked cameras. */
copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
viewops_data_state_restore(vod);
ret = OPERATOR_CANCELLED;
break;
}

3
source/blender/editors/space_view3d/view3d_navigate_move.c
View File

@@ -73,8 +73,7 @@ int viewmove_modal_impl(bContext *C,
break;
}
case VIEW_CANCEL: {
viewmove_apply_reset(vod);
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
viewops_data_state_restore(vod);
ret = OPERATOR_CANCELLED;
break;
}

4
source/blender/editors/space_view3d/view3d_navigate_roll.c
View File

@@ -140,9 +140,7 @@ static int viewroll_modal(bContext *C, wmOperator *op, const wmEvent *event)
break;
}
case VIEW_CANCEL: {
/* Note this does not remove auto-keys on locked cameras. */
copy_qt_qt(vod->rv3d->viewquat, vod->init.quat);
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
viewops_data_state_restore(vod);
ret = OPERATOR_CANCELLED;
break;
}

16
source/blender/editors/space_view3d/view3d_navigate_rotate.c
View File

@@ -317,21 +317,7 @@ int viewrotate_modal_impl(bContext *C,
break;
}
case VIEW_CANCEL: {
/* Note this does not remove auto-keys on locked cameras. */
copy_qt_qt(vod->rv3d->viewquat, vod->init.quat);
/* The offset may have change when rotating around objects or last-brush. */
copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
/* The dist may have changed when orbiting from a camera view.
* In this case the `dist` is calculated based on the camera relative to the `ofs`. */
vod->rv3d->dist = vod->init.dist;
vod->rv3d->persp = vod->init.persp;
vod->rv3d->view = vod->init.view;
vod->rv3d->view_axis_roll = vod->init.view_axis_roll;
/* NOTE: there is no need to restore "last" values (as set by #ED_view3d_lastview_store). */
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
viewops_data_state_restore(vod);
ret = OPERATOR_CANCELLED;
break;
}

11
source/blender/editors/space_view3d/view3d_navigate_zoom.c
View File

@@ -369,16 +369,7 @@ int viewzoom_modal_impl(bContext *C,
break;
}
case VIEW_CANCEL: {
/* Note this does not remove auto-keys on locked cameras. */
vod->rv3d->dist = vod->init.dist;
/* The offset may have change when zooming to mouse position. */
copy_v3_v3(vod->rv3d->ofs, vod->init.ofs);
vod->rv3d->camzoom = vod->init.camzoom;
/* Zoom to mouse position in camera view changes these values. */
vod->rv3d->camdx = vod->init.camdx;
vod->rv3d->camdy = vod->init.camdy;
ED_view3d_camera_lock_sync(vod->depsgraph, vod->v3d, vod->rv3d);
viewops_data_state_restore(vod);
ret = OPERATOR_CANCELLED;
break;
}