Skip to content

Commit

Permalink
Rework annotation layer index handling
Browse files Browse the repository at this point in the history
Because some annotation items have scale dependent bounding boxes,
we can only index items which have a fixed bounding box. Other
item bounds need to be dynamically determined based on a specific
render context. So rework the annotation layer index handling
to only index appropriate items and store other items in a non-indexed
item set.

This isn't ideal, because it means we need to clone ALL non-indexed
items upfront whenever we render an annotation layer (it's too expensive
to calculate their actual bounding box and selectively clone them,
as it's a process which blocks the main thread).

Hopefully we can think of an alternative approach to this down
the line so that we DO have some form of spatial index for scale
dependent items, unlocking better performance for annotation layers
with 10,000s of items. But we'll ignore that situation for now ;)
  • Loading branch information
nyalldawson committed Aug 31, 2021
1 parent b4acffb commit 47bcf09
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 15 deletions.
Expand Up @@ -73,6 +73,11 @@ Returns a unique (untranslated) string identifying the type of item.
virtual QgsRectangle boundingBox() const = 0;
%Docstring
Returns the bounding box of the item's geographic location, in the parent layer's coordinate reference system.
%End

virtual QgsRectangle boundingBox( const QgsRenderContext &context ) const;
%Docstring
Returns the bounding box of the item's geographic location, in the parent layer's coordinate reference system.
%End

virtual void render( QgsRenderContext &context, QgsFeedback *feedback ) = 0;
Expand Down
Expand Up @@ -100,9 +100,10 @@ Returns the item with the specified ``id``, or ``None`` if no matching item was
.. versionadded:: 3.22
%End

QStringList itemsInBounds( const QgsRectangle &bounds, QgsFeedback *feedback = 0 ) const;
QStringList itemsInBounds( const QgsRectangle &bounds, const QgsRenderContext &context, QgsFeedback *feedback = 0 ) const;
%Docstring
Returns a list of the IDs of all annotation items within the specified ``bounds``.
Returns a list of the IDs of all annotation items within the specified ``bounds`` (in layer CRS), when
rendered using the given render ``context``.

The optional ``feedback`` argument can be used to cancel the search early.

Expand Down
Expand Up @@ -29,6 +29,8 @@ Constructor for QgsAnnotationPointTextItem, containing the specified ``text`` at
%End
~QgsAnnotationPointTextItem();

virtual Qgis::AnnotationItemFlags flags() const;

virtual QString type() const;

virtual void render( QgsRenderContext &context, QgsFeedback *feedback );
Expand All @@ -47,6 +49,8 @@ Creates a new text at point annotation item.

virtual QgsRectangle boundingBox() const;

virtual QgsRectangle boundingBox( const QgsRenderContext &context ) const;


QgsPointXY point() const;
%Docstring
Expand Down
5 changes: 5 additions & 0 deletions src/core/annotations/qgsannotationitem.h
Expand Up @@ -101,6 +101,11 @@ class CORE_EXPORT QgsAnnotationItem
*/
virtual QgsRectangle boundingBox() const = 0;

/**
* Returns the bounding box of the item's geographic location, in the parent layer's coordinate reference system.
*/
virtual QgsRectangle boundingBox( const QgsRenderContext &context ) const { Q_UNUSED( context ) return boundingBox();}

/**
* Renders the item to the specified render \a context.
*
Expand Down
45 changes: 40 additions & 5 deletions src/core/annotations/qgsannotationlayer.cpp
Expand Up @@ -123,7 +123,10 @@ QString QgsAnnotationLayer::addItem( QgsAnnotationItem *item )
{
const QString uuid = QUuid::createUuid().toString();
mItems.insert( uuid, item );
mSpatialIndex->insert( uuid, item->boundingBox() );
if ( item->flags() & Qgis::AnnotationItemFlag::ScaleDependentBoundingBox )
mNonIndexedItems.insert( uuid );
else
mSpatialIndex->insert( uuid, item->boundingBox() );

triggerRepaint();

Expand All @@ -136,7 +139,17 @@ bool QgsAnnotationLayer::removeItem( const QString &id )
return false;

std::unique_ptr< QgsAnnotationItem> item( mItems.take( id ) );
mSpatialIndex->remove( id, item->boundingBox() );

auto it = mNonIndexedItems.find( id );
if ( it == mNonIndexedItems.end() )
{
mSpatialIndex->remove( id, item->boundingBox() );
}
else
{
mNonIndexedItems.erase( it );
}

item.reset();

triggerRepaint();
Expand All @@ -149,6 +162,7 @@ void QgsAnnotationLayer::clear()
qDeleteAll( mItems );
mItems.clear();
mSpatialIndex = std::make_unique< QgsAnnotationLayerSpatialIndex >();
mNonIndexedItems.clear();

triggerRepaint();
}
Expand All @@ -163,7 +177,8 @@ QgsAnnotationItem *QgsAnnotationLayer::item( const QString &id )
return mItems.value( id );
}

QStringList QgsAnnotationLayer::itemsInBounds( const QgsRectangle &bounds, QgsFeedback *feedback ) const

QStringList QgsAnnotationLayer::queryIndex( const QgsRectangle &bounds, QgsFeedback *feedback ) const
{
QStringList res;

Expand All @@ -175,6 +190,19 @@ QStringList QgsAnnotationLayer::itemsInBounds( const QgsRectangle &bounds, QgsFe
return res;
}

QStringList QgsAnnotationLayer::itemsInBounds( const QgsRectangle &bounds, const QgsRenderContext &context, QgsFeedback *feedback ) const
{
QStringList res = queryIndex( bounds, feedback );
// we also have to search through any non-indexed items
for ( const QString &uuid : mNonIndexedItems )
{
if ( mItems.value( uuid )->boundingBox( context ).intersects( bounds ) )
res << uuid;
}

return res;
}

Qgis::MapLayerProperties QgsAnnotationLayer::properties() const
{
// annotation layers are always editable
Expand All @@ -190,7 +218,10 @@ QgsAnnotationLayer *QgsAnnotationLayer::clone() const
for ( auto it = mItems.constBegin(); it != mItems.constEnd(); ++it )
{
layer->mItems.insert( it.key(), ( *it )->clone() );
layer->mSpatialIndex->insert( it.key(), ( *it )->boundingBox() );
if ( ( *it )->flags() & Qgis::AnnotationItemFlag::ScaleDependentBoundingBox )
layer->mNonIndexedItems.insert( it.key() );
else
layer->mSpatialIndex->insert( it.key(), ( *it )->boundingBox() );
}

return layer.release();
Expand Down Expand Up @@ -234,6 +265,7 @@ bool QgsAnnotationLayer::readXml( const QDomNode &layerNode, QgsReadWriteContext
qDeleteAll( mItems );
mItems.clear();
mSpatialIndex = std::make_unique< QgsAnnotationLayerSpatialIndex >();
mNonIndexedItems.clear();

const QDomNodeList itemsElements = layerNode.toElement().elementsByTagName( QStringLiteral( "items" ) );
if ( itemsElements.size() == 0 )
Expand All @@ -249,7 +281,10 @@ bool QgsAnnotationLayer::readXml( const QDomNode &layerNode, QgsReadWriteContext
if ( item )
{
item->readXml( itemElement, context );
mSpatialIndex->insert( id, item->boundingBox() );
if ( item->flags() & Qgis::AnnotationItemFlag::ScaleDependentBoundingBox )
mNonIndexedItems.insert( id );
else
mSpatialIndex->insert( id, item->boundingBox() );
mItems.insert( id, item.release() );
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/core/annotations/qgsannotationlayer.h
Expand Up @@ -128,13 +128,14 @@ class CORE_EXPORT QgsAnnotationLayer : public QgsMapLayer
QgsAnnotationItem *item( const QString &id );

/**
* Returns a list of the IDs of all annotation items within the specified \a bounds.
* Returns a list of the IDs of all annotation items within the specified \a bounds (in layer CRS), when
* rendered using the given render \a context.
*
* The optional \a feedback argument can be used to cancel the search early.
*
* \since QGIS 3.22
*/
QStringList itemsInBounds( const QgsRectangle &bounds, QgsFeedback *feedback = nullptr ) const;
QStringList itemsInBounds( const QgsRectangle &bounds, const QgsRenderContext &context, QgsFeedback *feedback = nullptr ) const;

Qgis::MapLayerProperties properties() const override;
QgsAnnotationLayer *clone() const override SIP_FACTORY;
Expand All @@ -153,6 +154,11 @@ class CORE_EXPORT QgsAnnotationLayer : public QgsMapLayer
QgsCoordinateTransformContext mTransformContext;

std::unique_ptr< QgsAnnotationLayerSpatialIndex > mSpatialIndex;
QSet< QString > mNonIndexedItems;

QStringList queryIndex( const QgsRectangle &bounds, QgsFeedback *feedback = nullptr ) const;

friend class QgsAnnotationLayerRenderer;

};

