Fix various issues with application of raster styles from QML-files #38083
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When applying a style file with a colorrampshader node but without a colorramp node, the actual color values of the items of the ramp are overwritten by the default ramp (currently 'Spectral'). Files with not colorramp node happened to be created by earlier QGIS versions - at least with 2.10. This is because the default ramp is applied after populating the color ramp item list. Applying a color ramp emits a colorRampChanged() signal. This signal causes new colors from the ramp applied to the items, overwriting the already set ones. This is not desired. Fix by populating the item list after applying the color ramp.
…derer When populating QgsSingleBandPseudoColorRenderer from an existing one the min/max values where set last. However if that renderers shader was a QgsColorRampShader without an actual color ramp, a default one was used. Now, if the color ramp shader contained a color map, we try to interpolate the colors with these min/max values, which are not set yet. So we set these values before loading any raster shader.
if mMin or mMax are not set yet for some reason, we should try our best to apply a reasonable color ramp.
m-kuhn
reviewed
Aug 9, 2020
|
Only one comment and it should be ready to merge. Thank you very much! |
thanks, will do! :) |
restore behavior of loadMinimumMaximumFromTree() to not change anything if the colormap contained no items.
m-kuhn
approved these changes
Aug 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes various issues with the application of styles from QML-files to raster layers. The general behavior is that a style is loaded from a QML with a colormap. Then this style seems to be show up OK in the canvas, but the actual color map assignment is sometimes wrong, and when you hit apply a wrong style is actually applied or the color scheme reverts back to a default one.
An example of this behavior (using ESACCI-LC data from http://maps.elie.ucl.ac.be/CCI/viewer/download.php)
This behavior is described in issues
Two major reasons for this behavior are probably the culprits:
applying a color ramp overwrites an already set colormap
When the QgsColorRampShaderwidget is populated by data from a QgsColorRampShader, the colormapTreeWidget is filled first (containing specific value->color assignments). After that it is checked if the shader has a colorramp and if it has none the default colorramp is applied. The problem is that this in the end emits colorRampChanged() which in turn repopulates the colorrampTreeWidget with new colors from the set colorramp (in this case the default one) overwriting the ones from the applied shader.
Fix: populate the colormap after the color ramp is applied. (7368e7f)
QgsColorRampShaderWidget::applyColorRamp() relies on valid mMin and mMax
A second problem is that the function QgsColorRampShaderWidget::applyColorRamp() assumes that the member variables mMin and mMax are already set when calculating new colors for the colormap. This is not necessarily the case as when a QgsSingleBandPseudoColorRendererWidget is populated by a QgsSingleBandPseudoColorRenderer the min/max form fields are set after the color ramp shader is applied. Then the colors for all the entries in the colormap are picked with a nan-value.
Fix:
This PR