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

Use weak layer pointers in labeling engine #4095

Merged
merged 2 commits into from
Feb 4, 2017

Conversation

nyalldawson
Copy link
Collaborator

This flips the labeling engine/providers to use weak map layer pointers instead of layer ids. It removes an existing use of the project instance (and avoids another one I was about to add!)

I've also added some typedefs for QPointer QgsMapLayer* - I got sick of writing these out in full!

@@ -760,7 +760,7 @@ QgsExpressionContextScope* QgsExpressionContextUtils::layerScope( const QgsMapLa

scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "layer_name" ), layer->name(), true ) );
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "layer_id" ), layer->id(), true ) );
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "layer" ), QVariant::fromValue<QPointer<QgsMapLayer> >( QPointer<QgsMapLayer>( const_cast<QgsMapLayer*>( layer ) ) ), true ) );
scope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "layer" ), QVariant::fromValue<QgsWeakMapLayerPointer >( QgsWeakMapLayerPointer( const_cast<QgsMapLayer*>( layer ) ) ), true ) );
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should remove the const from this method instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm... no strong opinions either way. Ideally we'd store a const pointer here I guess, but that doesn't seem possible

@m-kuhn
Copy link
Member

m-kuhn commented Feb 3, 2017

The concept looks good to me

Copy link
Member

@wonder-sk wonder-sk left a comment

Choose a reason for hiding this comment

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

Hi Nyall - the changes look good to me.

By the way, for classes that may be written to / read from XML (e.g. QgsAnnotation) we may need to use the helper QgsMapLayerRef structure, to allow delayed resolution from layer ID to pointer - but that's again for some future de-qgsproject::instance-ization work :-)

@nyalldawson nyalldawson merged commit a0cb645 into qgis:master Feb 4, 2017
@nyalldawson nyalldawson deleted the label_engine_layers branch February 4, 2017 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants