Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Avoid duplicate colors in random paletted raster rendering
When selecting Render type "Paletted/Unique values" for a raster layer
that has 23 distinct raster values, QGIS will generate 23 random colors
in QgsRandomColorRamp, and then these colors will be queried through
QgsPalettedRasterRenderer::classDataFromRaster().

The querying of random colors goes through the generic
QgsColorRamp::color(v) interface, which takes a v between 0.0 and 1.0.

To turn a raster value index (0,1,...,22) into a value between 0.0 and 1.0,
a division by 22 is applied. QgsRandomColorRamp then converts this value
back into an index by multiplying by 22 and casting the result to an int.

Dividing an integer by 22 and multiplying the result by 22 and casting
to an int is not lossless:

	>>> int(15 / 22 * 22)
	14

This causes e.g. the 14th and 15th raster value (0-indexed) to always
have the same random color, regardless of how many times you press
"Shuffle Random Colors" on a raster with 23 distinct values.

Note that this issue is not exhibited at all for rasters with fewer than
23 distinct values, and certain cardinalities exhibit the issue much
more than others; e.g. for 50 distinct values, there are 6 duplicates!

Apply std::round() so that the correct index is obtained.
  • Loading branch information
Mathias Rav authored and github-actions[bot] committed Aug 24, 2021
1 parent d39d357 commit b3cef88
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/core/qgscolorramp.cpp
Expand Up @@ -440,7 +440,13 @@ QColor QgsRandomColorRamp::color( double value ) const
int maxVal = 255;

//if value is nan, then use last precalculated color
int colorIndex = ( !std::isnan( value ) ? value : 1 ) * ( mTotalColorCount - 1 );
if ( std::isnan( value ) )
{
value = 1.0;
}
// Caller has converted an index into a value in [0.0, 1.0]
// by doing "index / (mTotalColorCount - 1)"; retrieve the original index.
int colorIndex = std::round( value * ( mTotalColorCount - 1 ) );
if ( mTotalColorCount >= 1 && mPrecalculatedColors.length() > colorIndex )
{
//use precalculated hue
Expand Down

0 comments on commit b3cef88

Please sign in to comment.