OSPRay 2.x curves API docs should be more self-contained...

[tachyon-john]

While trying to chase down an OSPRay 2.x error callback "embree internal error 'invalid argument'" for a very simple OSP_LINEAR, OSP_ROUND, curve with per-vertex-radii, I revisited the OSPRay docs to ensure that my vertex buffer was packed with the correct x/y/z/r formatted vec4fs as I had expected.

The current OSPRay docs more or less punt on explaining how to use the per-vertex curve variant, with only the following text: "Positions in vertex.position_radius format supports per-vertex varying radii with data type vec4f[] and instantiate Embree curves internally for the relevant type/basis mapping (See Embree documentation for discussion of curve types and data formatting)."

When I go to the Embree docs, I get the required details about vertex packing (which match what I had done), but then there is a lot of extra detail about the RTCurveFlags and the Embree-required "flags" buffer, which are clearly not really applicable when we're working with the OSPRay API.

I'm able to swim through the two sets of docs without drowning, but it's annoying to have to filter out non-applicable statements in the Embree docs while reading, and I'm sure that most OSPRay users would rather that the OSPRay curves API docs were unabridged here. I think a little selective cutting and pasting and editing from the Embree docs would take care of this. If the OSPRay curves docs are deferring to Embree docs because Embree's internal vertex packing data layouts are expected to change in the future, maybe that would be best explicitly stated.

I would approach the writing the OSPRay docs as if Embree didn't even exist, except in cases where it's absolutely necessary, as most users of OSPRay will probably have that mindset unless they're going to get into building custom OSPRay extension modules...

Best regards, John

[paulmelis]

Regarding the error you get it seems the per-vertex radii are not supported for OSP_LINEAR. At least, that's the impression I get from looking at apps/common/ospray_testing/builders/Curves.cpp where that case is missing.

Oh, and looking at this piece from ospray/geometry/Curves.cpp shouldn't you get an exception at commit() time for the case of using vertex.position_radius with basis OSP_LINEAR and type OSP_ROUND?

