[Raster] Add a Qgis::DataType::Int8 signed data type

[rouault]

Continuation of #51586

Probably 3.32 material at that point?

[nyalldawson]

Probably 3.32 material at that point

Is there anything in particular you're worried about here? From a look over the changes it's all additions to handling int8 only, so even buggy support for that is better then the current no support IMO.

[rouault]

Is there anything in particular you're worried about here?

no, not really. That could be fine in 3.30 as well.

[nyalldawson]

Let's get this in 3.30 👍

[rouault]

It might still be useful to merge #51586 and have it backported to 3.28 so that the next LTR gets a minimal GDAL support for Int8 rasters when/if GDAL 3.7 is used at some point with QGIS 3.28

[nyalldawson]

@rouault good point!

[agiudiceandrea]

@rouault souldn't the Int8 / GDT_Int8 value be added to the QgsGdalUtils::gdalDataTypeFromQgisDataType function?

QGIS/src/core/qgsgdalutils.cpp

Lines 453 to 497 in 0b59017

	GDALDataType QgsGdalUtils::gdalDataTypeFromQgisDataType( Qgis::DataType dataType )
	{
	switch ( dataType )
	{
	case Qgis::DataType::UnknownDataType:
	return GDALDataType::GDT_Unknown;
	break;
	case Qgis::DataType::Byte:
	return GDALDataType::GDT_Byte;
	break;
	case Qgis::DataType::UInt16:
	return GDALDataType::GDT_UInt16;
	break;
	case Qgis::DataType::Int16:
	return GDALDataType::GDT_Int16;
	break;
	case Qgis::DataType::UInt32:
	return GDALDataType::GDT_UInt32;
	break;
	case Qgis::DataType::Int32:
	return GDALDataType::GDT_Int32;
	break;
	case Qgis::DataType::Float32:
	return GDALDataType::GDT_Float32;
	break;
	case Qgis::DataType::Float64:
	return GDALDataType::GDT_Float64;
	break;
	case Qgis::DataType::CInt16:
	return GDALDataType::GDT_CInt16;
	break;
	case Qgis::DataType::CInt32:
	return GDALDataType::GDT_CInt32;
	break;
	case Qgis::DataType::CFloat32:
	return GDALDataType::GDT_CFloat32;
	break;
	case Qgis::DataType::CFloat64:
	return GDALDataType::GDT_CFloat64;
	break;
	case Qgis::DataType::ARGB32:
	case Qgis::DataType::ARGB32_Premultiplied:
	return GDALDataType::GDT_Unknown;
	break;
	};

[rouault]

souldn't the Int8 / GDT_Int8 value be added to the QgsGdalUtils::gdalDataTypeFromQgisDataType function?

good catch. added (weird that the compiler didn't emit a warning about the missing case)

[DelazJ]

@rouault Do I correctly understand that with this PR, the "output data types" parameter of the algs will get one more item (for those running gdal 3.7)?

[agiudiceandrea]

@DelazJ, the output data types for following GDAL algorithms should be updated:

ClipRasterByExtent.py
ClipRasterByMask.py
gdalcalc.py
GridAverage.py
GridDataMetrics.py
GridInverseDistance.py
GridInverseDistanceNearestNeighbor.py
GridLinear.py
GridNearestNeighbor.py
merge.py
proximity.py
rasterize.py
rearrange_bands.py
retile.py
translate.py
warp.py

Since it is an enum value, maybe it would be better to insert the new value at the end, in order to maintain compatibility with previous versions.

[DelazJ]

the output data types for following GDAL algorithms should be updated:

Plus a few QGIS ones (qgis/QGIS-Documentation@master...DelazJ:QGIS-Documentation:rasterTypesInclude), isn't it?

Since it is an enum value, maybe it would be better to insert the new value at the end, in order to maintain compatibility with previous versions.

That was my next question after the first one is confirmed. Considering that we have enum, where is that new value put ? (doesn't seem to be at the end in the code, so would it not break existing scripts?)

[rouault]

Re processing algorithms , I'm not familiar with them, so hard to comment for sure. But at first sight it seems that it is a drop down of strings, and that the index is stored (at least that's what I see in files like python/plugins/processing/tests/testdata/qgis_algorithm_tests4.yaml), which is a bit unfortunate. So yes, "Int8" if it was added should likely be put at the end to avoid breaking existing code/scripts. But perhaps I'm wrong

[agiudiceandrea]

Exactly.
For example:

QGIS/python/plugins/processing/algs/gdal/ClipRasterByExtent.py

Line 59 in f306546

self.TYPES = [self.tr('Use Input Layer Data Type'), 'Byte', 'Int16', 'UInt16', 'UInt32', 'Int32', 'Float32', 'Float64', 'CInt16', 'CInt32', 'CFloat32', 'CFloat64']

or:

QGIS/python/plugins/processing/algs/gdal/GridAverage.py

Line 56 in f306546

TYPES = ['Byte', 'Int16', 'UInt16', 'UInt32', 'Int32', 'Float32', 'Float64', 'CInt16', 'CInt32', 'CFloat32', 'CFloat64']

[agiudiceandrea]

Plus a few QGIS ones

For those ones (native:rasterbooleanand, native:rasterbooleanor, native:reclassifybylayer, native:reclassifybytable) the following code should be updated:

QGIS/src/analysis/processing/qgsrasteranalysisutils.cpp

Lines 187 to 203 in 6f55927

	void populateDataTypes()
	{
	if ( sDataTypes.empty() )
	{
	sDataTypes.append( qMakePair( QStringLiteral( "Byte" ), Qgis::DataType::Byte ) );
	sDataTypes.append( qMakePair( QStringLiteral( "Int16" ), Qgis::DataType::Int16 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "UInt16" ), Qgis::DataType::UInt16 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "Int32" ), Qgis::DataType::Int32 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "UInt32" ), Qgis::DataType::UInt32 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "Float32" ), Qgis::DataType::Float32 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "Float64" ), Qgis::DataType::Float64 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "CInt16" ), Qgis::DataType::CInt16 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "CInt32" ), Qgis::DataType::CInt32 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "CFloat32" ), Qgis::DataType::CFloat32 ) );
	sDataTypes.append( qMakePair( QStringLiteral( "CFloat64" ), Qgis::DataType::CFloat64 ) );
	}
	}