Skip to content
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

Merged
merged 3 commits into from
Feb 22, 2018

Conversation

favreau
Copy link
Member

@favreau favreau commented Feb 21, 2018

Use the --renderer/--camera command line arguments to specify the renderer/camera that Brayns should use as a default.

@favreau
Copy link
Member Author

favreau commented Feb 21, 2018

Retest this please

@favreau favreau force-pushed the master branch 2 times, most recently from dfe1899 to 5953e96 Compare February 21, 2018 18:51
#include <ospray/SDK/common/OSPCommon.h>

namespace brayns
{
OSPRayCamera::OSPRayCamera(const CameraType cameraType)
: Camera(cameraType)
OSPRayCamera::OSPRayCamera(ParametersManager& parametersManager)
Copy link
Contributor

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...

Copy link
Member Author

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...

Copy link
Member Author

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",
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

@tribal-tec tribal-tec left a 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) /
Copy link
Contributor

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();
Copy link
Contributor

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",
Copy link
Contributor

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)
Copy link
Contributor

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())
Copy link
Contributor

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);
Copy link
Contributor

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());
Copy link
Contributor

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

@favreau favreau merged commit 9c1b7cc into BlueBrain:master Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants