FGFS-4.1/simgear
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Refactor SGBuildingBin

Browse Source 
Trying to trace a crash that was reported, but along the way fix a leak
of BuildingBins. Also avoid allocating some std::strings on the heap.
This commit is contained in:
James Turner 2020-09-02 19:52:14 +01:00 committed by Automatic Release Builder
parent e28c4fa5ca
commit b2e149a737
3 changed files with 79 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
simgear/scene/tgdb/ReaderWriterSTG.cxx
View File

@ -221,29 +221,41 @@ struct ReaderWriterSTG::_ModelBin {
if (signBuilder.getSignsGroup()) if (signBuilder.getSignsGroup())
group->addChild(signBuilder.getSignsGroup()); group->addChild(signBuilder.getSignsGroup());
if (_buildingList.size() > 0) { if (!_buildingList.empty()) {
SGMaterialLibPtr matlib = _options->getMaterialLib(); SGMaterialLibPtr matlib = _options->getMaterialLib();
bool useVBOs = (_options->getPluginStringData("SimGear::USE_VBOS") == "ON"); bool useVBOs = (_options->getPluginStringData("SimGear::USE_VBOS") == "ON");
if (!matlib) { if (!matlib) {
SG_LOG( SG_TERRAIN, SG_ALERT, "Unable to get materials definition for buildings"); SG_LOG( SG_TERRAIN, SG_ALERT, "Unable to get materials definition for buildings");
} else { } else {
for (std::list<_BuildingList>::iterator i = _buildingList.begin(); i != _buildingList.end(); ++i) { for (const auto& b : _buildingList) {
// Build buildings for each list of buildings // Build buildings for each list of buildings
SGGeod geodPos = SGGeod::fromDegM(i->_lon, i->_lat, 0.0); SGGeod geodPos = SGGeod::fromDegM(b._lon, b._lat, 0.0);
SGMaterial* mat = matlib->find(i->_material_name, geodPos); SGSharedPtr<SGMaterial> mat = matlib->find(b._material_name, geodPos);
SGPath path = SGPath(i->_filename);
// trying to avoid crash on null material, see:
// https://sentry.io/organizations/flightgear/issues/1867075869
if (!mat) {
SG_LOG(SG_TERRAIN, SG_ALERT, "Building specifies unknown material: " << b._material_name);
continue;
}
const auto path = SGPath(b._filename);
SGBuildingBin* buildingBin = new SGBuildingBin(path, mat, useVBOs); SGBuildingBin* buildingBin = new SGBuildingBin(path, mat, useVBOs);
SGBuildingBinList buildingBinList; SGBuildingBinList bbList;
buildingBinList.push_back(buildingBin); bbList.push_back(buildingBin);
osg::MatrixTransform* matrixTransform; osg::MatrixTransform* matrixTransform;
matrixTransform = new osg::MatrixTransform(makeZUpFrame(SGGeod::fromDegM(i->_lon, i->_lat, i->_elev))); matrixTransform = new osg::MatrixTransform(makeZUpFrame(SGGeod::fromDegM(b._lon, b._lat, b._elev)));
matrixTransform->setName("rotateBuildings"); matrixTransform->setName("rotateBuildings");
matrixTransform->setDataVariance(osg::Object::STATIC); matrixTransform->setDataVariance(osg::Object::STATIC);
matrixTransform->addChild(createRandomBuildings(buildingBinList, osg::Matrix::identity(), _options)); matrixTransform->addChild(createRandomBuildings(bbList, osg::Matrix::identity(), _options));
group->addChild(matrixTransform); group->addChild(matrixTransform);
std::for_each(bbList.begin(), bbList.end(), [](SGBuildingBin* bb) {
delete bb;
});
} }
} }
} }

73
simgear/scene/tgdb/SGBuildingBin.cxx
View File

@ -497,14 +497,20 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
}; };
// Set up the building set based on the material definitions // Set up the building set based on the material definitions
SGBuildingBin::SGBuildingBin(const SGMaterial *mat, bool useVBOs) { SGBuildingBin::SGBuildingBin(const SGMaterial* mat, bool useVBOs) : _material(const_cast<SGMaterial*>(mat))
material_name = new std::string(mat->get_names()[0]); {
SG_LOG(SG_TERRAIN, SG_DEBUG, "Building material " << material_name); const auto& materialNames = mat->get_names();
material = mat; if (materialNames.empty()) {
texture = new std::string(mat->get_building_texture()); SG_LOG(SG_TERRAIN, SG_DEV_ALERT, "SGBuildingBin: material has zero names defined");
lightMap = new std::string(mat->get_building_lightmap()); } else {
_materialName = materialNames.front();
SG_LOG(SG_TERRAIN, SG_DEBUG, "Building material " << _materialName);
}
_textureName = mat->get_building_texture();
_lightMapName = mat->get_building_lightmap();
buildingRange = mat->get_building_range(); buildingRange = mat->get_building_range();
SG_LOG(SG_TERRAIN, SG_DEBUG, "Building texture " << texture); SG_LOG(SG_TERRAIN, SG_DEBUG, "Building texture " << _textureName);
} }
SGBuildingBin::~SGBuildingBin() { SGBuildingBin::~SGBuildingBin() {
@ -652,15 +658,15 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
if (buildingtype == SGBuildingBin::SMALL) { if (buildingtype == SGBuildingBin::SMALL) {
// Small building // Small building
// Maximum number of floors is 3, and maximum width/depth is 192m. // Maximum number of floors is 3, and maximum width/depth is 192m.
width = material->get_building_small_min_width() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_small_max_width() - material->get_building_small_min_width()); width = _material->get_building_small_min_width() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_small_max_width() - _material->get_building_small_min_width());
depth = material->get_building_small_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_small_max_depth() - material->get_building_small_min_depth()); depth = _material->get_building_small_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_small_max_depth() - _material->get_building_small_min_depth());
floors = SGMisc<double>::round(material->get_building_small_min_floors() + mt_rand(&seed) * (material->get_building_small_max_floors() - material->get_building_small_min_floors())); floors = SGMisc<double>::round(_material->get_building_small_min_floors() + mt_rand(&seed) * (_material->get_building_small_max_floors() - _material->get_building_small_min_floors()));
height = floors * (2.8 + mt_rand(&seed)); height = floors * (2.8 + mt_rand(&seed));
// Small buildings are never deeper than they are wide. // Small buildings are never deeper than they are wide.
if (depth > width) { depth = width; } if (depth > width) { depth = width; }
pitch_height = (mt_rand(&seed) < material->get_building_small_pitch()) ? 3.0 : 0.0; pitch_height = (mt_rand(&seed) < _material->get_building_small_pitch()) ? 3.0 : 0.0;
if (pitch_height == 0.0) { if (pitch_height == 0.0) {
roof_shape = 0; roof_shape = 0;
@ -671,18 +677,18 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
} }
} else if (buildingtype == SGBuildingBin::MEDIUM) { } else if (buildingtype == SGBuildingBin::MEDIUM) {
// MEDIUM BUILDING // MEDIUM BUILDING
width = material->get_building_medium_min_width() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_medium_max_width() - material->get_building_medium_min_width()); width = _material->get_building_medium_min_width() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_medium_max_width() - _material->get_building_medium_min_width());
depth = material->get_building_medium_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_medium_max_depth() - material->get_building_medium_min_depth()); depth = _material->get_building_medium_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_medium_max_depth() - _material->get_building_medium_min_depth());
floors = SGMisc<double>::round(material->get_building_medium_min_floors() + mt_rand(&seed) * (material->get_building_medium_max_floors() - material->get_building_medium_min_floors())); floors = SGMisc<double>::round(_material->get_building_medium_min_floors() + mt_rand(&seed) * (_material->get_building_medium_max_floors() - _material->get_building_medium_min_floors()));
height = floors * (2.8 + mt_rand(&seed)); height = floors * (2.8 + mt_rand(&seed));
while ((height > width) && (floors > material->get_building_medium_min_floors())) { while ((height > width) && (floors > _material->get_building_medium_min_floors())) {
// Ensure that medium buildings aren't taller than they are wide // Ensure that medium buildings aren't taller than they are wide
floors--; floors--;
height = floors * (2.8 + mt_rand(&seed)); height = floors * (2.8 + mt_rand(&seed));
} }
pitch_height = (mt_rand(&seed) < material->get_building_medium_pitch()) ? 3.0 : 0.0; pitch_height = (mt_rand(&seed) < _material->get_building_medium_pitch()) ? 3.0 : 0.0;
if (pitch_height == 0.0) { if (pitch_height == 0.0) {
roof_shape = 0; roof_shape = 0;
@ -693,11 +699,11 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
} }
} else { } else {
// LARGE BUILDING // LARGE BUILDING
width = material->get_building_large_min_width() + mt_rand(&seed) * (material->get_building_large_max_width() - material->get_building_large_min_width()); width = _material->get_building_large_min_width() + mt_rand(&seed) * (_material->get_building_large_max_width() - _material->get_building_large_min_width());
depth = material->get_building_large_min_depth() + mt_rand(&seed) * (material->get_building_large_max_depth() - material->get_building_large_min_depth()); depth = _material->get_building_large_min_depth() + mt_rand(&seed) * (_material->get_building_large_max_depth() - _material->get_building_large_min_depth());
floors = SGMisc<double>::round(material->get_building_large_min_floors() + mt_rand(&seed) * (material->get_building_large_max_floors() - material->get_building_large_min_floors())); floors = SGMisc<double>::round(_material->get_building_large_min_floors() + mt_rand(&seed) * (_material->get_building_large_max_floors() - _material->get_building_large_min_floors()));
height = floors * (2.8 + mt_rand(&seed)); height = floors * (2.8 + mt_rand(&seed));
pitch_height = (mt_rand(&seed) < material->get_building_large_pitch()) ? 3.0 : 0.0; pitch_height = (mt_rand(&seed) < _material->get_building_large_pitch()) ? 3.0 : 0.0;
if (pitch_height == 0.0) { if (pitch_height == 0.0) {
roof_shape = 0; roof_shape = 0;
@ -726,12 +732,11 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
} }
SGBuildingBin::BuildingType SGBuildingBin::getBuildingType(float roll) { SGBuildingBin::BuildingType SGBuildingBin::getBuildingType(float roll) {
if (roll < _material->get_building_small_fraction()) {
if (roll < material->get_building_small_fraction()) {
return SGBuildingBin::SMALL; return SGBuildingBin::SMALL;
} }
if (roll < (material->get_building_small_fraction() + material->get_building_medium_fraction())) { if (roll < (_material->get_building_small_fraction() + _material->get_building_medium_fraction())) {
return SGBuildingBin::MEDIUM; return SGBuildingBin::MEDIUM;
} }
@ -739,23 +744,23 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
} }
float SGBuildingBin::getBuildingMaxRadius(BuildingType type) { float SGBuildingBin::getBuildingMaxRadius(BuildingType type) {
if (type == SGBuildingBin::SMALL) return material->get_building_small_max_width(); if (type == SGBuildingBin::SMALL) return _material->get_building_small_max_width();
if (type == SGBuildingBin::MEDIUM) return material->get_building_medium_max_width(); if (type == SGBuildingBin::MEDIUM) return _material->get_building_medium_max_width();
if (type == SGBuildingBin::LARGE) return material->get_building_large_max_width(); if (type == SGBuildingBin::LARGE) return _material->get_building_large_max_width();
return 0; return 0;
} }
float SGBuildingBin::getBuildingMaxDepth(BuildingType type) { float SGBuildingBin::getBuildingMaxDepth(BuildingType type) {
if (type == SGBuildingBin::SMALL) return material->get_building_small_max_depth(); if (type == SGBuildingBin::SMALL) return _material->get_building_small_max_depth();
if (type == SGBuildingBin::MEDIUM) return material->get_building_medium_max_depth(); if (type == SGBuildingBin::MEDIUM) return _material->get_building_medium_max_depth();
if (type == SGBuildingBin::LARGE) return material->get_building_large_max_depth(); if (type == SGBuildingBin::LARGE) return _material->get_building_large_max_depth();
return 0; return 0;
} }
ref_ptr<Group> SGBuildingBin::createBuildingsGroup(Matrix transInv, const SGReaderWriterOptions* options) ref_ptr<Group> SGBuildingBin::createBuildingsGroup(Matrix transInv, const SGReaderWriterOptions* options)
{ {
ref_ptr<Effect> effect; ref_ptr<Effect> effect;
EffectMap::iterator iter = buildingEffectMap.find(*texture); auto iter = buildingEffectMap.find(_textureName);
if ((iter == buildingEffectMap.end())|| if ((iter == buildingEffectMap.end())||
(!iter->second.lock(effect))) (!iter->second.lock(effect)))
@ -764,16 +769,14 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
makeChild(effectProp, "inherits-from")->setStringValue("Effects/building"); makeChild(effectProp, "inherits-from")->setStringValue("Effects/building");
SGPropertyNode* params = makeChild(effectProp, "parameters"); SGPropertyNode* params = makeChild(effectProp, "parameters");
// Main texture - n=0 // Main texture - n=0
params->getChild("texture", 0, true)->getChild("image", 0, true) params->getChild("texture", 0, true)->getChild("image", 0, true)->setStringValue(_textureName);
->setStringValue(*texture);
// Light map - n=3 // Light map - n=3
params->getChild("texture", 3, true)->getChild("image", 0, true) params->getChild("texture", 3, true)->getChild("image", 0, true)->setStringValue(_lightMapName);
->setStringValue(*lightMap);
effect = makeEffect(effectProp, true, options); effect = makeEffect(effectProp, true, options);
if (iter == buildingEffectMap.end()) if (iter == buildingEffectMap.end())
buildingEffectMap.insert(EffectMap::value_type(*texture, effect)); buildingEffectMap.insert(EffectMap::value_type(_textureName, effect));
else else
iter->second = effect; // update existing, but empty observer iter->second = effect; // update existing, but empty observer
} }

11
simgear/scene/tgdb/SGBuildingBin.hxx
View File

@ -145,12 +145,11 @@ public:
}; };
private: private:
const SGSharedPtr<SGMaterial> _material;
const SGMaterial *material; std::string _materialName;
std::string _textureName;
std::string* material_name; std::string _lightMapName;
std::string* texture;
std::string* lightMap;
// Visibility range for buildings // Visibility range for buildings
float buildingRange; float buildingRange;
@ -187,7 +186,7 @@ public:
bool checkMinDist (SGVec3f p, float radius); bool checkMinDist (SGVec3f p, float radius);
std::string* getMaterialName() { return material_name; } const std::string& getMaterialName() const { return _materialName; }
BuildingType getBuildingType(float roll); BuildingType getBuildingType(float roll);
float getBuildingMaxRadius(BuildingType); float getBuildingMaxRadius(BuildingType);