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

[Ed Halley]

I think the big reason that stuff like Unions (and other things like 
symbolic array size constants) is not used in many structures right now 
is that the SDNA scanners can't read them.  That is, every member of 
every structure which has a hope of being saved to disk in a .blend must 
be given a stable name, must be a type that SDNA knows about, must be 
aligned in all kinds of kookie ways, and the moon must be gibbous 
waxing.  Nested unions are a nice idea, but the makesnda/ code has to be 
made smarter before you can just pile multiple variables at the same 
struct offset.

Chris Want wrote:
> 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
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at projects.blender.org
> http://projects.blender.org/mailman/listinfo/bf-committers
> 
> 
> 

-- 
[ e d @ h a l l e y . c c ]