RenderKit / ospray

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

Misleading warning relating to parameter type-check #536

Open paulmelis opened 2 years ago

paulmelis commented 2 years ago

Updating ospTutorial.cpp to render a single quad instead of 2 triangles:

std::vector<vec4ui> index = {vec4ui(0, 1, 2, 3)};

main()
{
    ospray::cpp::Geometry mesh("mesh");
    // ....
    mesh.setParam("index", ospray::cpp::CopiedData(index));
    mesh.commit();
}

With 2.10 and running with --osp:debug this gives an incorrect warning, as the index array isn't ignored, judging by the second line:

ospray::Mesh ignoring 'index' 1D array with element type vec4ui (while looking for 1D array of vec3ui)
ospray::Mesh created: #primitives=1, #vertices=4
johguenther commented 2 years ago

Similar to #396, where we agreed that a rewording to "while looking for 1D array of vec3ui" softens the edge of the warning. Apparently not enough?

paulmelis commented 2 years ago

Hmm, yes, I remember that one now. It indeed still throws me off, even with the rewording. I guess the main issue (probably not easy to solve) is that this type of warning is generated as part of a sequence of checks to set the parameter value, based on the supported types. But having a warning when one of those type option checks fails isn't that useful, because what is the user supposed to do with that information? As long as there's later a successful type match no warning is actually needed for the non-matching types.

In short: it would be better to only emit a warning if all types checked were incompatible, as the current form highlights a partial fail without explicitly mentioning the final success (i.e. nothing to worry about). And even though I originally reported the issue at the beginning of 2020, more than 2 years later it still puts me on the wrong foot, especially after picking up working with OSPRay again after a long time :grin:

johguenther commented 2 years ago

I agree that the warning is not ideal. Improving that will need a rather large overhaul of the parameter checking mechanisms (which may come with more ANARI conformance and infrastructure).