Am 11.09.2016 um 22:06 schrieb LanuHum:

> https://github.com/Lanuhum/Blender-C-

Some more comments:


All the PyObject* items in py2c.cpp:

First of all, you're using global variables, which you shouldn't use in
C++ programs. You should wrap all the stuff in py2c.cpp into a class.

Then, every time you set them to some value returned by PyWhatThe_Foo(),
you're receiving a pointer to data residing... well, /somewhere/. Nobody
knows. And, more importantly, given that it is /probably/ dynamically
allocated, nobody knows who is responsible for /releasing/ the memory.
And /how/ to release it. The <Python.h> functions may be striaghtforward
C code, in which case you need to release the stuff using `free()`. Or
it may be C++ code, in which case you need to release it using
`delete[]` (most certainly not `delete`; subtle but important difference
there). Or <Python.h> may provide a dedicated function to dispose of it.
Obviously you're not really sure or aware of this issue either, because
you never invoke any of these mechanisms.

There is some code at the end of parse_blend() that /looks/ like it was
intended to do the trick, but you commented it out. Worse yet, it
wouldn't even be sufficient.

*Every* time you assign something to one of these variables (and also
when the program exits), the previous content gets lost, and the memory
it pointed to remains in limbo: Your memory holes are legion.

What you (presumably) need to to is call PyDECREF() *every* time before
you *overwrite* one of those PyObject* variables.


You may want to have a look at boost.python, which seems to encapsulate
the Python API a bit better for use in C++, most notably allowing the
use of smart pointers rather than classic C pointers. Smart pointers
automatically release the data they point to when you assign a new value
to the smart pointer, or even when the variable goes out of scope.
(They're even smart enough to /not/ release data if another copy of the
smart pointer still refers to the data.)


PyName_AsName():

Same problem as above, but even worse: You need to pass the data out of
the function, so you can't just call PyDECRREF() on the data (if that's
even the proper way of releasing char* data returned by the Python API).
I would recopmmend constructing a std::string from it (creating a copy
of its contents), and call the adequate memory release function
immediately. You can then return the std::string, which will save the
caller from the task of pointer handling.


parse_verts():

You're using a vector<double> to store three coordinates. This is
inefficient: vector<> is designed to store dynamic arrays, /not/ what
mathematicians would call a vector. Instead, simply use:

    double coord[3];

You can leave the remainder of the function entirely unchanged.


parse_faces():

Same deal as above for the `face` variable.


ImageWindow:

Here you're solving the global variables the right way: By making it a
class, and working with an instance of it. /But/ you're falling short:
Your constructor constructs an assload of data using `new`, but the
destructor only releases three of those fields using `delete`. Also, for
some of the stuff you're creating I wonder whether you're ever properly
hooking it up to other stuff. Maybe it's a quirk of <fx.h>, but to me
every "new" without an assignment to its left-hand side (or something
else that stores its result value) reeks of a serious coding error.


ImageWindow::onCmdRender():

Beware that clock_gettime is /not/ portable.


Well, I guess that should suffice for starters. I shall warn you though:
I suspect you'll still have a looooooong way ahead of you, and I won't
be able to provide this type of support on a regular basis.

I don't mean to discourage you, but seriously: Do you /really/ feel up
to this challenge you have given yourself? This is supposed to evolve
into an interface between Blender and POV-Ray's render engine, right?

Post a reply to this message