Clipping planes #486
Clipping planes #486
Conversation
brayns/common/camera/Camera.h
Outdated
| virtual bool isSideBySideStereo() const { return false; } | ||
|
|
||
| /** @intenal */ | ||
| virtual void takeSceneParameters(const Scene&) {} |
There was a problem hiding this comment.
can be in ospray layer only?
There was a problem hiding this comment.
That's what I had originally, but I couldn't find a way to include the clipping planes in the SnapshotFunctor without doing dynamic_cast to the OSPRay objects.
There was a problem hiding this comment.
Hmm, why not let the renderer handle in both cases (snapshot and 'normal' use, rather than in the engine)? The renderer knows camera and scene. I'd like to avoid to leak this 'knowledge' about clipping planes update like that.
There was a problem hiding this comment.
The only solution would be to do it in Renderer::commit, but for that to work it needs to become responsible of committing the camera and the scene. The reason is that I cannot commit the camera before the planes are set and committing the camera after the renderer was not working. If you're happy with that, I can give it a try.
However, it's unclear if we will be causing a new problem as the camera and scene commit are not supposed to be called from somewhere else anymore.
Another solution is to commit. the camera from the OSPRay base renderer regardless of the commits done externally. This may be redundant work, but if the overhead is negligible it may be the cleanest solution.
In both cases I wonder what happens with alternative renderers, are all the different ISPC codes using OSPRayRenderer as the C++ side object? Is there a plugin that overrides OSPRayRenderer and is it supposed to call its commit methods in that case?
There was a problem hiding this comment.
Solution 2) is definitely working, and you would only commit the camera from the renderer if the scene/clipping-planes are modified, so not much overhead normally.
The OSPRayRenderer cannot be overridden by plugins; only engine-agnostic classes are visible for plugins. Plugins are also advised to avoid calling commit() on the brayns objects.
| @@ -198,6 +198,7 @@ void OSPRayEngine::preRender() | |||
| const auto& renderParams = _parametersManager.getRenderingParameters(); | |||
| if (renderParams.getAccumulation() != _frameBuffer->getAccumulation()) | |||
| _frameBuffer->setAccumulation(renderParams.getAccumulation()); | |||
| _camera->takeSceneParameters(*_scene); | |||
There was a problem hiding this comment.
just cast camera to OSPRayCamera to not need takeSceneParameters() in base class
There was a problem hiding this comment.
I don't even need to cast because _camera is an OSPCamera. Originally I had _camera->setClipPlanes(_scene->getClipPlanes()), but I changed it to make the smallest modification that also works in the SnapshotFunctor.
hernando
force-pushed
the
master
branch
4 times, most recently
from
afb2daa
to
4f88a8f
Compare
|
ping |
| @@ -43,6 +43,9 @@ class OSPRayCamera : public Camera | |||
| */ | |||
| void commit() final; | |||
|
|
|||
| /** */ | |||
|
retest this please |
hernando
force-pushed
the
master
branch
2 times, most recently
from
60d634c
to
5c5421a
Compare
From 0 to n planes now supported instead of either none or exactly 6.
There was a problem hiding this comment.
I wouldn't have added this to the Python client for Brayns.
Parsing data or inferring the type of the data should be the responsibility of the user and not the lib (in this case the params). The user should make sure that the passed data/params is proper json serializable.
No description provided.