Page MenuHome

Make the manipulator and transform visual center care for the deformed cage
Needs RevisionPublic

Authored by Grigory Revzin (revzin) on Jun 15 2014, 6:49 PM.

Details

Summary

A 'fix' for T40505, now manipulator and transform constraints and propcircle are drawn using data from the derived cage

In master:


In here:

Some issue is given by modifiers like mirror, where same CD_ORIGINDEX is set for two vertices - there's no way to know the exact original, but still much better than nothing!

Diff Detail

Event Timeline

Bastien Montagne (mont29) requested changes to this revision.Jun 15 2014, 8:36 PM

Very quick test, patch does not builds of hands on linux/gcc (see comments, MSVC tends to be much more laxist on those points by default, iirc). Also note that I get several reports about unused variables (but we can clean up this later).

Else, feature seems to work OK, nice fix imho!

Will have a closer look at code itself tomorrow.

source/blender/blenkernel/BKE_crazyspace.h
62–64

You have to declare those two structs in start of the file too (like Mesh etc.), this breaks gcc too.

source/blender/editors/transform/transform_manipulator.c
264

Should be static (triggers warning under linux/gcc).

310–314

Declarations of vars after code - errors in gcc too (C90 forbids this ;) ).

source/blender/blenkernel/intern/crazyspace.c
484–485

Sorry, forgot that 'declaration after code' case.

Grigory Revzin (revzin) updated this revision to Unknown Object (????).Jun 15 2014, 9:28 PM

Adressed issues that mont29 pointed out. Need to build the soc-2014-shapekey with gcc next week!

Bastien Montagne (mont29) requested changes to this revision.Jun 20 2014, 9:36 AM

All in one, patch looks good to me. We really need Campbell's advice here though, I’m only half-familiar with this area of code…

source/blender/blenkernel/BKE_crazyspace.h
37

Picky: better to try to avoid such meaning-less changes in patches. ;)

54

indices, not indeces.

source/blender/blenkernel/intern/crazyspace.c
455–464

Might be a matter of taste, but would rather deduplicate it this way (not so much important here, but below with longer code saves quite a bit of lines):

if (ese->htype == BM_VERT) {
    BMVert *v = (BMVert *)ese->ele;
    const int idx = derived_index_map ? derived_index_map[BM_elem_index_get(v)] : BM_elem_index_get(v);
    copy_v3_v3(cent, dm_verts[idx].co);
}

The same goes for similar cases below.

source/blender/editors/transform/transform_generics.c
1625–1649

Here again, you can deduplicate with a mere:

const int idx = vertexmap ?  vertexmap[BM_elem_index_get(emv)] : BM_elem_index_get(emv);
source/blender/editors/transform/transform_manipulator.c
266–271

Again, this can be done in one line:

const int derived_index = index_map ? index_map[edit_vert_index] : edit_vert_index;

As a side note, might be good to have a consistent naming for that index_map stuff. ;)

source/blender/editors/transform/transform_generics.c
1625–1649

I'd rather not deduplicate here - that way, we'll be checking that vertexmap exists on every loop step, and this code should be as fast as possible. Do you think it's not a big deal?

Grigory Revzin (revzin) updated this revision to Unknown Object (????).Jun 21 2014, 8:59 PM

Mainly deduplication using the '? :' operator, more consistent naming, got rid of gcc warnings

This revision now requires changes to proceed.Aug 11 2016, 6:08 AM