[Bf-python] bpython fragility..

Yann Vernier yann at algonet.se
Sun Jan 2 06:07:33 CET 2005


When trying to implement skeletons in the NWN model importer, I got
frustrated by more bugs in the python interface. Attached is a patch
which fixes a few trouble spots, points out a few others, but does not
fix the nastiest ones I've hit yet..

Changes in Armature: There was a broken function to test for a bone name
existing in an armature. Replaced with the non-broken function that did
the same job.
Changes in Bone: Made the object type check actually check type, rather
than blindly assuming any python object is a bone.
Changes in Texture: Return subtype correctly for envmaps. This was
requested by someone else but hasn't made it into CVS, so I thought I'd
just resubmit it here.

The nasty bug I still haven't tracked down occurs if you parent bones
after adding them into an armature. Doing so leaves duplicate
references.. I put a FIXME in Bone.c where I think the problem should be
adjusted, but I don't think there's a reference to the armature around
to allow the job to be done. 

I think the current Bone wrapper objects don't make much sense. We're
keeping a virtual python structure for a bone until it's added to an
armature, and then we're wrapping the armature bone. Parenting is broken
for the virtual bones (bone.children is None) and for true bones it's
weird (it appears possible to move bones between armatures with this,
but not using armature.addBone). I'd suggest having bones *always* be
associated with a real bone structure, and freeing it along with the
python object if it was never added into an armature. The python bone
wrapper could contain only the pointers to the bone and its armature to
cover this.

-- 
PGP fingerprint = 9242 DC15 2502 FEAB E15F  84C6 D538 EC09 5380 5746
-------------- next part --------------
? Armature.cxx
? Blender.pyx
? Makefile.in
? Material.c.diff
? NMesh.c.partialmess
? Object.c.current
? Object.c.diff1
? Object.c.diff2
? Object.c.diff3
? Text.pxd
? Text.pyx
? Texture.diff
? backup
? blenkernel.pxd
? blenlib.pxd
? changes.diff
? test.c
? test.h
? test.pxi
Index: Armature.c
===================================================================
RCS file: /cvsroot/bf-blender/blender/source/blender/python/api2_2x/Armature.c,v
retrieving revision 1.18
diff -u -r1.18 Armature.c
--- Armature.c	7 Oct 2004 19:25:39 -0000	1.18
+++ Armature.c	2 Jan 2005 04:59:04 -0000
@@ -102,7 +102,7 @@
 			     PyObject * v );
 static int Armature_compare( BPy_Armature * a1, BPy_Armature * a2 );
 static PyObject *Armature_repr( BPy_Armature * armature );