Expand Down
16 changes: 14 additions & 2 deletions src/core/annotations/qgsannotationlayerrenderer.cpp
Expand Up @@ -23,8 +23,20 @@ QgsAnnotationLayerRenderer::QgsAnnotationLayerRenderer( QgsAnnotationLayer *laye
, mFeedback( std::make_unique< QgsFeedback >() )
, mLayerOpacity( layer->opacity() )
{
// clone items from layer which fall inside the rendered extent
const QStringList items = layer->itemsInBounds( context.extent() );
// Clone items from layer which fall inside the rendered extent
// Because some items have scale dependent bounds, we have to accept some limitations here.
// first, we can use the layer's spatial index to very quickly retrieve items we know will fall within the visible
// extent. This will ONLY apply to items which have a non-scale-dependent bounding box though.

QSet< QString > items = qgis::listToSet( layer->queryIndex( context.extent() ) );

// we also have NO choice but to clone ALL non-indexed items (i.e. those with a scale-dependent bounding box)
// since these won't be in the layer's spatial index, and it's too expensive to determine their actual bounding box
// upfront (we are blocking the main thread right now!)

// TODO -- come up with some brilliant way to avoid this and also index scale-dependent items ;)
items.unite( layer->mNonIndexedItems );

mItems.reserve( items.size() );
std::transform( items.begin(), items.end(), std::back_inserter( mItems ),
[layer]( const QString & id ) -> QgsAnnotationItem* { return layer->item( id )->clone(); } );
Expand Down
12 changes: 12 additions & 0 deletions src/core/annotations/qgsannotationpointtextitem.cpp
Expand Up @@ -28,6 +28,7 @@ QgsAnnotationPointTextItem::QgsAnnotationPointTextItem( const QString &text, Qgs

Qgis::AnnotationItemFlags QgsAnnotationPointTextItem::flags() const
{
// in truth this should depend on whether the text format is scale dependent or not!
return Qgis::AnnotationItemFlag::ScaleDependentBoundingBox;
}

Expand Down Expand Up @@ -116,6 +117,17 @@ QgsRectangle QgsAnnotationPointTextItem::boundingBox() const
return QgsRectangle( mPoint.x(), mPoint.y(), mPoint.x(), mPoint.y() );
}

QgsRectangle QgsAnnotationPointTextItem::boundingBox( const QgsRenderContext &context ) const
{
const double widthInPixels = QgsTextRenderer::textWidth( context, mTextFormat, mText.split( '\n' ) );
const double heightInPixels = QgsTextRenderer::textHeight( context, mTextFormat, mText.split( '\n' ) );

const double widthInMapUnits = context.convertToMapUnits( widthInPixels, QgsUnitTypes::RenderPixels );
const double heightInMapUnits = context.convertToMapUnits( heightInPixels, QgsUnitTypes::RenderPixels );

return QgsRectangle( mPoint.x(), mPoint.y(), mPoint.x() + widthInMapUnits, mPoint.y() + heightInMapUnits );
}

QgsTextFormat QgsAnnotationPointTextItem::format() const
{
return mTextFormat;
Expand Down
2 changes: 2 additions & 0 deletions src/core/annotations/qgsannotationpointtextitem.h
Expand Up @@ -40,6 +40,7 @@ class CORE_EXPORT QgsAnnotationPointTextItem : public QgsAnnotationItem
QgsAnnotationPointTextItem( const QString &text, QgsPointXY point );
~QgsAnnotationPointTextItem() override;

Qgis::AnnotationItemFlags flags() const override;
QString type() const override;
void render( QgsRenderContext &context, QgsFeedback *feedback ) override;
bool writeXml( QDomElement &element, QDomDocument &document, const QgsReadWriteContext &context ) const override;
Expand All @@ -52,6 +53,7 @@ class CORE_EXPORT QgsAnnotationPointTextItem : public QgsAnnotationItem
bool readXml( const QDomElement &element, const QgsReadWriteContext &context ) override;
QgsAnnotationPointTextItem *clone() override SIP_FACTORY;
QgsRectangle boundingBox() const override;
QgsRectangle boundingBox( const QgsRenderContext &context ) const override;

/**
* Returns the point location of the text.
Expand Down
9 changes: 5 additions & 4 deletions tests/src/python/test_qgsannotationlayer.py
Expand Up @@ -159,10 +159,11 @@ def testItemsInBounds(self):
QgsAnnotationLineItem(QgsLineString([QgsPoint(11, 13), QgsPoint(12, 13), QgsPoint(12, 150)])))
item3uuid = layer.addItem(QgsAnnotationMarkerItem(QgsPoint(120, 13)))

self.assertFalse(layer.itemsInBounds(QgsRectangle(-10, -10, -9, 9)))
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(12, 13, 14, 15)), [item1uuid, item2uuid])
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(12, 130, 14, 150)), [item2uuid])
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(110, 0, 120, 20)), [item3uuid])
rc = QgsRenderContext()
self.assertFalse(layer.itemsInBounds(QgsRectangle(-10, -10, -9, 9), rc))
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(12, 13, 14, 15), rc), [item1uuid, item2uuid])
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(12, 130, 14, 150), rc), [item2uuid])
self.assertCountEqual(layer.itemsInBounds(QgsRectangle(110, 0, 120, 20), rc), [item3uuid])

def testReadWriteXml(self):
doc = QDomDocument("testdoc")
Expand Down

0 comments on commit 47bcf09

Please sign in to comment.