-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow use of renderers and cameras defined in external modules #310
Conversation
Retest this please |
dfe1899
to
5953e96
Compare
#include <ospray/SDK/common/OSPCommon.h> | ||
|
||
namespace brayns | ||
{ | ||
OSPRayCamera::OSPRayCamera(const CameraType cameraType) | ||
: Camera(cameraType) | ||
OSPRayCamera::OSPRayCamera(ParametersManager& parametersManager) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const& at least. still quite ugly that a engine-specific camera needs full access to all parameters...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can restrict it to renderingParameters instead. Does this looks better to you? Or same...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, cannot be const because it can be reinitialized by the camera
"raycast_Ns", | ||
"scivis"}; | ||
|
||
const std::string CAMERA_TYPE_NAMES[5] = {"perspective", "stereo", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the JSON name also has to change to default, no? for both, camera and renderer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already done in the jsonSerialization.h file. CAMERA_TYPE_NAMES defines the internal 'ospray' name of the camera, not the one that is exposed in JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost ;)
void RenderingParameters::initializeDefaultRenderers() | ||
{ | ||
_rendererNames.clear(); | ||
for (size_t i = 0; i < sizeof(RENDERER_INTERNAL_NAMES) / |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just _rendererNames{internal, internal+size} in ctor list
@@ -145,6 +159,14 @@ RenderingParameters::RenderingParameters() | |||
_renderers.push_back(RendererType::scientificvisualization); | |||
} | |||
|
|||
void RenderingParameters::initializeDefaultCameras() | |||
{ | |||
_cameraTypeNames.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as for renderers, just init list
|
||
const std::string CAMERA_TYPES[5] = {"perspective", "stereo", "orthographic", | ||
"panoramic", "clipped"}; | ||
const std::string RENDERER_INTERNAL_NAMES[7] = {"basic", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std.array instead, in case you need .size()
const std::string& renderer = vm[PARAM_RENDERER].as<std::string>(); | ||
for (size_t i = 0; i < sizeof(RENDERERS) / sizeof(RENDERERS[0]); ++i) | ||
if (renderer == RENDERERS[i]) | ||
for (size_t i = 0; !found && i < _rendererNames.size(); ++i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or shorter: found = std.find() == end()
{ | ||
Renderers renderersForScene; | ||
auto& rp = _parametersManager.getRenderingParameters(); | ||
for (const auto renderer : rp.getRenderers()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto&?
auto& rp = _parametersManager.getRenderingParameters(); | ||
for (const auto renderer : rp.getRenderers()) | ||
{ | ||
auto name = rp.getRendererAsString(renderer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto&?
@@ -32,9 +32,11 @@ OSPRayRenderer::OSPRayRenderer(const std::string& name, | |||
: Renderer(parametersManager) | |||
, _name(name) | |||
, _camera(0) | |||
, _renderer(0) | |||
{ | |||
_renderer = ospNewRenderer(name.c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to init list directly
Use the --renderer/--camera command line arguments to specify the renderer/camera that Brayns should use as a default.