qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.52k stars 2.99k forks source link

Add capability to "untag" a raster alpha band as a regular band #42794

Open jerstlouis opened 3 years ago

jerstlouis commented 3 years ago

Describe the bug A GeoTIFF with 2 64-bit float bands do not display properly. It seems that one band might always be used as an alpha channel, regardless of the visualization setting (e.g. assigning the first band to the red channel and the second to the green channel).

How to Reproduce Open the attached GeoTIFF and try different visualization options, including Multiband color.

QGIS and OS versions QGIS 3.19.0-Master 'Master' (629b5b09994) Artix Linux 64-bit

Additional context Discussed this with @rouault testOutput.tif.zip

jerstlouis commented 3 years ago

Confirming that the following diff works around the problem:

diff --git a/src/core/raster/qgsrasterlayer.cpp b/src/core/raster/qgsrasterlayer.cpp
index 843de4f74bb..e4df5263b2b 100644
--- a/src/core/raster/qgsrasterlayer.cpp
+++ b/src/core/raster/qgsrasterlayer.cpp
@@ -775,7 +775,7 @@ void QgsRasterLayer::setDataProvider( QString const &provider, const QgsDataProv
   // Auto set alpha band
   for ( int bandNo = 1; bandNo <= mDataProvider->bandCount(); bandNo++ )
   {
-    if ( mDataProvider->colorInterpretation( bandNo ) == QgsRaster::AlphaBand )
+    if ( 0 && mDataProvider->colorInterpretation( bandNo ) == QgsRaster::AlphaBand )
     {
       if ( auto *lRenderer = mPipe.renderer() )
       {
gioman commented 3 years ago

Confirming that the following diff works around the problem:

@jerstlouis can you create a pull request?

rouault commented 3 years ago

can you create a pull request?

not with the proposed patch hopefully :-) The real fix would address why colorInterpretation() wrongly returns AlphaBand

jerstlouis commented 3 years ago

@gioman @rouault right, it was only a temporary work-around to identify the whereabout of the problem ;) Thanks @rouault for clarifying this further.

I imagine this means there is still a data provider class in-between GDAL and QGIS where this colorInterpretation() is implemented, since you mentioned it's not a GDAL issue?

rouault commented 3 years ago

I imagine this means there is still a data provider class in-between GDAL and QGIS where this colorInterpretation() is implemented, since you mentioned it's not a GDAL issue?

yep: https://github.com/qgis/QGIS/blob/7715a1291675c4402f95b93622fcc343683fcbee/src/core/providers/gdal/qgsgdalprovider.cpp#L1593

jerstlouis commented 3 years ago

So with this check at the top:

  if ( mMaskBandExposedAsAlpha && bandNo == GDALGetRasterCount( mGdalDataset ) + 1 )
    return colorInterpretationFromGdal( GCI_AlphaBand );

the fact that there are exactly two bands seems to trigger this to be recognized as Alpha, because mMaskBandExposedAsAlpha is also set to true?

That is set per:

  if ( ( nMaskFlags == 0 && bandCount == 1 ) || nMaskFlags == GMF_PER_DATASET )
    mMaskBandExposedAsAlpha = true;

Without this it would still rely on GDAL's GDALGetRasterColorInterpretation() function.

Not sure how QGIS/GDAL can reliably know whether a band is intended as alpha or not, ideally users would still have control over that in the styling even if a default behavior is selected, just like they can chose to assign to the Red or Green or Blue channel. That is how we have this set up in our software and CMSS styling language, e.g.:

{
   colorChannels.r = B04 / 4000;
   colorChannels.g = B03 / 4000;
   colorChannels.b = B02 / 4000;
   colorChannels.a = 1.0 - B14;
}
rouault commented 3 years ago

Actually, given the testOutput.tif.zip you include, QGIS behaviour is correct since the 2nd band is an alpha one:

$ gdalinfo testOutput.tif
[...]
Band 1 Block=111x4 Type=Float64, ColorInterp=Gray
Band 2 Block=111x4 Type=Float64, ColorInterp=Alpha

This comes from the content of TIFF tags:

$ tiffinfo testOutput.tif 
[...]
  Photometric Interpretation: min-is-black
  Extra Samples: 1<unassoc-alpha>
  Samples/Pixel: 2
[...]
jerstlouis commented 3 years ago

Thanks for pointing this out this @rouault -- sorry for all the troubles. It might be worthwhile for QGIS to consider allowing styles (the raster symbolizer) to override the default alpha band interpretation (e.g. allowing to map any band or none to the alpha channel). Would it make sense to re-cast this issue into a feature request for that, or maybe to open a new one?

jerstlouis commented 3 years ago

NOTE: The original issue turned out to be a wrong assumption of the GDAL OGCAPI driver, which was limited to byte in https://github.com/OSGeo/gdal/commit/b75f14ab12155c6b6f4544a9894c41f2f9a7d241

But this illustrates that there is a need to override a default wrong assumption (having 2 byte bands where neither represents alpha seems like a perfectly valid scenario).

Pedro-Murteira commented 2 years ago

Still valid on QGIS 3.22.4 and 3.24.0.