[Bf-committers] Temporary storage for Edit{Vert|Edge|Face}

[Chris Want]

Hi folks,

You might recall that earlier this week I got
into trouble while trying to abuse short flags
f1, f2 of the EditEdge structure to hold a long
... this was a bad thing to do because it will
fail on 64 bit platforms.

I was told by Ton that I should use the field
'vn' ('vertex new') to hold my long -- this is
a pointer to an EditVert and so has sufficient
storage to hold the long.

Looking through the code, there are many instances
of this vn field of EditEdge and EditVert being
used to hold all kinds of things:

* pointers to EditVerts, not necessarily new ones
* longs
* pointers to EditFaces
* pointers to floats
* pointers to EVPtr (whatever that is)

And these uses involve ugly looking casts, and are
usually accompanied with a comment that states that
this use is an abuse of the pointer (which it is).

So I was grumbling about this on irc last night
when brecht had a great idea: why not use a union
to hold this temporary storage? I haven't used
unions much, but this seems like the kind of
application that a union was designed for.

Here's my proposal: replace vn with a union called
tmp:

         union {
                 /* some lean storage for temporary usage
                  * in editmesh routines
                  */
                 struct EditVert *v;
                 struct EditEdge *e;
                 struct EditFace *f;
                 float           *fp;
                 void            *p;
                 long             l;
         } tmp;

This union uses *no more storage* than what is currently
being used now. How does it work? Well, if you have
an EditVert eve and you need a long for temporary uage,
you would use eve->tmp.l ('temporary long') to store the
long, rather than doing a cast like (long) eve->vn.
If you need a pointer to an EditFace, you use eve->tmp.f
('temporary face') You don't need to cast anything, you
don't need to add an apologetic comment, and in both of
these cases the same storage is being used for each task.
A real life example in the code would look like this:

BEFORE:
/* the vertices, abuse ->vn as counter */
...
eve->vn= (EditVert *)(long)(a++);  /* counter */

AFTER:
/* the vertices, use ->tmp.l as counter */
...
eve->tmp.l = a++;  /* counter */

I think that the AFTER code is much nicer, and the
name we store it in gives a better clue about what's
being held ('temporary long' instead of 'vertex new').

Anyways, this email is getting kind of long. I've put
a patch in the patch tracker that implements these
kinds of temporary storage unions for EditVerts,
EditEdges, and EditFaces. I'd like to commit this,
but I think it would be best to have some discussion
first. The patch is here:

https://projects.blender.org/tracker/?func=detail&aid=3695&group_id=9&atid=127

Regards,
Chris