Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #9839 from wonder-sk/fix-3d-extra-terrain-updates
[3d] Fix unnecessary terrain map updates when changing 3D renderer
  • Loading branch information
wonder-sk committed Apr 23, 2019
2 parents 74d30b9 + 60572ac commit 1106f6a
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 9 deletions.
9 changes: 9 additions & 0 deletions python/gui/auto_generated/qgsmaplayerconfigwidget.sip.in
Expand Up @@ -36,6 +36,15 @@ A panel widget that can be shown in the map style dock
Keep the loading light as possible for speed in the UI.
%End

virtual bool shouldTriggerLayerRepaint() const;
%Docstring
Whether this config widget changes map layer properties in a way that triggerRepaint() should
be called for the layer after applying changes. This is true by default, but some config widgets
(for example 3D rendering config) do not need layer repaint as they do not modify 2D map rendering.

.. versionadded:: 3.8
%End

public slots:

virtual void apply() = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/app/3d/qgsvectorlayer3drendererwidget.h
Expand Up @@ -66,6 +66,9 @@ class QgsVectorLayer3DRendererWidget : public QgsMapLayerConfigWidget

void setDockMode( bool dockMode ) override;

//! Only modifies 3D renderer so we do not want layer repaint (which would trigger unnecessary terrain map update)
bool shouldTriggerLayerRepaint() const override { return false; }

public slots:
void apply() override;

Expand Down
22 changes: 15 additions & 7 deletions src/app/qgslayerstylingwidget.cpp
Expand Up @@ -272,6 +272,7 @@ void QgsLayerStylingWidget::apply()
QWidget *current = mWidgetStack->mainPanel();

bool styleWasChanged = false;
bool triggerRepaint = false; // whether the change needs the layer to be repainted
if ( QgsLabelingWidget *widget = qobject_cast<QgsLabelingWidget *>( current ) )
{
widget->apply();
Expand All @@ -287,31 +288,34 @@ void QgsLayerStylingWidget::apply()
QgsRendererAbstractMetadata *m = QgsApplication::rendererRegistry()->rendererMetadata( layer->renderer()->type() );
undoName = QStringLiteral( "Style Change - %1" ).arg( m->visibleName() );
styleWasChanged = true;
triggerRepaint = true;
}
}
else if ( QgsRasterTransparencyWidget *widget = qobject_cast<QgsRasterTransparencyWidget *>( current ) )
{
widget->apply();
styleWasChanged = true;
triggerRepaint = true;
}
else if ( qobject_cast<QgsRasterHistogramWidget *>( current ) )
{
mRasterStyleWidget->apply();
styleWasChanged = true;
triggerRepaint = true;
}
else if ( QgsMapLayerConfigWidget *widget = qobject_cast<QgsMapLayerConfigWidget *>( current ) )
{
widget->apply();
styleWasChanged = true;
triggerRepaint = widget->shouldTriggerLayerRepaint();
}

pushUndoItem( undoName );
pushUndoItem( undoName, triggerRepaint );

if ( styleWasChanged )
{
emit styleChanged( mCurrentLayer );
QgsProject::instance()->setDirty( true );
mCurrentLayer->triggerRepaint();
}
connect( mCurrentLayer, &QgsMapLayer::styleChanged, this, &QgsLayerStylingWidget::updateCurrentWidgetLayer );
}
Expand Down Expand Up @@ -609,25 +613,26 @@ void QgsLayerStylingWidget::liveApplyToggled( bool value )
settings.setValue( QStringLiteral( "UI/autoApplyStyling" ), value );
}

void QgsLayerStylingWidget::pushUndoItem( const QString &name )
void QgsLayerStylingWidget::pushUndoItem( const QString &name, bool triggerRepaint )
{
QString errorMsg;
QDomDocument doc( QStringLiteral( "style" ) );
QDomElement rootNode = doc.createElement( QStringLiteral( "qgis" ) );
doc.appendChild( rootNode );
mCurrentLayer->writeStyle( rootNode, doc, errorMsg, QgsReadWriteContext() );
mCurrentLayer->undoStackStyles()->push( new QgsMapLayerStyleCommand( mCurrentLayer, name, rootNode, mLastStyleXml ) );
mCurrentLayer->undoStackStyles()->push( new QgsMapLayerStyleCommand( mCurrentLayer, name, rootNode, mLastStyleXml, triggerRepaint ) );
// Override the last style on the stack
mLastStyleXml = rootNode.cloneNode();
}


