RenderKit / ospray

An Open, Scalable, Portable, Ray Tracing Based Rendering Engine for High-Fidelity Visualization
http://ospray.org
Apache License 2.0
985 stars 180 forks source link

API duplication between OSPObject and OSPDevice #456

Closed target-san closed 3 years ago

target-san commented 3 years ago

Hello,

ATM OSPRay has two sets of similar APIs for object management purposes

  1. ospRetain, ospRelease, ospCommit, ospSetParam, ospRemoveParam for all descendants of osp::ManagedObject
  2. ospDeviceRetain, ospDeviceRelease, ospDeviceCommit, ospDeviceSetParam, ospDeviceRemoveParam for osp::Device

These API sets have same sets of parameters, except they work on different hierarchies of objects, yet doing same things. This creates additional burden, especially when writing bindings to other languages. The most manifesting issue is writing type-safe bindings for parameter setters, which need to be duplicated or use some dynamic dispatch to abstract out ospSetParam vs ospDeviceSetParam dichotomy. Would it be possible to allow devices be managed by APIs for normal objects and make ospDevice* redirect to them?

Thanks

jeffamstutz commented 3 years ago

Unfortunately no, we cannot do this by design. The difference between (1) and (2) is that API calls in (1) go through the device, while API calls in (2) operate on the device itself. For example, each of our device implementations (cpu, mpi_offload, mpi_distributed) each separately implement ospCommit based on their implementation choices: it isn't a global for "all devices" definition. Thus for operations on the device itself, we intentionally cannot go through the device to set/commit parameters on the device (as opposed to objects created by a device). Because we do not have overloading in C99, we are forced to use unique API function names to achieve the unique code path.

target-san commented 3 years ago

Well, I though more about something like "if object is device, do for device, else do for normal object" inside function.

jeffamstutz commented 3 years ago

We can't implement that because we have no universal definition for what an internal handle representation is: each device defines the opaque handle values differently (handles are essentially just void *). Thus we would have to have a device first to be able to answer "is this handle an object or device?". Privatizing this concern to separate API calls gives devices freedom to entirely define what handles mean.

jeffamstutz commented 3 years ago

I will completely acknowledge the desire though to unify these: I spent plenty of time trying to implement the very suggestion you made when OSPRay v2.0 was being initially defined and built in 2019....based on the properties we wanted on handles and device implementations, there wasn't a pathway to unification unfortunately.

target-san commented 3 years ago

I may have missed something but, um, make Device inherit from ManagedObject, like you do with all its existing descendants? Not in 2.0 due to stability guarantees, though it should be possible in principle.

jeffamstutz commented 3 years ago

Right, the problem is that from the type system perspective all handles look the same (all are aliases to void*): so the problem arises that we don't know if it is safe to even assume that the handle is an address to a ManagedObject because the interpretation of that handle is privately defined by the device it came from (for example: some devices define object handles as segmented index information into some private object store). While the ISPCDevice is our flagship implementation we ship with OSPRay, it is not the only one hence the reason we have the device abstraction to begin with.

target-san commented 3 years ago

Ok, I get it now. Thanks for your time and have a nice day.

jeffamstutz commented 3 years ago

You too, happy to clarify! :)