Skip to content

Commit

Permalink
Address review
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Jul 26, 2024
1 parent 2a0d807 commit c0651bb
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 88 deletions.
33 changes: 14 additions & 19 deletions src/core/annotations/qgsannotationpictureitem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,7 @@ QgsAnnotationPictureItem::QgsAnnotationPictureItem( Qgis::PictureFormat format,
);
QgsSimpleLineSymbolLayer *borderSymbol = new QgsSimpleLineSymbolLayer( QColor( 0, 0, 0 ) );
borderSymbol->setPenJoinStyle( Qt::MiterJoin );
mBorderSymbol = std::make_unique< QgsFillSymbol >(
QgsSymbolLayerList
{
borderSymbol
}
);
mFrameSymbol = std::make_unique< QgsFillSymbol >( QgsSymbolLayerList{ borderSymbol } );
}

QgsAnnotationPictureItem::~QgsAnnotationPictureItem() = default;
Expand Down Expand Up @@ -185,11 +180,11 @@ void QgsAnnotationPictureItem::render( QgsRenderContext &context, QgsFeedback *
break;
}

if ( mDrawBorder && mBorderSymbol )
if ( mDrawFrame && mFrameSymbol )
{
mBorderSymbol->startRender( context );
mBorderSymbol->renderPolygon( painterBounds, nullptr, nullptr, context );
mBorderSymbol->stopRender( context );
mFrameSymbol->startRender( context );
mFrameSymbol->renderPolygon( painterBounds, nullptr, nullptr, context );
mFrameSymbol->stopRender( context );
}

}
Expand All @@ -216,11 +211,11 @@ bool QgsAnnotationPictureItem::writeXml( QDomElement &element, QDomDocument &doc
element.appendChild( backgroundElement );
}

element.setAttribute( QStringLiteral( "frameEnabled" ), mDrawBorder ? QStringLiteral( "1" ) : QStringLiteral( "0" ) );
if ( mBorderSymbol )
element.setAttribute( QStringLiteral( "frameEnabled" ), mDrawFrame ? QStringLiteral( "1" ) : QStringLiteral( "0" ) );
if ( mFrameSymbol )
{
QDomElement frameElement = document.createElement( QStringLiteral( "frameSymbol" ) );
frameElement.appendChild( QgsSymbolLayerUtils::saveSymbol( QStringLiteral( "frameSymbol" ), mBorderSymbol.get(), document, context ) );
frameElement.appendChild( QgsSymbolLayerUtils::saveSymbol( QStringLiteral( "frameSymbol" ), mFrameSymbol.get(), document, context ) );
element.appendChild( frameElement );
}

Expand Down Expand Up @@ -429,7 +424,7 @@ bool QgsAnnotationPictureItem::readXml( const QDomElement &element, const QgsRea
setBackgroundSymbol( QgsSymbolLayerUtils::loadSymbol< QgsFillSymbol >( backgroundSymbolElem, context ) );
}

