QgsRasterFileWriter::writeDataRaster(): do not set nodata on output when unneeded

rouault · rouault · commit e0d38ba3f912 · 2016-05-28T10:52:36.000+02:00
Currently if the source raster has no nodata value, the writer will still write
nan as the output nodata value, ignoring the flag that specifies if there is a
nodata value or not. On a raster with byte data type, this will cause
confusion on reading since the nodata value will be somehow cast as 0.

Make QgsRasterChecker check for nodata consistency between source and target, and
add a test file that shows the issue.
diff --git a/src/core/raster/qgsrasterchecker.cpp b/src/core/raster/qgsrasterchecker.cpp
@@ -95,8 +95,13 @@ bool QgsRasterChecker::runTest( const QString& theVerifiedKey, QString theVerifi
     compare( "Source data type", verifiedProvider->srcDataType( band ), expectedProvider->srcDataType( band ), mReport, typesOk );
     compare( "Data type", verifiedProvider->dataType( band ), expectedProvider->dataType( band ), mReport, typesOk );
 
-    // TODO: not yet sure if noDataValue() should exist at all
-    //compare( "No data (NULL) value", verifiedProvider->noDataValue( band ), expectedProvider->noDataValue( band ), mReport, typesOk );
+    // Check nodata
+    bool noDataOk = true;
+    compare( "No data (NULL) value existence flag", verifiedProvider->srcHasNoDataValue( band ), expectedProvider->srcHasNoDataValue( band ), mReport, noDataOk );
+    if ( verifiedProvider->srcHasNoDataValue( band ) && expectedProvider->srcHasNoDataValue( band ) )
+    {
+      compare( "No data (NULL) value", verifiedProvider->srcNoDataValue( band ), expectedProvider->srcNoDataValue( band ), mReport, noDataOk );
+    }
 
     bool statsOk = true;
     QgsRasterBandStats verifiedStats =  verifiedProvider->bandStatistics( band );
@@ -122,7 +127,7 @@ bool QgsRasterChecker::runTest( const QString& theVerifiedKey, QString theVerifi
     mReport += "</table>";
     mReport += "<br>";
 
-    if ( !statsOk || !typesOk )
+    if ( !statsOk || !typesOk || !noDataOk )
     {
       allOk = false;
       // create values table anyway so that values are available
diff --git a/src/core/raster/qgsrasterfilewriter.cpp b/src/core/raster/qgsrasterfilewriter.cpp
@@ -342,7 +342,7 @@ QgsRasterFileWriter::WriterError QgsRasterFileWriter::writeDataRaster(
   {
     iter->startRasterRead( i, nCols, nRows, outputExtent );
     blockList.push_back( nullptr );
-    if ( destProvider ) // no tiles
+    if ( destProvider && destHasNoDataValueList.value( i - 1 ) ) // no tiles
     {
       destProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );
     }
@@ -436,7 +436,10 @@ QgsRasterFileWriter::WriterError QgsRasterFileWriter::writeDataRaster(
         //write data to output file. todo: loop over the data list
         for ( int i = 1; i <= nBands; ++i )
         {
-          partDestProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );
+          if ( destHasNoDataValueList.value( i - 1 ) )
+          {
+            partDestProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );
+          }
           partDestProvider->write( destBlockList[i - 1]->bits( 0 ), i, iterCols, iterRows, 0, 0 );
           delete destBlockList[i - 1];
           addToVRT( partFileName( fileIndex ), i, iterCols, iterRows, iterLeft, iterTop );
diff --git a/tests/testdata/raster/byte.tif b/tests/testdata/raster/byte.tif

Original file line number	Diff line number	Diff line change
`@@ -342,7 +342,7 @@ QgsRasterFileWriter::WriterError QgsRasterFileWriter::writeDataRaster(`
`342`	`342`	`{`
`343`	`343`	`iter->startRasterRead( i, nCols, nRows, outputExtent );`
`344`	`344`	`blockList.push_back( nullptr );`
`345`		`- if ( destProvider ) // no tiles`
	`345`	`+ if ( destProvider && destHasNoDataValueList.value( i - 1 ) ) // no tiles`
`346`	`346`	`{`
`347`	`347`	`destProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );`
`348`	`348`	`}`
`@@ -436,7 +436,10 @@ QgsRasterFileWriter::WriterError QgsRasterFileWriter::writeDataRaster(`
`436`	`436`	`//write data to output file. todo: loop over the data list`
`437`	`437`	`for ( int i = 1; i <= nBands; ++i )`
`438`	`438`	`{`
`439`		`- partDestProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );`
	`439`	`+ if ( destHasNoDataValueList.value( i - 1 ) )`
	`440`	`+ {`
	`441`	`+ partDestProvider->setNoDataValue( i, destNoDataValueList.value( i - 1 ) );`
	`442`	`+ }`
`440`	`443`	`partDestProvider->write( destBlockList[i - 1]->bits( 0 ), i, iterCols, iterRows, 0, 0 );`
`441`	`444`	`delete destBlockList[i - 1];`
`442`	`445`	`addToVRT( partFileName( fileIndex ), i, iterCols, iterRows, iterLeft, iterTop );`