Login
Username:

Password:

Remember me



Lost Password?

Register now!
Main Menu
H3D.org Feeds
Forum feeds News feeds
H3D.org Forum Index
   Programming Issues
     Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?		Register To Post

Threaded | Newest First Previous Topic | Next Topic | Bottom
Poster Thread
karlsvec
Posted on: 2012/7/25 1:06
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Hello,

I'm seeing what may be incorrect behavior of EXTERNPROTO statements in H3D trunk.

Example:

My external prototype that I'll use via EXTERNPROTO:

test_ProtoValue.x3d
<X3D>
    <Scene>
        <ProtoDeclare name='SphereProto'>
            <ProtoInterface>
                <field name='radius' type='SFFloat' accessType='inputOutput' value='0.5'/>
            </ProtoInterface>
            <ProtoBody>
                <Shape>
                    <Appearance>
                        <Material diffuseColor='1 0 0'/>
                    </Appearance>
                    <Sphere>
                        <IS>
                            <connect nodeField='radius' protoField='radius'/>
                        </IS>
                    </Sphere>
                </Shape>
            </ProtoBody>
        </ProtoDeclare>
    </Scene>
</X3D>


And the scene that creates the ProtoInstance:

test_ProtoValueExtern.x3d
<X3D>
    <Scene>
        <ExternProtoDeclare name='SphereProto' url='test_ProtoValue.x3d#SphereProto'>
            <field name='radius' type='SFFloat' accessType='inputOutput'/>
        </ExternProtoDeclare>
        <Group>
            <ProtoInstance name='SphereProto'/>
        </Group>
    </Scene>
</X3D>


When I load this scene in H3DLoad, I expect to see a red Sphere with radius 0.5, but instead I see nothing (or perhaps a Sphere with zero radius). If I add "value='0.5'" to the ExternProtoDeclare part, I *do* see the Sphere.

However, this is not how I thought prototypes worked, and seems to disagree with the X3D specification (emphasis mine):

Quote:

4.4.5.2 EXTERNPROTO interface semantics

The semantics of the EXTERNPROTO are exactly the same as for a PROTO statement, except that *default field values are not specified locally*. In addition, events sent to an instance of an externally prototyped node may be ignored until the implementation of the node is found.

Until the definition has been loaded, the browser shall determine the initial value of inputOutput fields using the following rules (in order of precedence):

the user-defined value in the instantiation (if one is specified);
the default value for that field type.
For outputOnly fields, the initial value on startup will be the default value for that field type. During the loading of an EXTERNPROTO, if an initial value of an outputOnly field is found, that value is applied to the field and no event is generated.

The names and types of the fields of the interface declaration shall be a subset of those defined in the implementation. Declaring a field with a non-matching name is an error, as is declaring a field with a matching name but a different type.

It is recommended that user-defined field names defined in EXTERNPROTO interface statements follow the naming conventions described in 4.4.2.2 Field semantics.


So, it seems that H3D is not looking to the actual prototype declaration to set default field values.

I plan to look into this further and submit a bug report on the Mantis tracker.


X3D files:
http://vrac.iastate.edu/~karlsvec/files/test_ProtoValue.x3d
http://vrac.iastate.edu/~karlsvec/files/test_ProtoValueExtern.x3d

- Karl
karlsvec
Posted on: 2012/7/28 0:37
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Ok, I've been working on a patch to fix the ExternProtoDeclare problem as well as the nested Prototype instantiation problem (see http://www.h3dapi.org/modules/newbb/v ... ost_id=8067#forumpost8067).

Here's what I have so far:
Index: include/H3D/ProtoDeclaration.h
===================================================================
--- include/H3D/ProtoDeclaration.h    (revision 1943)
+++ include/H3D/ProtoDeclaration.h    (working copy)
@@ -82,8 +82,12 @@
       body( _body ),
       body_extra( _body_extra ) {}
 
+    /// Copy constructor
+    /// param _other the other ProtoDeclaration object to copy
+    ProtoDeclaration( const ProtoDeclaration& _other );
+
     /// Get the string with the internal scenegraph of the prototype.
-    const string &getProtoBody() {
+    const string &getProtoBody() const {
       return body;
     }
 
@@ -99,12 +103,12 @@
     }
     
     /// Get the proto body extras.
-    const vector<string> & getProtoBodyExtra( ) {
+    const vector<string> & getProtoBodyExtra( ) const {
       return body_extra;
     }
 
     /// Get the name of the prototype.
-    const string& getName() {
+    const string& getName() const {
       return name;
     }
 
@@ -132,6 +136,16 @@
       }
       return NULL;
     }
