Page MenuHome

Fix T66802: Bug with "Transfer: Edge Slide" when an edge loop is fully occluded
Needs ReviewPublic

Authored by Germano Cavalcante (mano-wii) on Jul 13 2019, 8:14 PM.

Details

Summary

When an edge loop is fully occluded the direction of movement is not calculated resulting in unpredictable behavior.

This patch proposes to always calculate the direction of the edge loop and only prevent occluded edges from affecting mval_dir value.

(A loop change is also proposed to increase efficiency).

Report: T66802

Diff Detail

Repository
rB Blender
Branch
arcpatch-D5247_1 (branched from master)
Build Status
Buildable 4090
Build 4090: arc lint + arc unit

Event Timeline

A loop change is also proposed to increase efficiency.

May I suggest you to split this into two patches (when committing at least). We shouldn't mix a bug-fix with a performance improvement unless the performance improvement is needed in a direct relation with the bugfix.

Hi @Germano Cavalcante (mano-wii) there are a few warnings with the patch:

//source/blender/editors/transform/transform.c: In function ‘calcEdgeSlide_mval_range’:
//source/blender/editors/transform/transform.c:6925:11: warning: unused variable ‘e’ [-Wunused-variable]
   BMEdge *e;
           ^
//source/blender/editors/transform/transform.c:6924:10: warning: unused variable ‘iter’ [-Wunused-variable]
   BMIter iter;
          ^~~~
//source/blender/editors/transform/transform.c:6912:10: warning: unused variable ‘bm’ [-Wunused-variable]
   BMesh *bm = em->bm;
          ^~
//source/blender/editors/transform/transform.c:6904:49: warning: unused parameter ‘sv_table’ [-Wunused-parameter]
                                      const int *sv_table,
                                                 ^~~~~~~~

Thanks for taking a look.
A bit annoying to find these warnings in the middle of the other 180 msvc19's warnings (but you can hide by group so it's not a justification).

I can separate the optimization into another commit.
(Without it the code runs at least twice per vertex and edge calling even three raycast per edge!)

I can separate the optimization into another commit.

This helps the review as well, since the patch becomes smaller.

Also note that your patch will likely need a manual merge conflict fix because of: 5ca302cb0cb40506e1f8a5f22e9baa3738ab4a58