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.

Sign up for GitHub

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

Jump to bottom

Fix crash in 3d when changing symbol properties #38413

Merged
merged 2 commits into from
Aug 21, 2020
Merged

Fix crash in 3d when changing symbol properties #38413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f97b84
Don't store references to symbols in 3d symbol handlers
nyalldawson Aug 21, 2020
a3ee8c1
Fix memory leak when no model point positions are available
nyalldawson Aug 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 28 additions & 25 deletions src/3d/symbols/qgsline3dsymbol_p.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@
class QgsBufferedLine3DSymbolHandler : public QgsFeature3DHandler
{
public:
QgsBufferedLine3DSymbolHandler( const QgsLine3DSymbol &symbol, const QgsFeatureIds &selectedIds )
: mSymbol( symbol ), mSelectedIds( selectedIds ) {}
QgsBufferedLine3DSymbolHandler( const QgsLine3DSymbol *symbol, const QgsFeatureIds &selectedIds )
: mSymbol( static_cast< QgsLine3DSymbol *>( symbol->clone() ) )
, mSelectedIds( selectedIds ) {}

bool prepare( const Qgs3DRenderContext &context, QSet<QString> &attributeNames ) override;
void processFeature( QgsFeature &feature, const Qgs3DRenderContext &context ) override;
Expand All @@ -67,7 +68,7 @@ class QgsBufferedLine3DSymbolHandler : public QgsFeature3DHandler
void makeEntity( Qt3DCore::QEntity *parent, const Qgs3DRenderContext &context, LineData &out, bool selected );

// input specific for this class
const QgsLine3DSymbol &mSymbol;
std::unique_ptr< QgsLine3DSymbol > mSymbol;
// inputs - generic
QgsFeatureIds mSelectedIds;

Expand All @@ -82,7 +83,7 @@ bool QgsBufferedLine3DSymbolHandler::prepare( const Qgs3DRenderContext &context,
{
Q_UNUSED( attributeNames )

const QgsPhongTexturedMaterialSettings *texturedMaterialSettings = dynamic_cast< const QgsPhongTexturedMaterialSettings * >( mSymbol.material() );
const QgsPhongTexturedMaterialSettings *texturedMaterialSettings = dynamic_cast< const QgsPhongTexturedMaterialSettings * >( mSymbol->material() );

outNormal.tessellator.reset( new QgsTessellator( context.map().origin().x(), context.map().origin().y(), true,
false, false, false, texturedMaterialSettings ? texturedMaterialSettings->requiresTextureCoordinates() : false,
Expand Down Expand Up @@ -118,28 +119,28 @@ void QgsBufferedLine3DSymbolHandler::processFeature( QgsFeature &f, const Qgs3DR
const double mitreLimit = 0;

QgsGeos engine( g );
QgsAbstractGeometry *buffered = engine.buffer( mSymbol.width() / 2., nSegments, endCapStyle, joinStyle, mitreLimit ); // factory
QgsAbstractGeometry *buffered = engine.buffer( mSymbol->width() / 2., nSegments, endCapStyle, joinStyle, mitreLimit ); // factory

if ( QgsWkbTypes::flatType( buffered->wkbType() ) == QgsWkbTypes::Polygon )
{
QgsPolygon *polyBuffered = static_cast<QgsPolygon *>( buffered );
processPolygon( polyBuffered, f.id(), mSymbol.height(), mSymbol.extrusionHeight(), context, out );
processPolygon( polyBuffered, f.id(), mSymbol->height(), mSymbol->extrusionHeight(), context, out );
}
else if ( QgsWkbTypes::flatType( buffered->wkbType() ) == QgsWkbTypes::MultiPolygon )
{
QgsMultiPolygon *mpolyBuffered = static_cast<QgsMultiPolygon *>( buffered );
for ( int i = 0; i < mpolyBuffered->numGeometries(); ++i )
{
QgsPolygon *polyBuffered = static_cast<QgsPolygon *>( mpolyBuffered->polygonN( i ) )->clone(); // need to clone individual geometry parts
processPolygon( polyBuffered, f.id(), mSymbol.height(), mSymbol.extrusionHeight(), context, out );
processPolygon( polyBuffered, f.id(), mSymbol->height(), mSymbol->extrusionHeight(), context, out );
}
delete buffered;
}
}

void QgsBufferedLine3DSymbolHandler::processPolygon( QgsPolygon *polyBuffered, QgsFeatureId fid, float height, float extrusionHeight, const Qgs3DRenderContext &context, LineData &out )
{
Qgs3DUtils::clampAltitudes( polyBuffered, mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), height, context.map() );
Qgs3DUtils::clampAltitudes( polyBuffered, mSymbol->altitudeClamping(), mSymbol->altitudeBinding(), height, context.map() );

Q_ASSERT( out.tessellator->dataVerticesCount() % 3 == 0 );
uint startingTriangleIndex = static_cast<uint>( out.tessellator->dataVerticesCount() / 3 );
Expand Down Expand Up @@ -168,13 +169,13 @@ void QgsBufferedLine3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, cons
QgsMaterialContext materialContext;
materialContext.setIsSelected( selected );
materialContext.setSelectionColor( context.map().selectionColor() );
Qt3DRender::QMaterial *mat = mSymbol.material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Triangles, materialContext );
Qt3DRender::QMaterial *mat = mSymbol->material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Triangles, materialContext );

// extract vertex buffer data from tessellator
QByteArray data( ( const char * )out.tessellator->data().constData(), out.tessellator->data().count() * sizeof( float ) );
int nVerts = data.count() / out.tessellator->stride();

const QgsPhongTexturedMaterialSettings *texturedMaterialSettings = dynamic_cast< const QgsPhongTexturedMaterialSettings * >( mSymbol.material() );
const QgsPhongTexturedMaterialSettings *texturedMaterialSettings = dynamic_cast< const QgsPhongTexturedMaterialSettings * >( mSymbol->material() );

QgsTessellatedPolygonGeometry *geometry = new QgsTessellatedPolygonGeometry( true, false, false,
texturedMaterialSettings ? texturedMaterialSettings->requiresTextureCoordinates() : false );
Expand Down Expand Up @@ -203,8 +204,9 @@ void QgsBufferedLine3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, cons
class QgsSimpleLine3DSymbolHandler : public QgsFeature3DHandler
{
public:
QgsSimpleLine3DSymbolHandler( const QgsLine3DSymbol &symbol, const QgsFeatureIds &selectedIds )
: mSymbol( symbol ), mSelectedIds( selectedIds )
QgsSimpleLine3DSymbolHandler( const QgsLine3DSymbol *symbol, const QgsFeatureIds &selectedIds )
: mSymbol( static_cast< QgsLine3DSymbol *>( symbol->clone() ) )
, mSelectedIds( selectedIds )
{
}

Expand All @@ -218,7 +220,7 @@ class QgsSimpleLine3DSymbolHandler : public QgsFeature3DHandler
Qt3DExtras::QPhongMaterial *material( const QgsLine3DSymbol &symbol ) const;

// input specific for this class
const QgsLine3DSymbol &mSymbol;
std::unique_ptr< QgsLine3DSymbol > mSymbol;
// inputs - generic
QgsFeatureIds mSelectedIds;

Expand All @@ -233,8 +235,8 @@ bool QgsSimpleLine3DSymbolHandler::prepare( const Qgs3DRenderContext &context, Q
{
Q_UNUSED( attributeNames )

outNormal.init( mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), mSymbol.height(), &context.map() );
outSelected.init( mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), mSymbol.height(), &context.map() );
outNormal.init( mSymbol->altitudeClamping(), mSymbol->altitudeBinding(), mSymbol->height(), &context.map() );
outSelected.init( mSymbol->altitudeClamping(), mSymbol->altitudeBinding(), mSymbol->height(), &context.map() );

return true;
}
Expand Down Expand Up @@ -284,7 +286,7 @@ void QgsSimpleLine3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const
QgsMaterialContext materialContext;
materialContext.setIsSelected( selected );
materialContext.setSelectionColor( context.map().selectionColor() );
Qt3DRender::QMaterial *mat = mSymbol.material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Lines, materialContext );
Qt3DRender::QMaterial *mat = mSymbol->material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Lines, materialContext );

// geometry renderer

Expand Down Expand Up @@ -313,8 +315,9 @@ void QgsSimpleLine3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const
class QgsThickLine3DSymbolHandler : public QgsFeature3DHandler
{
public:
QgsThickLine3DSymbolHandler( const QgsLine3DSymbol &symbol, const QgsFeatureIds &selectedIds )
: mSymbol( symbol ), mSelectedIds( selectedIds )
QgsThickLine3DSymbolHandler( const QgsLine3DSymbol *symbol, const QgsFeatureIds &selectedIds )
: mSymbol( static_cast< QgsLine3DSymbol * >( symbol->clone() ) )
, mSelectedIds( selectedIds )
{
}

Expand All @@ -329,7 +332,7 @@ class QgsThickLine3DSymbolHandler : public QgsFeature3DHandler
Qt3DExtras::QPhongMaterial *material( const QgsLine3DSymbol &symbol ) const;

// input specific for this class
const QgsLine3DSymbol &mSymbol;
std::unique_ptr< QgsLine3DSymbol > mSymbol;
// inputs - generic
QgsFeatureIds mSelectedIds;

Expand All @@ -346,8 +349,8 @@ bool QgsThickLine3DSymbolHandler::prepare( const Qgs3DRenderContext &context, QS

outNormal.withAdjacency = true;
outSelected.withAdjacency = true;
outNormal.init( mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), mSymbol.height(), &context.map() );
outSelected.init( mSymbol.altitudeClamping(), mSymbol.altitudeBinding(), mSymbol.height(), &context.map() );
outNormal.init( mSymbol->altitudeClamping(), mSymbol->altitudeBinding(), mSymbol->height(), &context.map() );
outSelected.init( mSymbol->altitudeClamping(), mSymbol->altitudeBinding(), mSymbol->height(), &context.map() );

return true;
}
Expand Down Expand Up @@ -396,15 +399,15 @@ void QgsThickLine3DSymbolHandler::makeEntity( Qt3DCore::QEntity *parent, const Q
QgsMaterialContext materialContext;
materialContext.setIsSelected( selected );
materialContext.setSelectionColor( context.map().selectionColor() );
Qt3DRender::QMaterial *mat = mSymbol.material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Lines, materialContext );
Qt3DRender::QMaterial *mat = mSymbol->material()->toMaterial( QgsMaterialSettingsRenderingTechnique::Lines, materialContext );
if ( !mat )
{
QgsSimpleLineMaterialSettings defaultMaterial;
mat = defaultMaterial.toMaterial( QgsMaterialSettingsRenderingTechnique::Lines, materialContext );
}

if ( QgsLineMaterial *lineMaterial = dynamic_cast< QgsLineMaterial * >( mat ) )
lineMaterial->setLineWidth( mSymbol.width() );
lineMaterial->setLineWidth( mSymbol->width() );

Qt3DCore::QEntity *entity = new Qt3DCore::QEntity;

Expand Down Expand Up @@ -436,10 +439,10 @@ namespace Qgs3DSymbolImpl
return nullptr;

if ( lineSymbol->renderAsSimpleLines() )
return new QgsThickLine3DSymbolHandler( *lineSymbol, layer->selectedFeatureIds() );
return new QgsThickLine3DSymbolHandler( lineSymbol, layer->selectedFeatureIds() );
//return new QgsSimpleLine3DSymbolHandler( symbol, layer->selectedFeatureIds() );
else
return new QgsBufferedLine3DSymbolHandler( *lineSymbol, layer->selectedFeatureIds() );
return new QgsBufferedLine3DSymbolHandler( lineSymbol, layer->selectedFeatureIds() );
}

Qt3DCore::QEntity *entityForLine3DSymbol( const Qgs3DMapSettings &map, QgsVectorLayer *layer, const QgsLine3DSymbol &symbol )
Expand Down
Loading