Skip to content

Commit

Permalink
Raster layer properties: fix renderer widget that was reset, and fix …
Browse files Browse the repository at this point in the history 
…wrong behaviour on cancel.

2 issues :
- when opening the raster layer properties dialog, it used to reset the min/max value
  to custom values, due to a bad interaction with a recent change in the histogram code
  (likely introduced in 4f3cf68)
- when closing the raster layer properties dialog on a multiband renderer in updated
  extent mode for example, it got result to whole raster statistics due to
  QgsRasterLayerProperties::setRendererWidget() reseting stuff. Honestly the code in
  that method that changes value in the renderer object seems to be completely
  inappropriate for a method that you would expect to only affects GUI/widgets.
  • Loading branch information
@rouault
rouault committed Mar 29, 2017
1 parent ca5f99d commit ef26d95
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
26 changes: 10 additions & 16 deletions src/app/qgsrasterlayerproperties.cpp
View file
Expand Up @@ -370,6 +370,7 @@ QgsRasterLayerProperties::QgsRasterLayerProperties( QgsMapLayer *lyr, QgsMapCanv

//fill available renderers into combo box
QgsRasterRendererRegistryEntry entry;
mDisableRenderTypeComboBoxCurrentIndexChanged = true;
Q_FOREACH ( const QString &name, QgsApplication::rasterRendererRegistry()->renderersList() )
{
if ( QgsApplication::rasterRendererRegistry()->rendererData( name, entry ) )
Expand All @@ -381,28 +382,20 @@ QgsRasterLayerProperties::QgsRasterLayerProperties( QgsMapLayer *lyr, QgsMapCanv
}
}
}
mDisableRenderTypeComboBoxCurrentIndexChanged = false;

int widgetIndex = 0;
if ( renderer )
{
QString rendererType = renderer->type();
int widgetIndex = mRenderTypeComboBox->findData( rendererType );
widgetIndex = mRenderTypeComboBox->findData( rendererType );
if ( widgetIndex != -1 )
{
mDisableRenderTypeComboBoxCurrentIndexChanged = true;
mRenderTypeComboBox->setCurrentIndex( widgetIndex );
mDisableRenderTypeComboBoxCurrentIndexChanged = false;
}

//prevent change between singleband color renderer and the other renderers
// No more necessary, combo entries according to layer type
#if 0
if ( rendererType == "singlebandcolordata" )
{
mRenderTypeComboBox->setEnabled( false );
}
else
{
mRenderTypeComboBox->removeItem( mRenderTypeComboBox->findData( "singlebandcolordata" ) );
}
#endif
if ( rendererType == QLatin1String( "singlebandcolordata" ) && mRenderTypeComboBox->count() == 1 )
{
// no band rendering options for singlebandcolordata, so minimize group box
Expand All @@ -413,7 +406,8 @@ QgsRasterLayerProperties::QgsRasterLayerProperties( QgsMapLayer *lyr, QgsMapCanv
mBandRenderingGrpBx->updateGeometry();
}
}
on_mRenderTypeComboBox_currentIndexChanged( mRenderTypeComboBox->currentIndex() );

on_mRenderTypeComboBox_currentIndexChanged( widgetIndex );

// update based on lyr's current state
sync();
Expand Down Expand Up @@ -560,7 +554,7 @@ void QgsRasterLayerProperties::setRendererWidget( const QString &rendererName )
QgsDebugMsg( "renderer has widgetCreateFunction" );
// Current canvas extent (used to calc min/max) in layer CRS
QgsRectangle myExtent = mMapCanvas->mapSettings().outputExtentToLayerExtent( mRasterLayer, mMapCanvas->extent() );
if ( oldWidget )
if ( oldWidget && ( !oldRenderer || rendererName != oldRenderer->type() ) )
{
if ( rendererName == "singlebandgray" )
{
Expand Down Expand Up @@ -1138,7 +1132,7 @@ void QgsRasterLayerProperties::on_buttonBuildPyramids_clicked()

void QgsRasterLayerProperties::on_mRenderTypeComboBox_currentIndexChanged( int index )
{
if ( index < 0 )
if ( index < 0 || mDisableRenderTypeComboBoxCurrentIndexChanged )
{
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/app/qgsrasterlayerproperties.h
View file
Expand Up @@ -194,5 +194,7 @@ class APP_EXPORT QgsRasterLayerProperties : public QgsOptionsDialogBase, private
/** Previous layer style. Used to reset style to previous state if new style
* was loaded but dialog is canceled */
QgsMapLayerStyle mOldStyle;

bool mDisableRenderTypeComboBoxCurrentIndexChanged = false;
};
#endif
4 changes: 2 additions & 2 deletions src/gui/raster/qgsrasterhistogramwidget.cpp
View file
Expand Up @@ -963,7 +963,7 @@ void QgsRasterHistogramWidget::applyHistoMin()
if ( bandNo == mRendererWidget->selectedBand( i ) )
{
min = leHistoMin->text();
if ( mHistoUpdateStyleToMinMax )
if ( mHistoUpdateStyleToMinMax && mRendererWidget->min( i ) != min )
{
mRendererWidget->setMin( min, i );
if ( mRendererWidget->minMaxWidget() )
Expand Down Expand Up @@ -998,7 +998,7 @@ void QgsRasterHistogramWidget::applyHistoMax()
if ( bandNo == mRendererWidget->selectedBand( i ) )
{
max = leHistoMax->text();
if ( mHistoUpdateStyleToMinMax )
if ( mHistoUpdateStyleToMinMax && mRendererWidget->max( i ) != max )
{
mRendererWidget->setMax( max, i );
if ( mRendererWidget->minMaxWidget() )
Expand Down

1 comment on commit ef26d95

@nirvn
Copy link
Contributor

@nirvn nirvn commented on ef26d95 Mar 30, 2017

Choose a reason for hiding this comment

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

@rouault , thanks, this effectively made it possible to open a raster layer properties window without messing with the renderer's settings again.

While I have you on the line, I'll highlight another messy situation with raster & min/max computation: when the min/max is computed through "current canvas", subsequently panning around and then changing the raster's brightness / saturation / contrast / resampling mode / etc. through the style dock will trigger an (unwanted) re-calculation of the min/max using a new canvas extent.

Effectively what seems to happen here is that changing any of the above-mentioned values recreates a renderer widget, and triggers a min/max computation.

Please sign in to comment.