QgsMapLayerStyleCommand::QgsMapLayerStyleCommand( QgsMapLayer *layer, const QString &text, const QDomNode &current, const QDomNode &last )
QgsMapLayerStyleCommand::QgsMapLayerStyleCommand( QgsMapLayer *layer, const QString &text, const QDomNode &current, const QDomNode &last, bool triggerRepaint )
: QUndoCommand( text )
, mLayer( layer )
, mXml( current )
, mLastState( last )
, mTime( QTime::currentTime() )
, mTriggerRepaint( triggerRepaint )
{
}

Expand All @@ -636,15 +641,17 @@ void QgsMapLayerStyleCommand::undo()
QString error;
QgsReadWriteContext context = QgsReadWriteContext();
mLayer->readStyle( mLastState, error, context );
mLayer->triggerRepaint();
if ( mTriggerRepaint )
mLayer->triggerRepaint();
}

void QgsMapLayerStyleCommand::redo()
{
QString error;
QgsReadWriteContext context = QgsReadWriteContext();
mLayer->readStyle( mXml, error, context );
mLayer->triggerRepaint();
if ( mTriggerRepaint )
mLayer->triggerRepaint();
}

bool QgsMapLayerStyleCommand::mergeWith( const QUndoCommand *other )
Expand All @@ -665,6 +672,7 @@ bool QgsMapLayerStyleCommand::mergeWith( const QUndoCommand *other )

mXml = otherCmd->mXml;
mTime = otherCmd->mTime;
mTriggerRepaint |= otherCmd->mTriggerRepaint;
return true;
}

Expand Down
5 changes: 3 additions & 2 deletions src/app/qgslayerstylingwidget.h
Expand Up @@ -56,7 +56,7 @@ class APP_EXPORT QgsLayerStyleManagerWidgetFactory : public QgsMapLayerConfigWid
class APP_EXPORT QgsMapLayerStyleCommand : public QUndoCommand
{
public:
QgsMapLayerStyleCommand( QgsMapLayer *layer, const QString &text, const QDomNode &current, const QDomNode &last );
QgsMapLayerStyleCommand( QgsMapLayer *layer, const QString &text, const QDomNode &current, const QDomNode &last, bool triggerRepaint = true );

/**
* Returns unique ID for this kind of undo command.
Expand All @@ -75,6 +75,7 @@ class APP_EXPORT QgsMapLayerStyleCommand : public QUndoCommand
QDomNode mXml;
QDomNode mLastState;
QTime mTime;
bool mTriggerRepaint = true;
};

class APP_EXPORT QgsLayerStylingWidget : public QWidget, private Ui::QgsLayerStylingWidgetBase
Expand Down Expand Up @@ -129,7 +130,7 @@ class APP_EXPORT QgsLayerStylingWidget : public QWidget, private Ui::QgsLayerSty
void liveApplyToggled( bool value );

private:
void pushUndoItem( const QString &name );
void pushUndoItem( const QString &name, bool triggerRepaint = true );
int mNotSupportedPage;
int mLayerPage;
QTimer *mAutoApplyTimer = nullptr;
Expand Down
8 changes: 8 additions & 0 deletions src/gui/qgsmaplayerconfigwidget.h
Expand Up @@ -45,6 +45,14 @@ class GUI_EXPORT QgsMapLayerConfigWidget : public QgsPanelWidget
*/
QgsMapLayerConfigWidget( QgsMapLayer *layer, QgsMapCanvas *canvas, QWidget *parent = nullptr );

/**
* Whether this config widget changes map layer properties in a way that triggerRepaint() should
* be called for the layer after applying changes. This is true by default, but some config widgets
* (for example 3D rendering config) do not need layer repaint as they do not modify 2D map rendering.
* \since QGIS 3.8
*/
virtual bool shouldTriggerLayerRepaint() const { return true; }

public slots:

/**
Expand Down

0 comments on commit 1106f6a

Please sign in to comment.