mDrawBorder = element.attribute( QStringLiteral( "frameEnabled" ), QStringLiteral( "0" ) ).toInt();
mDrawFrame = element.attribute( QStringLiteral( "frameEnabled" ), QStringLiteral( "0" ) ).toInt();
const QDomElement frameSymbolElem = element.firstChildElement( QStringLiteral( "frameSymbol" ) ).firstChildElement();
if ( !frameSymbolElem.isNull() )
{
Expand All @@ -452,9 +447,9 @@ QgsAnnotationPictureItem *QgsAnnotationPictureItem::clone() const
if ( mBackgroundSymbol )
item->setBackgroundSymbol( mBackgroundSymbol->clone() );

item->setFrameEnabled( mDrawBorder );
if ( mBorderSymbol )
item->setFrameSymbol( mBorderSymbol->clone() );
item->setFrameEnabled( mDrawFrame );
if ( mFrameSymbol )
item->setFrameSymbol( mFrameSymbol->clone() );

item->copyCommonProperties( this );
return item.release();
Expand Down Expand Up @@ -551,12 +546,12 @@ void QgsAnnotationPictureItem::setBackgroundSymbol( QgsFillSymbol *symbol )

const QgsFillSymbol *QgsAnnotationPictureItem::frameSymbol() const
{
return mBorderSymbol.get();
return mFrameSymbol.get();
}

void QgsAnnotationPictureItem::setFrameSymbol( QgsFillSymbol *symbol )
{
mBorderSymbol.reset( symbol );
mFrameSymbol.reset( symbol );
}

QSizeF QgsAnnotationPictureItem::fixedSize() const
Expand Down
8 changes: 4 additions & 4 deletions src/core/annotations/qgsannotationpictureitem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ class CORE_EXPORT QgsAnnotationPictureItem : public QgsAnnotationItem
* \see setFrameEnabled()
* \see frameSymbol()
*/
bool frameEnabled() const { return mDrawBorder; }
bool frameEnabled() const { return mDrawFrame; }

/**
* Sets whether the item's frame should be rendered.
*
* \see frameEnabled()
* \see setFrameSymbol()
*/
void setFrameEnabled( bool enabled ) { mDrawBorder = enabled; }
void setFrameEnabled( bool enabled ) { mDrawFrame = enabled; }

/**
* Returns the symbol used to render the item's frame.
Expand Down Expand Up @@ -246,8 +246,8 @@ class CORE_EXPORT QgsAnnotationPictureItem : public QgsAnnotationItem
bool mLockAspectRatio = true;
bool mDrawBackground = false;
std::unique_ptr< QgsFillSymbol > mBackgroundSymbol;
bool mDrawBorder = false;
std::unique_ptr< QgsFillSymbol > mBorderSymbol;
bool mDrawFrame = false;
std::unique_ptr< QgsFillSymbol > mFrameSymbol;

#ifdef SIP_RUN
QgsAnnotationPictureItem( const QgsAnnotationPictureItem &other );
Expand Down
1 change: 0 additions & 1 deletion src/gui/annotations/qgsannotationitemguiregistry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "qgscreateannotationitemmaptool_impl.h"

#include <QImageReader>
#include <QFileDialog>

//
// QgsAnnotationItemAbstractGuiMetadata
Expand Down
22 changes: 11 additions & 11 deletions src/gui/annotations/qgsannotationitemwidget_impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,9 @@ QgsAnnotationPictureItemWidget::QgsAnnotationPictureItemWidget( QWidget *parent
mBackgroundSymbolButton->setSymbolType( Qgis::SymbolType::Fill );
mBackgroundSymbolButton->setDialogTitle( tr( "Background" ) );
mBackgroundSymbolButton->registerExpressionContextGenerator( this );
mBorderSymbolButton->setSymbolType( Qgis::SymbolType::Fill );
mBorderSymbolButton->setDialogTitle( tr( "Frame" ) );
mBorderSymbolButton->registerExpressionContextGenerator( this );
mFrameSymbolButton->setSymbolType( Qgis::SymbolType::Fill );
mFrameSymbolButton->setDialogTitle( tr( "Frame" ) );
mFrameSymbolButton->registerExpressionContextGenerator( this );

connect( mPropertiesWidget, &QgsAnnotationItemCommonPropertiesWidget::itemChanged, this, [ = ]
{
Expand All @@ -655,10 +655,10 @@ QgsAnnotationPictureItemWidget::QgsAnnotationPictureItemWidget( QWidget *parent
} );

connect( mLockAspectRatioCheck, &QCheckBox::toggled, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mBorderCheckbox, &QGroupBox::toggled, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mFrameCheckbox, &QGroupBox::toggled, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mBackgroundCheckbox, &QGroupBox::toggled, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mBackgroundSymbolButton, &QgsSymbolButton::changed, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mBorderSymbolButton, &QgsSymbolButton::changed, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mFrameSymbolButton, &QgsSymbolButton::changed, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );
connect( mSizeUnitWidget, &QgsUnitSelectionWidget::changed, this, &QgsAnnotationPictureItemWidget::onWidgetChanged );

connect( mWidthSpinBox, qOverload< double >( &QDoubleSpinBox::valueChanged ), this, &QgsAnnotationPictureItemWidget::setWidth );
Expand Down Expand Up @@ -699,9 +699,9 @@ void QgsAnnotationPictureItemWidget::updateItem( QgsAnnotationItem *item )
pictureItem->setFixedSizeUnit( mSizeUnitWidget->unit() );

pictureItem->setBackgroundEnabled( mBackgroundCheckbox->isChecked() );
pictureItem->setFrameEnabled( mBorderCheckbox->isChecked() );
pictureItem->setFrameEnabled( mFrameCheckbox->isChecked() );
pictureItem->setBackgroundSymbol( mBackgroundSymbolButton->clonedSymbol< QgsFillSymbol >() );
pictureItem->setFrameSymbol( mBorderSymbolButton->clonedSymbol< QgsFillSymbol >() );
pictureItem->setFrameSymbol( mFrameSymbolButton->clonedSymbol< QgsFillSymbol >() );

mPropertiesWidget->updateItem( pictureItem );
}
Expand All @@ -718,8 +718,8 @@ void QgsAnnotationPictureItemWidget::setContext( const QgsSymbolWidgetContext &c
mPropertiesWidget->setContext( context );
mBackgroundSymbolButton->setMapCanvas( context.mapCanvas() );
mBackgroundSymbolButton->setMessageBar( context.messageBar() );
mBorderSymbolButton->setMapCanvas( context.mapCanvas() );
mBorderSymbolButton->setMessageBar( context.messageBar() );
mFrameSymbolButton->setMapCanvas( context.mapCanvas() );
mFrameSymbolButton->setMessageBar( context.messageBar() );
}

QgsExpressionContext QgsAnnotationPictureItemWidget::createExpressionContext() const
Expand Down Expand Up @@ -768,9 +768,9 @@ bool QgsAnnotationPictureItemWidget::setNewItem( QgsAnnotationItem *item )
if ( const QgsSymbol *symbol = pictureItem->backgroundSymbol() )
mBackgroundSymbolButton->setSymbol( symbol->clone() );

mBorderCheckbox->setChecked( pictureItem->frameEnabled() );
mFrameCheckbox->setChecked( pictureItem->frameEnabled() );
if ( const QgsSymbol *symbol = pictureItem->frameSymbol() )
mBorderSymbolButton->setSymbol( symbol->clone() );
mFrameSymbolButton->setSymbol( symbol->clone() );

mWidthSpinBox->setValue( pictureItem->fixedSize().width() );
mHeightSpinBox->setValue( pictureItem->fixedSize().height() );
Expand Down
106 changes: 53 additions & 53 deletions src/ui/annotations/qgsannotationpicturewidgetbase.ui
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<item row="5" column="0">
<widget class="QGroupBox" name="mBackgroundCheckbox">
<property name="title">
<string>Draw background</string>
<string>Background</string>
</property>
<property name="checkable">
<bool>true</bool>
Expand Down Expand Up @@ -111,38 +111,6 @@
</property>
</spacer>
</item>
<item row="6" column="0">
<widget class="QGroupBox" name="mBorderCheckbox">
<property name="title">
<string>Draw border</string>
</property>
<property name="checkable">
<bool>true</bool>
</property>
<layout class="QGridLayout" name="gridLayout_3">
<item row="0" column="0">
<widget class="QLabel" name="label_2">
<property name="text">
<string>Symbol</string>
</property>
</widget>
</item>
<item row="0" column="1">
<widget class="QgsSymbolButton" name="mBorderSymbolButton">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="text">
<string>Change…</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
<item row="3" column="0">
<widget class="QgsStackedWidget" name="mSizeStackedWidget">
<property name="sizePolicy">
Expand Down Expand Up @@ -198,8 +166,15 @@
</property>
</widget>
</item>
<item row="1" column="1">
<widget class="QgsDoubleSpinBox" name="mHeightSpinBox">
<item row="2" column="0">
<widget class="QLabel" name="label_13">
<property name="text">
<string>Unit</string>
</property>
</widget>
</item>
<item row="0" column="1">
<widget class="QgsDoubleSpinBox" name="mWidthSpinBox">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
Expand All @@ -223,8 +198,8 @@
</property>
</widget>
</item>
<item row="0" column="1">
<widget class="QgsDoubleSpinBox" name="mWidthSpinBox">
<item row="1" column="1">
<widget class="QgsDoubleSpinBox" name="mHeightSpinBox">
<property name="sizePolicy">
<sizepolicy hsizetype="MinimumExpanding" vsizetype="Fixed">
<horstretch>0</horstretch>
Expand All @@ -248,6 +223,13 @@
</property>
</widget>
</item>
<item row="1" column="0">
<widget class="QLabel" name="label_11">
<property name="text">
<string>Height</string>
</property>
</widget>
</item>
<item row="0" column="2" rowspan="2">
<widget class="QgsRatioLockButton" name="mLockAspectRatio">
<property name="sizePolicy">
Expand All @@ -264,20 +246,6 @@
</property>
</widget>
</item>
<item row="1" column="0">
<widget class="QLabel" name="label_11">
<property name="text">
<string>Height</string>
</property>
</widget>
</item>
<item row="2" column="0">
<widget class="QLabel" name="label_13">
<property name="text">
<string>Unit</string>
</property>
</widget>
</item>
<item row="2" column="1">
<widget class="QgsUnitSelectionWidget" name="mSizeUnitWidget" native="true">
<property name="minimumSize">
Expand All @@ -302,6 +270,38 @@
</property>
</widget>
</item>
<item row="4" column="0">
<widget class="QGroupBox" name="mFrameCheckbox">
<property name="title">
<string>Frame</string>
</property>
<property name="checkable">
<bool>true</bool>
</property>
<layout class="QGridLayout" name="gridLayout_3">
<item row="0" column="0">
<widget class="QLabel" name="label_2">
<property name="text">
<string>Symbol</string>
</property>
</widget>
</item>
<item row="0" column="1">
<widget class="QgsSymbolButton" name="mFrameSymbolButton">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Fixed">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="text">
<string>Change…</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
</layout>
</widget>
<customwidgets>
Expand Down Expand Up @@ -356,10 +356,10 @@
<tabstop>mLockAspectRatio</tabstop>
<tabstop>mHeightSpinBox</tabstop>
<tabstop>mSizeUnitWidget</tabstop>
<tabstop>mFrameCheckbox</tabstop>
<tabstop>mFrameSymbolButton</tabstop>
<tabstop>mBackgroundCheckbox</tabstop>
<tabstop>mBackgroundSymbolButton</tabstop>
<tabstop>mBorderCheckbox</tabstop>
<tabstop>mBorderSymbolButton</tabstop>
</tabstops>
<resources/>
<connections/>
Expand Down

0 comments on commit c0651bb

Please sign in to comment.