[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: 30-bit displaying with Krita?
From:       Adrian Page <adrian () pagenet ! plus ! com>
Date:       2010-01-10 16:22:46
Message-ID: 4B49FED6.4060706 () pagenet ! plus ! com
[Download RAW message or body]

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


_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop


[prev in list] [next in list] [prev in thread] [next in thread]