void Curves::commit()
{
  indexData = getParamDataT<uint32_t>("index", true);
  normalData = nullptr;
  tangentData = nullptr;
  vertexData = getParamDataT<vec3f>("vertex.position");
  texcoordData = getParamDataT<vec2f>("vertex.texcoord");

  if (vertexData) { // round, linear curves with constant radius
    radius = getParam<float>("radius", 0.01f);
    curveType = (OSPCurveType)getParam<int>("type", OSP_ROUND);
    curveBasis = (OSPCurveBasis)getParam<int>("basis", OSP_LINEAR);
    if (curveMap[std::make_pair(curveType, curveBasis)]
        != RTC_GEOMETRY_TYPE_USER)
      throw std::runtime_error(
          "constant-radius curves need to be of type OSP_ROUND and have "
          "basis OSP_LINEAR");
  } else { // embree curves
    vertexData = getParamDataT<vec4f>("vertex.position_radius", true);
    radius = 0.0f;

    curveType = (OSPCurveType)getParam<int>("type", OSP_UNKNOWN_CURVE_TYPE);
    if (curveType == OSP_UNKNOWN_CURVE_TYPE)
      throw std::runtime_error("curves geometry has invalid 'type'");

    curveBasis = (OSPCurveBasis)getParam<int>("basis", OSP_UNKNOWN_CURVE_BASIS);

    if (curveBasis == OSP_UNKNOWN_CURVE_BASIS)
      throw std::runtime_error("curves geometry has invalid 'basis'");

    if (curveBasis == OSP_LINEAR && curveType != OSP_FLAT) {
      throw std::runtime_error(
          "curves geometry with linear basis must be of flat type or have "
          "constant radius");
    }

[tachyon-john]

I definitely get no runtime exception, only the error callback gets triggered, and only with the mysterious text: embree internal error 'invalid argument' It seems the curves implementation in OSPRay still has some loose ends, but I would think they should be reasonably easy to shore up.

[paulmelis]

I definitely get no runtime exception

Yes it's strange. Even leaving off the (say) type parameter should trigger in this case. But the exceptions are probably handled higher up and perhaps are turned into warning messages.

[tachyon-john]

Yeah, I have been wondering if the fact that I have an error handler callback registered is preventing OSPRay from triggering the runtime error exceptions deep down in the code snippet you posted. If having an error callback registered alters runtime error handling strategy at all, that would be worth mentioning in the docs, but we'll see what the team says.

[jeffamstutz]

Re exceptions: because OSPRay is a C99 API, it's never acceptable to leak exceptions through public API calls, so all internally thrown exceptions will always be turned into errors invoking the user set error callback.

That is true for C++ exceptions, other cases of error handling and warnings are done through other more explicit means, which may have gaps. For example, there some issues in our curves geometry which isn't properly validated before getting to Embree.

[tachyon-john]

Jeff, in the case here where there's a missing implementation, how is it that I'm getting the Embree error rather than the exception messages in the code? Is Embree triggering first here for some reason? It makes perfect sense to convert the exceptions into error callbacks. Now I'm just wondering about why I'm not seeing OSPRay-specific error text in this particular case where Paul has shown that there's an unsupported/unimplemented combination and at least OSPRay itself does have safety checks there.

[paulmelis]

Re exceptions: because OSPRay is a C99 API, it's never acceptable to leak exceptions through public API calls, so all internally thrown exceptions will always be turned into errors invoking the user set error callback.

Ah, of course, makes sense. I can see the exception being thrown in the code for the unsupported curves case, but it doesn't get turned into a device error it seems.

[paulmelis]

Ah, of course, makes sense. I can see the exception being thrown in the code for the unsupported curves case, but it doesn't get turned into a device error it seems.

I was wrong, the error function is called, with message "curves geometry with linear basis must be of flat type or have constant radius", followed by the "embree internal error 'invalid argument'".

@tachyon-john Are you not seeing those errors with your own error handler?

Edit: what is weird is that when using --osp:debug and not setting any user error handler only the Embree error is printed, but not the curves error. @jeffamstutz is that expected behaviour?

[tachyon-john]

Paul, I did a fresh built/test cycle and double-checked, and it turns out that the I missed the first error callback in the long scrollback for the VMD test scene I'm using, with all my debugging output enabled. After re-running and checking all the way back to the earliest phase of rendering, I do get both error messages as expected. So, that resolves the only question I still had there. I missed the earlier message simply as a result of the fact that my code is generating the GeometricModel objects in one phase and finalizing the complete set of Instances in the world much later in a different phase. I had only seen the second Embree internal errors during the last phase when the last Group/Instance objects were being allocated/assigned/committed to the world.

[johguenther]

Sorry for the delayed answer from my end. Some additional notes:

• I totally agree that OSPRay's documentation should be self-contained. I though we fixed that for curves (see the sections below, explaining the different curve types and its requirements), but we apparently missed to remove the first paragraph pointing to the Embree docu. Will be fixed in next release.
• Curves indeed has some loose ends
  • implementation is currently a mix of using Embree's curves and a user geometry; we want to only use Embree in future, to also the support currently missing round-linear-varying-radius combination (which however would also mean that the constant radius case will internally be translated to varying radius, which leads to more memory consumption due to copied position and radius data – comments?)
  • vertex.tangent for HERMITE is actually a vec4f, containing the derivative of the radius in the 4th component.
• would be nice to know where the reported Embree error comes from: is this from the Group, referencing an invalid geometry (the not-created curves)?

[paulmelis]

which however would also mean that the constant radius case will internally be translated to varying radius, which leads to more memory consumption due to copied position and radius data – comments?

Having a single radius value on the geometry on the OSPRay side would be a good thing to keep, in my opinion, regardless of the underlying implementation. If using Embree-only geometry is the best way then I guess the extra memory usage and bit of setup time underwater is worth it for the fixed-radius case. You can always use the position_radius variant if you don't want the extra copy.

would be nice to know where the reported Embree error comes from: is this from the Group, referencing an invalid geometry (the not-created curves)?

It's indeed the Group commit.

[paulmelis]

Edit: what is weird is that when using --osp:debug and not setting any user error handler only the Embree error is printed, but not the curves error. @jeffamstutz is that expected behaviour?

Ignore this one please, was an error on my side

[tachyon-john]

VMD contains use cases for each of these scenarios: a) single-radius per entire array of cylinders, b) radius-per cylinder, c) per-vertex radii (fields of cones, and so-called "sausage" tube representations with atomic-property-modulated width

If the back-end (Embree) only supports per-vertex radii natively, then I would likely setup VMD to translate all of its internal cylinder/cone geometry to the Embree-native format on-the-fly, to favor the Embree back-end's native implementation without additional copies or memory use.

[johguenther]

c) will always work, thus fine if VMD wants to the conversion. a) will be offered by OSPRay for convenience (internal conversion) b) no plans to re-add this

[tachyon-john]

@johguenther So, for VMD to implement b) it will just emit pairs of verts with the same radius for each cylinder and it ought to be fine.