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

Reworked simulations renderers #512

Merged
merged 5 commits into from
Aug 15, 2018
Merged

Reworked simulations renderers #512

merged 5 commits into from
Aug 15, 2018

Conversation

favreau
Copy link
Member

@favreau favreau commented Aug 13, 2018

…g speed

@favreau favreau requested a review from karjonas August 13, 2018 16:24
@@ -1,9 +1,7 @@
/* Copyright (c) 2015-2016, EPFL/Blue Brain Project
/* Copyright (c) 2015-2017, EPFL/Blue Brain Project
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018

uint32 transferFunctionSize;

// Alpha correction
float alphaCorrection;
};

inline varying vec4f
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar function is used also for the simulation renderer, can they be merged?

@favreau favreau changed the title Reworked the particle renderer to improve visual quality and renderin… Reworked the simulations renderers Aug 14, 2018
@favreau favreau changed the title Reworked the simulations renderers Reworked simulations renderers Aug 14, 2018
@favreau favreau force-pushed the master branch 2 times, most recently from 30f2623 to 7ffcb6f Compare August 14, 2018 16:17
@favreau
Copy link
Member Author

favreau commented Aug 14, 2018

Retest this please

namespace brayns
{
/**
* The AbstractRenderer class implements a base renderer for all Brayns custom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// Value offset is out of range, return error color
return color;

// Normalize the value according colormap size
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according (to) colormap

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


struct BasicSimulationRenderer
{
SimulationRenderer abstract;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better name than abstract, like simulation_renderer or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to super (to remain consistent with OSPRay)

{
AbstractRenderer abstract;
SimulationRenderer abstract;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better name than abstract, like simulation_renderer or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito

@ppodhajski
Copy link
Contributor

retest this please

@karjonas
Copy link
Contributor

Why is tests/files/images/testdataallmini50color.png updated? Should it not be the same still?

@favreau favreau merged commit c01ca64 into BlueBrain:master Aug 15, 2018
favreau pushed a commit to favreau/Brayns that referenced this pull request Aug 15, 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.

4 participants