retaining and releasing correctly

[sampotter]

(See also https://github.com/sampotter/python-embree/issues/7.)

To manage the lifetimes of objects, each Embree "type" provides a pair of retain and release functions, e.g. rtcRetainDevice and rtcReleaseDevice. See the Embree API for more details, e.g.:

The API is designed in an object-oriented manner, e.g. it contains device objects (RTCDevice type), scene objects (RTCScene type), geometry objects (RTCGeometry type), buffer objects (RTCBuffer type), and BVH objects (RTCBVH type). All objects are reference counted, and handles can be released by calling the appropriate release function (e.g. rtcReleaseDevice) or retained by incrementing the reference count (e.g. rtcRetainDevice). In general, API calls that access the same object are not thread-safe, unless specified differently. However, attaching geometries to the same scene and performing ray queries in a scene is thread-safe.

One goal of this library is to make it so that the user doesn't need to think about this. I had previously tried to strategically insert retain and release calls in the constructors and destructors of the wrapped Embree types in embree.pyx, but ran into trouble getting this to work correctly, so I just removed these calls at some point (not ideal! see https://github.com/sampotter/python-embree/issues/7).

Inserting a call to retain in the constructor and release in the destructor doesn't seem to be enough. Unlike a language like C++, Cython doesn't have particularly fine-grained control of object lifetimes. Let's see if we can figure out how to do this the right way in this issue.

Another option would be to delegate this to the user, forcing them to manually retain and release themselves. This is a simple way to go, but would make the library a bit less useful, IMO.

[aluo-x]

My initial belief was that embree itself was a stateless library, with all necessary state tracked by this python layer. In retrospect the low level API has to be stateful to provide the needed performance.

My 2 cents is that the current method of delegating this to the user is a good compromise, but should be made explicit (in the README), and allows this embree3 wrapper to be a thin layer without too much extra complexity.

An additional point is that not everyone uses this library in the same way. My use case is that for any single object, I will repeatedly cast different sets of rays, with different origins/directions. This allows me to reuse any previously constructed acceleration structures, and manually free after finishing. I do this for tens of thousands of shapes.

[sampotter]

Thanks for your input. I think that makes sense. I certainly wouldn't consider myself an Embree expert. There might be certain use patterns that depend on the way that retain and release are used. Indeed, it would be good not to assume too much about them. Plus, this will make translating Embree code to Python even simpler. (Although, "ignore all calls to retain and release when translating" is simpler yet.)

One thing I worry about a little is leaking Embree objects (e.g. object instance gets destroyed before releasing), but I guess this is just as much of an issue in Embree used from C as it is in this wrapper library.

I'll go ahead and make sure that all types that I've wrapped so far have retain and release methods added to them and add a little blurb in the README about this so that people are clear what's going on before closing this issue.

[sampotter]

Fixed in 267b54033045011ae626145c4e5ff9b8bd62d276.