Hello, we recently changed the default image format produced by the Qt frontend to RGB888 since that is improves rendering speed within Splash and yields a smaller QImage (and hence less copying for the Qt4 frontend).
However, doing some heap profiling, I noticed that this change should probably be reverted since this implies a conversion and hence at least one additional copy when converting the resulting QImage to a QPixmap with most Qt platforms, especially with Xcb using the Raster pixmap implementation. The increase in maximum working set size this yields will surely compensate any memory size savings of using RGB888 images as a source. Also, what we gain in speed during rendering, we would probably loose to some degree even when painting these RGB888 images directly, since Qt's Raster paint engine has no optimized blending functions for RGB888. The previous default format of ARGB32 was also suboptimal in this regards, since it would still imply that QRasterPlatformPixmap detaches from the QImage's data yielding an additional copy on which it would just change the format from ARGB32 to RGB32 after explicitly checking that it was actually given opaque data. Hence I think the correct choice is RGB32 for opaque pixel data since this has optimized blending functions and QRasterPlatformPixmap will neither convert nor copy this as it will usually coincide with the screens native format (as per QNativeImage::systemFormat). For IgnorePaperColor, the same can be achieved by choosing ARGB32_Premultiplied instead of ARGB32 which will also not be copied or converted by the Raster pixmap implementation. Attached is a patch that makes those two changes yielding significant reductions in maximum working set size. Also note that while the premultiplication does have a performance impact, it slows down the IgnorePaperColor path by less than one percent w.r.t. the default path. Best regards, Adam.
From 2bdc0d4bba86f73acb8cfe4373e3427ca1e692cf Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Sun, 16 Aug 2015 21:01:52 +0200 Subject: [PATCH 1/2] Add premultiplied alpha channel to SplashBitmap This extends the convertToXBGR method so that it can either * leave the alpha channel of the bitmap data opaque, * copy the alpha channel into the bitmap data, * transfer the alpha channel into the bitmap data and perform premultiplication on the bitmap data. --- qt4/src/poppler-page.cc | 6 +++++- qt5/src/poppler-page.cc | 6 +++++- splash/SplashBitmap.cc | 49 ++++++++++++++++++++++++++++++++++++++----------- splash/SplashBitmap.h | 11 +++++++++-- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/qt4/src/poppler-page.cc b/qt4/src/poppler-page.cc index 292f960..e965ee8 100644 --- a/qt4/src/poppler-page.cc +++ b/qt4/src/poppler-page.cc @@ -368,7 +368,11 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, // If we use DeviceN8, convert to XBGR8. // If requested, also transfer Splash's internal alpha channel. if (overprintPreview || ignorePaperColor) { - if (bitmap->convertToXBGR(ignorePaperColor)) { + const SplashBitmap::ConversionMode mode = ignorePaperColor + ? SplashBitmap::conversionAlpha + : SplashBitmap::conversionOpaque; + + if (bitmap->convertToXBGR(mode)) { SplashColorPtr data = bitmap->getDataPtr(); if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { diff --git a/qt5/src/poppler-page.cc b/qt5/src/poppler-page.cc index 8e071c1..58cea3a 100644 --- a/qt5/src/poppler-page.cc +++ b/qt5/src/poppler-page.cc @@ -368,7 +368,11 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, // If we use DeviceN8, convert to XBGR8. // If requested, also transfer Splash's internal alpha channel. if (overprintPreview || ignorePaperColor) { - if (bitmap->convertToXBGR(ignorePaperColor)) { + const SplashBitmap::ConversionMode mode = ignorePaperColor + ? SplashBitmap::conversionAlpha + : SplashBitmap::conversionOpaque; + + if (bitmap->convertToXBGR(mode)) { SplashColorPtr data = bitmap->takeData(); if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { diff --git a/splash/SplashBitmap.cc b/splash/SplashBitmap.cc index c905c97..7c14b31 100644 --- a/splash/SplashBitmap.cc +++ b/splash/SplashBitmap.cc @@ -461,7 +461,7 @@ void SplashBitmap::getRGBLine(int yl, SplashColorPtr line) { } } -void SplashBitmap::getXBGRLine(int yl, SplashColorPtr line, bool useAlpha) { +void SplashBitmap::getXBGRLine(int yl, SplashColorPtr line, ConversionMode conversionMode) { SplashColor col; double c, m, y, k, c1, m1, y1, k1, r, g, b; @@ -501,16 +501,34 @@ void SplashBitmap::getXBGRLine(int yl, SplashColorPtr line, bool useAlpha) { y1 = 1 - y; k1 = 1 - k; cmykToRGBMatrixMultiplication(c, m, y, k, c1, m1, y1, k1, r, g, b); - *line++ = dblToByte(clip01(b)); - *line++ = dblToByte(clip01(g)); - *line++ = dblToByte(clip01(r)); - *line++ = useAlpha ? getAlpha(x, yl) : 255; + + if (conversionMode == conversionAlphaPremultiplied) { + const double a = getAlpha(x, yl) / 255.0; + + *line++ = dblToByte(clip01(b * a)); + *line++ = dblToByte(clip01(g * a)); + *line++ = dblToByte(clip01(r * a)); + } else { + *line++ = dblToByte(clip01(b)); + *line++ = dblToByte(clip01(g)); + *line++ = dblToByte(clip01(r)); + } + + if (conversionMode != conversionOpaque) { + *line++ = getAlpha(x, yl); + } else { + *line++ = 255; + } } } -GBool SplashBitmap::convertToXBGR(bool useAlpha) { +static inline Guchar div255(int x) { + return (Guchar)((x + (x >> 8) + 0x80) >> 8); +} + +GBool SplashBitmap::convertToXBGR(ConversionMode conversionMode) { if (mode == splashModeXBGR8) { - if (useAlpha) { + if (conversionMode != conversionOpaque) { // Copy the alpha channel into the fourth component so that XBGR becomes ABGR. const SplashColorPtr dbegin = data; const SplashColorPtr dend = data + rowSize * height; @@ -518,11 +536,20 @@ GBool SplashBitmap::convertToXBGR(bool useAlpha) { Guchar *const abegin = alpha; Guchar *const aend = alpha + width * height; - SplashColorPtr d = dbegin + 3; + SplashColorPtr d = dbegin; Guchar *a = abegin; - for(; d < dend && a < aend; d += 4, a += 1) { - *d = *a; + if (conversionMode == conversionAlphaPremultiplied) { + for (; d < dend && a < aend; d += 4, a += 1) { + d[0] = div255(d[0] * *a); + d[1] = div255(d[1] * *a); + d[2] = div255(d[2] * *a); + d[3] = *a; + } + } else { + for (d += 3; d < dend && a < aend; d += 4, a += 1) { + *d = *a; + } } } @@ -534,7 +561,7 @@ GBool SplashBitmap::convertToXBGR(bool useAlpha) { if (newdata != NULL) { for (int y = 0; y < height; y++) { unsigned char *row = newdata + y * newrowSize; - getXBGRLine(y, row, useAlpha); + getXBGRLine(y, row, conversionMode); } if (rowSize < 0) { gfree(data + (height - 1) * rowSize); diff --git a/splash/SplashBitmap.h b/splash/SplashBitmap.h index 9fa7613..4417af7 100644 --- a/splash/SplashBitmap.h +++ b/splash/SplashBitmap.h @@ -76,11 +76,18 @@ public: SplashError writeImgFile(SplashImageFileFormat format, FILE *f, int hDPI, int vDPI, const char *compressionString = ""); SplashError writeImgFile(ImgWriter *writer, FILE *f, int hDPI, int vDPI); - GBool convertToXBGR(bool useAlpha = false); + enum ConversionMode + { + conversionOpaque, + conversionAlpha, + conversionAlphaPremultiplied + }; + + GBool convertToXBGR(ConversionMode conversionMode = conversionOpaque); void getPixel(int x, int y, SplashColorPtr pixel); void getRGBLine(int y, SplashColorPtr line); - void getXBGRLine(int y, SplashColorPtr line, bool useAlpha = false); + void getXBGRLine(int y, SplashColorPtr line, ConversionMode conversionMode = conversionOpaque); #if SPLASH_CMYK void getCMYKLine(int y, SplashColorPtr line); #endif -- 2.5.0 From 13e43746cfdd9f5f188cb709dd33e071c667ece5 Mon Sep 17 00:00:00 2001 From: Adam Reichold <[email protected]> Date: Sun, 16 Aug 2015 21:05:38 +0200 Subject: [PATCH 2/2] Change default Qt frontend image format Change the default image format used by the Qt frontends to XBGR/RGB32 which allows efficient blending and pixmap conversion via Qt. Also use the premultiplied conversion for IgnorePaperColor to elide a further copy for pixmap conversion. --- qt4/src/poppler-page.cc | 46 +++++++++++++++------------------------------- qt5/src/poppler-page.cc | 46 +++++++++++++++------------------------------- 2 files changed, 30 insertions(+), 62 deletions(-) diff --git a/qt4/src/poppler-page.cc b/qt4/src/poppler-page.cc index e965ee8..16a18f1 100644 --- a/qt4/src/poppler-page.cc +++ b/qt4/src/poppler-page.cc @@ -327,11 +327,7 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, bgColor[2] = m_page->parentDoc->paperColor.red(); } - SplashColorMode colorMode = splashModeRGB8; - - const bool ignorePaperColor = m_page->parentDoc->m_hints & Document::IgnorePaperColor; - if (ignorePaperColor) colorMode = splashModeXBGR8; - + SplashColorMode colorMode = splashModeXBGR8; #if SPLASH_CMYK if (overprintPreview) colorMode = splashModeDeviceN8; #endif @@ -340,6 +336,8 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, if (m_page->parentDoc->m_hints & Document::ThinLineShape) thinLineMode = splashThinLineShape; if (m_page->parentDoc->m_hints & Document::ThinLineSolid) thinLineMode = splashThinLineSolid; + const bool ignorePaperColor = m_page->parentDoc->m_hints & Document::IgnorePaperColor; + SplashOutputDev splash_output( colorMode, 4, gFalse, @@ -367,45 +365,31 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, // If we use DeviceN8, convert to XBGR8. // If requested, also transfer Splash's internal alpha channel. - if (overprintPreview || ignorePaperColor) { - const SplashBitmap::ConversionMode mode = ignorePaperColor - ? SplashBitmap::conversionAlpha - : SplashBitmap::conversionOpaque; - - if (bitmap->convertToXBGR(mode)) { - SplashColorPtr data = bitmap->getDataPtr(); - - if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { - // Convert byte order from RGBX to XBGR. - for (int i = 0; i < bh; ++i) { - for (int j = 0; j < bw; ++j) { - SplashColorPtr pixel = &data[i * brs + j]; - - qSwap(pixel[0], pixel[3]); - qSwap(pixel[1], pixel[2]); - } - } - } + const SplashBitmap::ConversionMode mode = ignorePaperColor + ? SplashBitmap::conversionAlphaPremultiplied + : SplashBitmap::conversionOpaque; - // Construct a Qt image sharing the raw bitmap data. - img = QImage(data, bw, bh, brs, QImage::Format_ARGB32).copy(); - } - } else { + const QImage::Format format = ignorePaperColor + ? QImage::Format_ARGB32_Premultiplied + : QImage::Format_RGB32; + + if (bitmap->convertToXBGR(mode)) { SplashColorPtr data = bitmap->getDataPtr(); if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { - // Convert byte order from BGR to RGB. + // Convert byte order from RGBX to XBGR. for (int i = 0; i < bh; ++i) { for (int j = 0; j < bw; ++j) { SplashColorPtr pixel = &data[i * brs + j]; - qSwap(pixel[0], pixel[2]); + qSwap(pixel[0], pixel[3]); + qSwap(pixel[1], pixel[2]); } } } // Construct a Qt image sharing the raw bitmap data. - img = QImage(data, bw, bh, brs, QImage::Format_RGB888).copy(); + img = QImage(data, bw, bh, brs, format).copy(); } #endif break; diff --git a/qt5/src/poppler-page.cc b/qt5/src/poppler-page.cc index 58cea3a..3832861 100644 --- a/qt5/src/poppler-page.cc +++ b/qt5/src/poppler-page.cc @@ -327,11 +327,7 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, bgColor[2] = m_page->parentDoc->paperColor.red(); } - SplashColorMode colorMode = splashModeRGB8; - - const bool ignorePaperColor = m_page->parentDoc->m_hints & Document::IgnorePaperColor; - if (ignorePaperColor) colorMode = splashModeXBGR8; - + SplashColorMode colorMode = splashModeXBGR8; #if SPLASH_CMYK if (overprintPreview) colorMode = splashModeDeviceN8; #endif @@ -340,6 +336,8 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, if (m_page->parentDoc->m_hints & Document::ThinLineShape) thinLineMode = splashThinLineShape; if (m_page->parentDoc->m_hints & Document::ThinLineSolid) thinLineMode = splashThinLineSolid; + const bool ignorePaperColor = m_page->parentDoc->m_hints & Document::IgnorePaperColor; + SplashOutputDev splash_output( colorMode, 4, gFalse, @@ -367,45 +365,31 @@ QImage Page::renderToImage(double xres, double yres, int x, int y, int w, int h, // If we use DeviceN8, convert to XBGR8. // If requested, also transfer Splash's internal alpha channel. - if (overprintPreview || ignorePaperColor) { - const SplashBitmap::ConversionMode mode = ignorePaperColor - ? SplashBitmap::conversionAlpha - : SplashBitmap::conversionOpaque; - - if (bitmap->convertToXBGR(mode)) { - SplashColorPtr data = bitmap->takeData(); - - if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { - // Convert byte order from RGBX to XBGR. - for (int i = 0; i < bh; ++i) { - for (int j = 0; j < bw; ++j) { - SplashColorPtr pixel = &data[i * brs + j]; - - qSwap(pixel[0], pixel[3]); - qSwap(pixel[1], pixel[2]); - } - } - } + const SplashBitmap::ConversionMode mode = ignorePaperColor + ? SplashBitmap::conversionAlphaPremultiplied + : SplashBitmap::conversionOpaque; - // Construct a Qt image holding (and also owning) the raw bitmap data. - img = QImage(data, bw, bh, brs, QImage::Format_ARGB32, gfree, data); - } - } else { + const QImage::Format format = ignorePaperColor + ? QImage::Format_ARGB32_Premultiplied + : QImage::Format_RGB32; + + if (bitmap->convertToXBGR(mode)) { SplashColorPtr data = bitmap->takeData(); if (QSysInfo::ByteOrder == QSysInfo::BigEndian) { - // Convert byte order from BGR to RGB. + // Convert byte order from RGBX to XBGR. for (int i = 0; i < bh; ++i) { for (int j = 0; j < bw; ++j) { SplashColorPtr pixel = &data[i * brs + j]; - qSwap(pixel[0], pixel[2]); + qSwap(pixel[0], pixel[3]); + qSwap(pixel[1], pixel[2]); } } } // Construct a Qt image holding (and also owning) the raw bitmap data. - img = QImage(data, bw, bh, brs, QImage::Format_RGB888, gfree, data); + img = QImage(data, bw, bh, brs, format, gfree, data); } #endif break; -- 2.5.0
signature.asc
Description: OpenPGP digital signature
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
