danomatika / swig-openframeworks

a SWIG interface for openFrameworks with included Makefile, submodule this in your language wrapper addons
Other
39 stars 11 forks source link

swig 3.0.2 seems to throw errors on ofNode.h #7

Closed joshuajnoble closed 9 years ago

joshuajnoble commented 9 years ago

I'm getting an error on the following:

../../libs/openFrameworks/3d/ofNode.h:205: Error: Syntax error in input(3).
make: *** [bindings] Error 1

Using swig 3.0.2 to try to get around the import error in 3.0.5. Is this a mis-configuration on my part somehow?

danomatika commented 9 years ago

What version of OF are you using and what's on that particular line? We're only testing against the current release version.

danomatika commented 9 years ago

Also, what language bindings are you generating? Pre-generated bindings are already available in ofxPython and ofxLua ...

joshuajnoble commented 9 years ago

Python for OF 0.9 and I didn't know that, will investigate as soon as get off this bus.

On Jun 12, 2015, at 16:08, Dan Wilcox notifications@github.com wrote:

Also, what language bindings are you generating? Pre-generated bindings are already available in ofxPython and ofxLua ...

— Reply to this email directly or view it on GitHub.

joshuajnoble commented 9 years ago

Yeah, I'm looking at generating new bindings for 0.9 more out of curiosity than anything else, this isn't a bug but rather a feature request/investigation. ofxPython currently only supports 0.8.4, which is great but I was curious about updating myself & trying to learn a bit more.

danomatika commented 9 years ago

Looking at that line, I bet it's probably due to SWIG not understanding the default assignment calling a function. Nothing like overly fancy C++ isms. This means we either file a bug report and change the source file or wrk around it by defining the class manually in the interface file and ignore/redefine the problem function. I had to do this with ofTrueTypeFont for now since the static font string define declaration was causing a SWIG error. I did a bug fix which should be in OF 0.9 by name.

Basically, I spent a lot of time searching for SWIG errors as we built the interface file. SWIG is pretty smart at figuring things out by default, but it's not very good with edge cases as things like nested class structs or unions (hence some tricky stuff to make ofColor work).

joshuajnoble commented 9 years ago

Was just about to drop the line in my last comment :) I'd love to help get this up to 0.9 to a) have it ready for 0.9 and b) to get my head around SWIG better. Trying to read through the SWIG docs to see if there's anything helpful in there but not finding much.

danomatika commented 9 years ago

Nothing helpful in understanding SWIG or the error? The docs are quite good: http://www.swig.org/Doc3.0/SWIG.html#SWIG

As for the error, the simple answer is that it's not SWIGs fault. We need to either change the source code or route around it using a manual declaration aka don't use %include "ofxNode.h" which tells SWIG to parse the file and auto generate bindings for things it finds there.

joshuajnoble commented 9 years ago

No they're quite good but I wasn't finding anyting helpful in understanding workarounds for the error, though it sounds as though manual declarations are what I was looking for. Changing ofNode itself seems like a non-starter.

danomatika commented 9 years ago

Changing ofNode itself seems like a non-starter.

I totally disagree. The fix for that error would be to assign the default value to NULL, then perform that assignment in the function implementation. I'm pretty sure it is ofGetCurrentRenderer().get() that SWIG has a problem with, since it is not a static value or something passed by reference, but a function call which SWIG cannot easily perform inside the wrapper it generates.

joshuajnoble commented 9 years ago

I'll confess to not understanding why the assignment is done in the declaration but being inclined to believe that there was a reason for doing so.

danomatika commented 9 years ago

There was, it's faster to type that then to do it an admittedly simpler but more robust way. I'm inclined to change it since it will work exactly the same way while still being understandable ... something I feel the OF core has lost a long time ago :(

joshuajnoble commented 9 years ago

And voila it works. Typing at the expense of readability isn't OF-y imho. Unless you want to do it, I'll put in that PR right now.

danomatika commented 9 years ago

Cool. This is what I was thinking:

header:

void transformGL(ofBaseRenderer * renderer = NULL) const;

cpp:

void ofNode::transformGL(ofBaseRenderer * renderer) const {
    if(renderer == NULL) {
        renderer = ofGetCurrentRenderer().get();
    }
    renderer->pushMatrix();
    renderer->multMatrix( getGlobalTransformMatrix() );
}

I suppose it adds an NULL check which hopefully won't bother performance much.

danomatika commented 9 years ago

Note: These kinds of issues haven't popped up very often so far. I only had to submit bug fixes maybe 2-3 times for edge cases like this, i.e. https://github.com/danomatika/openFrameworks/commit/e50e52614642e1568f8f207b56b8d5c8e0eb54d1.