30-bit displaying with Krita?

[Adrian Page]

Boudewijn Rempt wrote:
> On Saturday 09 January 2010, Boudewijn Rempt wrote:
>> On Friday 08 January 2010, Adrian Page wrote:
>>> Boudewijn Rempt wrote:
>>>> Adrian investigated the Qt source and came to the conclusision that
>>>> just setting the right datatype would indeed be enough. I'm not sure
>>>> whether he's making the change, or whether I'm going to do it this
>>>> weekend, but it'll be done.
>>> I haven't done anything as I think you put it on your feature list,
>>> though I could do it if you wanted.
>> I'm giving it a try -- it'll be good for me, to use my opengl books again.
>> I think however that it's a bit more work, since I see we only use other
>> texture formats than GL_RGBA8 if a colorspace returns true for
>> hasDynamicRange.
> 
> Ok, here's my first attempt. It tries to convert non-rgba colorspace images
> that have a channel depth better than 8 bit integer to rgba16 as well. 
> diff --git a/krita/ui/opengl/kis_opengl_image_textures.cpp b/krita/ui/opengl/kis_opengl_image_textures.cpp
> index 0ffb44a..083814f 100644
> --- a/krita/ui/opengl/kis_opengl_image_textures.cpp
> +++ b/krita/ui/opengl/kis_opengl_image_textures.cpp
> @@ -33,9 +33,10 @@
>  #endif
>  
>  #include <KoColorSpace.h>
> -#include "KoColorSpaceRegistry.h"
> -#include "KoColorProfile.h"
> -#include "KoIntegerMaths.h"
> +#include <KoColorSpaceRegistry.h>
> +#include <KoColorProfile.h>
> +#include <KoIntegerMaths.h>
> +#include <KoColorModelStandardIds.h>
>  
>  #include "kis_global.h"
>  
> @@ -243,21 +244,12 @@ void KisOpenGLImageTextures::updateImageTextureTiles(const QRect& rect)
>  
>                  if (m_imageTextureInternalFormat == GL_RGBA8) {
>                      tileUpdateImage = m_image->convertToQImage(tileUpdateRect.x(), tileUpdateRect.y(),
> -                                      tileUpdateRect.width(), tileUpdateRect.height(),
> -                                      m_monitorProfile);
> -
> +                                                               tileUpdateRect.width(), tileUpdateRect.height(),
> +                                                               m_monitorProfile);
> +
>                      pixels = tileUpdateImage.bits();
> -                } else {
> +                }
> +                else {
>                      pixels = new quint8[tileUpdateRect.width() * tileUpdateRect.height() * m_image->colorSpace()->pixelSize()];
>                      Q_CHECK_PTR(pixels);
>                      deletePixelsAfterUse = true;
> @@ -267,9 +259,13 @@ void KisOpenGLImageTextures::updateImageTextureTiles(const QRect& rect)
>  
>  #if defined(HAVE_GLEW) && defined(HAVE_OPENEXR)
>                      // XXX: generalise
> -                    if (m_image->colorSpace()->id() == "RgbAF16") {
> -                        if (m_imageTextureType == GL_FLOAT) {
>  
> +                    if (m_image->colorSpace()->colorModelId() == RGBAColorModelID ) {
> +                        KoID colorDepthId = m_image->colorSpace()->colorDepthId();
> +                        if (colorDepthId == Integer16BitsColorDepthID) {
> +                            // Any code needed here?

pixels should be converted to the m_monitorProfile, but see below.

> +                        }
> +                        else if (colorDepthId == Float16BitsColorDepthID && m_imageTextureType == GL_FLOAT) {
>                              // Convert half to float as we don't have ARB_half_float_pixel
>                              const int NUM_RGBA_COMPONENTS = 4;
>                              qint32 halfCount = tileUpdateRect.width() * tileUpdateRect.height() * NUM_RGBA_COMPONENTS;
> @@ -285,9 +281,18 @@ void KisOpenGLImageTextures::updateImageTextureTiles(const QRect& rect)
>                              }
>                              delete [] pixels;
>                              pixels = reinterpret_cast<quint8 *>(pixels_as_floats);
> -                        } else {
> -                            Q_ASSERT(m_imageTextureType == GL_HALF_FLOAT_ARB);
>                          }
> +                        else if (colorDepthId == Float32BitsColorDepthID) {
> +                            // Any code needed here?

This depends on m_imageTextureInternalFormat as it might need to be 
converted to rgb16 if not using hdr exposure program, more below...

> +                        }
> +                    }
> +                    else {
> +                        // Convert to 16 bit rgba from the current colorspace (which is integer 16, float 16 or float 32, but not rgba)
> +                        const KoColorSpace* rgb16 = KoColorSpaceRegistry::instance()->rgb16();

This should pass in m_monitorProfile.

> +                        quint8* rgb16pixels = rgb16->allocPixelBuffer(tileUpdateRect.width() * tileUpdateRect.height());
> +                        m_image->colorSpace()->convertPixelsTo(pixels, rgb16pixels, rgb16, tileUpdateRect.width() * tileUpdateRect.height());
> +                        delete[] pixels;
> +                        pixels = rgb16pixels;
>                      }
>  #endif
>                  }

I think to catch all cases this block of code would be better if it uses 
m_imageTextureInternalFormat to decide which conversions are needed, e.g:

if (m_imageTextureInternalFormat == GL_RGBA8)
	convert to QImage as now
else
	read pixels from image
	if (m_imageTextureInternalFormat == GL_RGBA16)
		convert pixels to rgb16 (using m_monitorProfile)
	else if (colorDepthId == Float16BitsColorDepthID && m_imageTextureType 
== GL_FLOAT)
		convert pixels from half to float as now


>  void KisOpenGLImageTextures::setImageTextureFormat()
>  {
>      m_imageTextureInternalFormat = GL_RGBA8;
>      m_imageTextureType = GL_UNSIGNED_BYTE;
> -
> +    m_usingHDRExposureProgram = imageCanUseHDRExposureProgram(m_image);
>  
>  #ifdef HAVE_GLEW
> -    QString colorSpaceId = m_image->colorSpace()->id();
> -    m_usingHDRExposureProgram = false;
> +    KoID colorModelId = m_image->colorSpace()->colorModelId();
> +    KoID colorDepthId = m_image->colorSpace()->colorDepthId();
>  
>      dbgUI << "Choosing texture format:";
>  
> -    if (imageCanUseHDRExposureProgram(m_image)) {
> -
> -        if (colorSpaceId == "RGBAF16HALF") {
> +    if (colorModelId == RGBAColorModelID) {
> +        if (colorDepthId == Float16BitsColorDepthID) {

This should be && m_usingHDRExposureProgram.

>  
>              if (GLEW_ARB_texture_float) {
>                  m_imageTextureInternalFormat = GL_RGBA16F_ARB;
> @@ -523,7 +502,8 @@ void KisOpenGLImageTextures::setImageTextureFormat()
>  
>              m_usingHDRExposureProgram = true;
>  
> -        } else if (colorSpaceId == "RGBAF32") {
> +        }
> +        else if (colorDepthId == Float32BitsColorDepthID) {

Same again, && m_usingHDRExposureProgram. Looks good otherwise.

Have you noticed any performance change? Painting has become very 
stop-start on my desktop in windows - haven't tried any other setups 
yet. RGBA8 might have a fast path in the driver/hardware, but it's 
probably too soon to say anymore.

Adrian