+
+    /// Get the field declarations for this ProtoDeclaration
+    /// Returns a reference to the field_declarations member variable
+    const std::list< FieldDeclaration >& getFieldDeclarations() const {
+      return field_declarations;
+    }
+
+    void addExternProtoDeclaration( const ProtoDeclaration& extern_proto_dec ) {
+      extern_proto_declarations.push_back( extern_proto_dec );
+    }
     
     /// Create a new X3DPrototypeInstance instance using the ProtoDeclaration. 
     X3DPrototypeInstance *newProtoInstance();
@@ -144,6 +158,7 @@
     // as the main body but defined after it.
     vector< string > body_extra; 
     std::list< FieldDeclaration > field_declarations;
+    std::list< ProtoDeclaration > extern_proto_declarations;
 
     AutoRef< Node > createProtoInstanceNodeX3D( PrototypeInstance *proto,
                                                 X3D::DEFNodes *dn,
Index: src/ProtoDeclaration.cpp
===================================================================
--- src/ProtoDeclaration.cpp    (revision 1943)
+++ src/ProtoDeclaration.cpp    (working copy)
@@ -42,6 +42,17 @@
 
 using namespace H3D;
 
+ProtoDeclaration::ProtoDeclaration( const ProtoDeclaration& _other ) {
+  name = _other.getName();
+  body = _other.getProtoBody();
+  body_extra = _other.getProtoBodyExtra();
+
+  for( std::list< FieldDeclaration >::const_iterator i = _other.getFieldDeclarations().begin();
+    i != _other.getFieldDeclarations().end(); i++ ) {
+    addFieldDeclaration( ( *i ).name, ( *i ).type, ( *i ).access_type, ( *i ).value );
+  }
+}
+
 X3DPrototypeInstance *ProtoDeclaration::newProtoInstance() { 
   PrototypeInstance *proto = new PrototypeInstance( NULL );
   try {
@@ -164,7 +175,12 @@
                                                               const string &body ) {
 #ifdef HAVE_XERCES
   auto_ptr< SAX2XMLReader > parser( X3D::getNewXMLParser() );
-  X3D::X3DSAX2Handlers handler(dn);
+  PrototypeVector proto_vector;
+  for( std::list< ProtoDeclaration >::iterator i = extern_proto_declarations.begin();
+    i != extern_proto_declarations.end(); i++ ) {
+    proto_vector.push_back( new ProtoDeclaration( *i ) );
+  }
+  X3D::X3DSAX2Handlers handler( dn, NULL, &proto_vector );
   handler.proto_instance = proto;
   stringstream s;
   s << body;
Index: src/X3DSAX2Handlers.cpp
===================================================================
--- src/X3DSAX2Handlers.cpp    (revision 1943)
+++ src/X3DSAX2Handlers.cpp    (working copy)
@@ -883,7 +883,15 @@
     }
   } else if( localname_string == "ExternProtoDeclare" ) {
     if( proto_declaration ) {
-      proto_declarations->push_back( proto_declaration );
+      // TODO: check that the field names, types, and
+      // access types in the EXTERNPROTO match the actual
+      // PROTO
+      //
+      // If any sort of mismatch is found, should
+      // probably throw an XMLParseError exception
+
+      //proto_declarations->push_back( proto_declaration );
+      delete proto_declaration;
       proto_declaration = NULL;
     }
     defining_extern_proto = false;
@@ -932,8 +940,14 @@
     string s_name = toString( name );
     ProtoDeclaration *proto = 
       proto_declarations->getProtoDeclaration( s_name );
-    if( proto ) return proto->newProtoInstance();
-    else {
+    if( proto ) {
+      for( PrototypeVector::iterator i = proto_declarations->begin(); i != proto_declarations->end(); i++ ) {
+        if( *i != proto ) {
+          proto->addExternProtoDeclaration( *( *i ));
+        }
+      }
+      return proto->newProtoInstance();
+    } else {
       Console(3) << "Warning: Could not find PROTO declaration for "" << s_name  
            << "" instance (" << toString( locator->getSystemId() )
            << ", line " << locator->getLineNumber() << ")" << endl;
@@ -997,6 +1011,11 @@
 
         try {
           X3D::createX3DNodeFromURL( base_url, NULL, NULL, &proto_vector );
+
+          // Deep-copy the contents of proto_vector into proto_declarations
+          for( PrototypeVector::iterator i = proto_vector.begin(); i != proto_vector.end(); i++ ) {
+            proto_declarations->push_back( new ProtoDeclaration( *( *i ) ));
+          }
         } catch( const Exception::H3DException & ) {
           continue;
         }


This patch does mainly two things:

1) prevent prototype declarations that are gathered from parsing ExternProtoDeclares from being discarded in X3DSAX2Handlers::handleExternProtoDeclareElement() when the proto_vector object (created on the stack) goes out of scope. The proto declarations are deep-copied into the proto_declarations member.
2) Make external prototype declarations available for use in ProtoDeclaration::newProtoInstance(), in the event that the ProtoBody has other ProtoInstances pulled in via ExternProtoDeclare.

At the moment, the patch makes it so that H3D pretty much ignores any field elements within an ExternProtoDeclare. What should probably happen is that these fields are checked against the actual ProtoDeclare to make sure they match up.

Right now the patch feels like a bit of a hack, since it involves lots of copying of ProtoDeclaration objects and a modification to the design of the ProtoDeclaration class. I'd really appreciate some feedback from the SenseGraphics devs on this. Perhaps someone can propose a more elegant solution? (although I must admit, to my eyes X3DSAX2Handlers is a bit of a mess).

UPDATE: I've mulled this over a bit more, and it might make sense to introduce a new ProtoInstanceBuilder class into H3D. All the ProtoInstance creation code could be moved from ProtoDeclaration into this new class. The ProtoInstanceBuilder could have a single static method to construct the new ProtoInstance node, given the ProtoDeclaration in question and any other ProtoDeclarations found while parsing the X3D:

static X3DPrototypeInstance* ProtoInstanceBuilder::newInstance( ProtoDeclaration* proto, const PrototypeVector& other_proto_declarations );


This at least would prevent having to clutter the ProtoDeclaration class with code for storing other ProtoDeclarations. Food for thought.

Many thanks,

- Karl
karlsvec
Posted on: 2012/8/15 20:21
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Bumping this thread. Hopefully it will draw the attention of the SenseGraphics devs.
Markus
Posted on: 2012/8/20 12:28
Webmaster
Joined: 2006/3/27
From: SenseGraphics
Posts: 1920
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Hello,

Did you try an x3d-only example with some x3d-compliant browser to see if you get the result you expect? H3DViewer will always be a bit behind if they change something in their specification.

As for giving you a good answer on how to solve/restructure it will sadly have to wait. I just got back from vacation and have other issues to check atm. The biggest problem for me would be to check the proto-x3d-specification since I was not the one that implemented in the first place and I have barely or never used it, so you probably know more about it at the moment.
karlsvec
Posted on: 2012/8/22 0:03
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
I will try loading these scenes with the Xj3D browser and report back.
karlsvec
Posted on: 2012/8/23 0:38
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
I loaded the test scene with the latest Xj3D 2.0 pre-release snapshot, and it works the way I expect it to (as I described in the first post). ProtoInstance field values are specified in the actual PROTO declaration, and any 'value' elements in the EXTERNPROTO declaration are ignored.

So, this is definitely a bug in the way H3D handles PROTO/EXTERNPROTO.

EDIT: there was one slight hiccup. Xj3D seems to insist that the radius field on a Sphere node is initializeOnly, so I had to make some adjustments to the example scene (probably a bug in Xj3D).

Many thanks,

- Karl
Markus
Posted on: 2012/8/23 8:40
Webmaster
Joined: 2006/3/27
From: SenseGraphics
Posts: 1920
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Ok, then there is probably quite a big bug, did you make a bug report? I know it might seem like we ignore them but we really do not, it just takes a bit of time to find time to test them. Especially if they are a bit more complicated and need more thought as well as X3D specification interpretation.
karlsvec
Posted on: 2012/8/23 22:38
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
The bug report is here (ID 0000296). I included a link to this thread in the bug report, as well as my hacked-together example patch.
Markus
Posted on: 2012/8/24 8:10
Webmaster
Joined: 2006/3/27
From: SenseGraphics
Posts: 1920
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
Thank you, we appreciate your work.
I hope that you find a way to get your scene setup the way you want it in a good way even if you can not rely on protos at the moment. If you are H3D bound (and it does not have to work in other X3D viewers) you can use python to "fix" issues with default values, or to define the scene there instead.
karlsvec
Posted on: 2012/8/24 19:23
Quite a regular
Joined: 2008/7/1
From:
Posts: 42
Re: Incorrect behavior of ExternProtoDeclare in H3D trunk (r1943)?
For now I am using my temporary patch. It allows me to do what I need to with prototypes until a permanent solution arrives in H3D.

Thanks,

- Karl
Threaded | Newest First Previous Topic | Next Topic | Top

Register To Post
 



(C) 2012 SenseGraphics AB    ---    Powered by XOOPS