30-bit displaying with Krita?

[Boudewijn Rempt]

On Sunday 10 January 2010, Adrian Page wrote:
> 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.
> 

Ok, I'll try to fix these issues tomorrow & submit a new patch :-)

> 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.

Hm... Yes -- I think I did notice a bit of a performance degradation when 
painting on a 16 bit rgba canvas.

-- 
Boudewijn Rempt | http://www.valdyas.org