Added properties to materials #667
Conversation
brayns/engine/Model.h
Outdated
| @@ -387,7 +388,7 @@ class Model | |||
| * existing geometry in the model. Missing materials are created with the | |||
| * default parameters | |||
| */ | |||
| BRAYNS_API void createMissingMaterials(); | |||
| BRAYNS_API void createMissingMaterials(const PropertyMap& properties = {}); | |||
There was a problem hiding this comment.
that is not called other than this anyways, so the parameter is not needed for this function?!
There was a problem hiding this comment.
Properties are forwarded to the material itself (line 558). It's true that Brayns never calls this method with properties, but this can still be done by a plugin for instance (that's actually what I do and why I need this extra parameter).
There was a problem hiding this comment.
Shouldn't you pass the material type name as well?
There was a problem hiding this comment.
As discussed offline, this createMissingMaterials() shall be gone completely. The CircuitViewer plugin uses it though, but imo that shouldn't be necessary. @hernando could you have a look there?
|
lgtm |
engines/ospray/OSPRayModel.cpp
Outdated
| { | ||
| MaterialPtr material = std::make_shared<OSPRayMaterial>(); | ||
| material->setName(name); | ||
| material->setProperties(properties); |
There was a problem hiding this comment.
Add a constructor with these parameters parameters?
brayns/engine/Model.h
Outdated
| @@ -387,7 +388,7 @@ class Model | |||
| * existing geometry in the model. Missing materials are created with the | |||
| * default parameters | |||
| */ | |||
| BRAYNS_API void createMissingMaterials(); | |||
| BRAYNS_API void createMissingMaterials(const PropertyMap& properties = {}); | |||
There was a problem hiding this comment.
Shouldn't you pass the material type name as well?
brayns/engine/Scene.cpp
Outdated
| @@ -440,6 +440,9 @@ void Scene::_updateEnvironmentMap() | |||
| } | |||
|
|
|||
| if (_backgroundMaterial->isModified()) | |||
| { | |||
| _backgroundMaterial->resetModified(); // TODO???? | |||
There was a problem hiding this comment.
no, to my knowledge that's wrong. All the materials are reset at some point
| @@ -702,4 +704,32 @@ PropertyMap MorphologyLoader::getCLIProperties() | |||
| pm.setProperty(PROP_USE_SDF_GEOMETRIES); | |||
| return pm; | |||
| } | |||
|
|
|||
| void createMissingMaterials(Model& model) | |||
There was a problem hiding this comment.
that's still nuclear bomb to kill a fly :) The materials that are required to be created should be known in here, but @hernando said he will look into this.
There was a problem hiding this comment.
I would make it a different PR.
There was a problem hiding this comment.
Let me figure it out, I was the last one touching that code.
plugins/CircuitViewer/io/utils.h
Outdated
| @@ -27,8 +27,8 @@ namespace brayns | |||
| { | |||
| namespace | |||
| { | |||
| brion::GIDSet _gidsFromRange(const uint32_t first, const uint32_t last, | |||
| const double fraction, const int32_t seed) | |||
| inline brion::GIDSet _gidsFromRange(const uint32_t first, const uint32_t last, | |||
There was a problem hiding this comment.
I somewhat had to make this inline in order to avoid a 'defined but not used [-Werror=unused-function]' compilation error :-/
plugins/CircuitViewer/io/utils.h
Outdated
| @@ -59,8 +59,8 @@ brion::GIDSet _gidsFromRange(const uint32_t first, const uint32_t last, | |||
| return brion::GIDSet(gids.begin(), gids.end()); | |||
| } | |||
|
|
|||
| brion::GIDSet _keyToGIDorRange(const std::string& key, const double fraction, | |||
| const int32_t seed) | |||
| inline brion::GIDSet _keyToGIDorRange(const std::string& key, | |||
brayns/engine/Scene.cpp
Outdated
| @@ -440,6 +440,9 @@ void Scene::_updateEnvironmentMap() | |||
| } | |||
|
|
|||
| if (_backgroundMaterial->isModified()) | |||
| { | |||
| _backgroundMaterial->resetModified(); // TODO???? | |||
| @@ -702,4 +704,32 @@ PropertyMap MorphologyLoader::getCLIProperties() | |||
| pm.setProperty(PROP_USE_SDF_GEOMETRIES); | |||
| return pm; | |||
| } | |||
|
|
|||
| void createMissingMaterials(Model& model) | |||
There was a problem hiding this comment.
I would make it a different PR.
favreau
force-pushed
the
master
branch
2 times, most recently
from
c59b117
to
e4b15bf
Compare
| @@ -107,6 +107,8 @@ class MorphologyLoader : public Loader | |||
|
|
|||
| PropertyMap _defaults; // command line defaults | |||
| }; | |||
|
|
|||
| void createMissingMaterials(Model& model); | |||
There was a problem hiding this comment.
rm this line aka revert this file
Adding properties to materials allows definition of extra properties that can be processed by the renderer.