-static int doesBoneName_exist( char *name, bArmature * arm );
+static Bone *testBoneNameInArmature( bArmature * arm, char *name );
 
 //---------------- Python TypeArmature structure definition:-----------
 PyTypeObject Armature_Type = {
@@ -151,7 +151,7 @@
 
 //------------------append_childrenToList-----------------------------------
 static void append_childrenToList( Bone * parent, PyObject * listbones )
-{
+{		/* Recursive; finds all bones that are children of given bone */
 	Bone *child = NULL;
 
 	//append children 
@@ -165,12 +165,12 @@
 
 //------------------unique_BoneName----------------------------
 static void unique_BoneName( char *name, bArmature * arm )
-{
+{		/* Ensures a name is unique by adding .number */
 	char tempname[64];
 	int number;
 	char *dot;
 
-	if( doesBoneName_exist( name, arm ) ) {
+	if( testBoneNameInArmature( arm, name ) ) {
 		/*      Strip off the suffix */
 		dot = strchr( name, '.' );
 		if( dot )
@@ -178,7 +178,7 @@
 
 		for( number = 1; number <= 999; number++ ) {
 			sprintf( tempname, "%s.%03d", name, number );
-			if( !doesBoneName_exist( tempname, arm ) ) {
+			if( !testBoneNameInArmature( arm, tempname ) ) {
 				strcpy( name, tempname );
 				return;
 			}
@@ -186,27 +186,9 @@
 	}
 }
 
-//------------------doesBoneName_exist----------------------------
-static int doesBoneName_exist( char *name, bArmature * arm )
-{
-	Bone *parent = NULL;
-	Bone *child = NULL;
-
-	for( parent = arm->bonebase.first; parent; parent = parent->next ) {
-		if( !strcmp( name, parent->name ) )
-			return 1;
-		for( child = parent->childbase.first; child;
-		     child = child->next ) {
-			if( !strcmp( name, child->name ) )
-				return 1;
-		}
-	}
-	return 0;
-}
-
 //------------------testChildInChildbase--------------------------
 static int testChildInChildbase( Bone * bone, Bone * test )
-{
+{		/* Recursive; tests if a bone is among the descendants of another */
 	Bone *child;
 	for( child = bone->childbase.first; child; child = child->next ) {
 		if( child == test ) {
@@ -224,7 +206,7 @@
 
 //------------------testBoneInArmature-----------------------------
 static int testBoneInArmature( bArmature * arm, Bone * test )
-{
+{		/* Checks if a bone is in an armature */
 	Bone *root;
 	for( root = arm->bonebase.first; root; root = root->next ) {
 		if( root == test ) {
@@ -242,7 +224,7 @@
 
 //-----------------testChildNameInChildbase--------------------------
 static Bone *testChildNameInChildbase( Bone * bone, char *name )
-{
+{		/* Recursive; test if a bone name is among the descendants of a bone */
 	Bone *child;
 	Bone *test;
 	for( child = bone->childbase.first; child; child = child->next ) {
@@ -261,7 +243,7 @@
 
 //----------------testBoneNameInArmature----------------------------
 static Bone *testBoneNameInArmature( bArmature * arm, char *name )
-{
+{		/* Full recursive check of a bone name existing in an armature */
 	Bone *bone;
 	Bone *test;
 	for( bone = arm->bonebase.first; bone; bone = bone->next ) {
@@ -576,6 +558,8 @@
 	if( !updateBoneData( py_bone, parent ) )
 		return EXPP_ReturnPyObjError( PyExc_AttributeError,
 					      "bone struct empty" );
+
+	assert(parent == py_bone->bone->parent);
 
 	//make sure the name is unique for this armature
 	unique_BoneName( py_bone->bone->name, self->armature );
Index: Bone.c
===================================================================
RCS file: /cvsroot/bf-blender/blender/source/blender/python/api2_2x/Bone.c,v
retrieving revision 1.22
diff -u -r1.22 Bone.c
--- Bone.c	7 Oct 2004 19:25:39 -0000	1.22
+++ Bone.c	2 Jan 2005 04:59:04 -0000
@@ -232,6 +232,7 @@
 //--------------- Bone module internal callbacks-----------------
 
 //--------------- updatePyBone------------------------------------
+// Updates a BPy_Bone from its bone
 int updatePyBone( BPy_Bone * self )
 {
 	int x, y;
@@ -292,6 +293,7 @@
 }
 
 //--------------- updateBoneData------------------------------------
+// Updates a bone from its BPy_Bone counterpart
 int updateBoneData( BPy_Bone * self, Bone * parent )
 {
 	//called from Armature.addBone()
@@ -342,6 +344,7 @@
 }
 
 //--------------- testChildbase----------------------------------
+// Recursively checks if bone is an ancestor of test
 static int testChildbase( Bone * bone, Bone * test )
 {
 	Bone *child;
@@ -949,7 +952,7 @@
 	int i;
 
 	if( !self->bone ) {	//test to see if linked to armature
-		//use python vars
+		//use python vars	FIXME - bones may already be parented before adding to armatures!
 		return EXPP_incr_ret( Py_None );
 	} else {
 		//use bone datastruct
@@ -980,6 +983,9 @@
 			 ( PyExc_AttributeError,
 			   "expected string argument" ) );
 
+	/* FIXME recipe for disaster. did anyone consider that the
+	 * size for strncpy should be for the DESTINATION buffer?!??
+	 * Besides which strlen is inefficient for Python strings */
 	if( !self->bone ) {	//test to see if linked to armature
 		//use python vars
 		BLI_strncpy( self->name, name, strlen( name ) + 1 );
@@ -1177,13 +1183,13 @@
 	float M_boneObjectspace[4][4];
 	float iM_parentRest[4][4];
 
-	if( !PyArg_ParseTuple( args, "O", &py_bone ) )
+	if( !PyArg_ParseTuple( args, "O!", &Bone_Type, &py_bone ) )
 		return ( EXPP_ReturnPyObjError
 			 ( PyExc_TypeError,
 			   "expected bone object argument" ) );
 
 	if( !self->bone ) {	//test to see if linked to armature
-		//use python vars
+		//use python vars	FIXME buffer overflow.
 		BLI_strncpy( self->parent, py_bone->name,
 			     strlen( py_bone->name ) + 1 );
 	} else {
@@ -1233,6 +1239,8 @@
 			self->bone->parent = py_bone->bone;
 
 		} else {	//not previously parented
+
+			//FIXME remove root bone from armature!
 
 			//add to the childbase of new parent
 			BLI_addtail( &py_bone->bone->childbase, self->bone );
Index: Texture.c
===================================================================
RCS file: /cvsroot/bf-blender/blender/source/blender/python/api2_2x/Texture.c,v
retrieving revision 1.7
diff -u -r1.7 Texture.c
--- Texture.c	7 Oct 2004 19:25:40 -0000	1.7
+++ Texture.c	2 Jan 2005 04:59:05 -0000
@@ -915,9 +915,14 @@
 {
 	PyObject *attr = NULL;
 	const char *stype = NULL;
+	int n_stype;
 
+	if( self->texture->type == EXPP_TEX_TYPE_ENVMAP )
+		n_stype = self->texture->env->stype;
+	else
+		n_stype = self->texture->stype;
 	if( EXPP_map_getStrVal( tex_stype_map[self->texture->type],
-				self->texture->stype, &stype ) )
+				n_stype, &stype ) )
 		attr = PyString_FromString( stype );
 
 	if( !attr )
@@ -1449,6 +1454,9 @@
 		   EXPP_map_getShortVal( tex_stype_map
 					 [EXPP_TEX_TYPE_DISTNOISE], stype,
 					 &self->texture->noisebasis ) ) );
+	else if( ( self->texture->type == EXPP_TEX_TYPE_ENVMAP &&
+	      EXPP_map_getShortVal( tex_stype_map[self->texture->type],
+				    stype, &self->texture->env->stype ) ) );
 	else if( !EXPP_map_getShortVal
 		 ( tex_stype_map[self->texture->type], stype,
 		   &self->texture->stype ) )
@@ -1475,7 +1483,10 @@
 		return EXPP_ReturnPyObjError( PyExc_ValueError,
 					      "invalid stype (for this type)" );
 
-	self->texture->stype = stype;
+	if( self->texture->type == EXPP_TEX_TYPE_ENVMAP )
+		self->texture->env->stype = stype;
+	else
+		self->texture->stype = stype;
 
 	Py_INCREF( Py_None );
 	return Py_None;
@@ -1780,7 +1791,10 @@
 		attr = Py_BuildValue( "(f,f,f)", tex->rfac, tex->gfac,
 				      tex->gfac );
 	else if( STREQ( name, "stype" ) )
-		attr = PyInt_FromLong( tex->stype );
+		if( self->texture->type == EXPP_TEX_TYPE_ENVMAP )
+			attr = PyInt_FromLong( tex->env->stype );
+		else
+			attr = PyInt_FromLong( tex->stype );
 	else if( STREQ( name, "turbulence" ) )
 		attr = PyFloat_FromDouble( tex->turbul );
 	else if( STREQ( name, "type" ) )
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.blender.org/pipermail/bf-python/attachments/20050102/9a9fed74/attachment.sig>


More information about the Bf-python mailing list