Hi all, looking at bug #11027 (getRGBLine based color space conversion can use bad tables) I realized that the byte_lookup table used for getLine methods in ImageColorMap was broken for some color spaces (Lab color space in the document attached to the bug report). I tried to fix the byte_lookup table, but I noticed that for some color spaces, like Lab, that don't implement getLine methods, getRGB / getGray, etc. are called for every pixel anyway even when getRGBLine or getGrayLine is used. For those cases we can just avoid allocating byte_lookup table, convert color values, and so on, and just directly call getRGB, getGray, etc. See attached patches. It fixes the problem for backends using getLine methods (cairo and Arthur I think)
ok to commit? -- Carlos Garcia Campos PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
From d7027fe28f139c91a2a6b53c175bd67c42f45218 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos <[email protected]> Date: Sun, 19 Jul 2009 15:51:43 +0200 Subject: [PATCH] Remove unused variable --- poppler/GfxState.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/poppler/GfxState.h b/poppler/GfxState.h index 6c8be49..b4ebb56 100644 --- a/poppler/GfxState.h +++ b/poppler/GfxState.h @@ -958,7 +958,6 @@ private: GfxColorComp * // lookup table lookup[gfxColorMaxComps]; Guchar *byte_lookup; - Guchar *tmp_line; double // minimum values for each component decodeLow[gfxColorMaxComps]; double // max - min value for each component -- 1.6.0.4
From 72bc5e75f22ccd999ee612507df48a006e1b5548 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos <[email protected]> Date: Sun, 19 Jul 2009 15:53:49 +0200 Subject: [PATCH] Don't use byte_lookup table when color space doesn't support getLine methods For color spaces that don't implement getRGBLine or getGrayLine methods, getRGB or getGray are called for every pixel, however we were allocating the byte_lookup table and converting colors in those cases too. Instead of falling back to generic methods in the base class, the new methods useGetRGBLine and useGetGrayLine have been added to he base class, so that when they are not suopported in the current color space byte_lookup table is not used at all. Fixes bug #11027. --- poppler/GfxState.cc | 125 ++++++++++++++++++++++++++++++--------------------- poppler/GfxState.h | 21 +++++++- 2 files changed, 91 insertions(+), 55 deletions(-) diff --git a/poppler/GfxState.cc b/poppler/GfxState.cc index 932bfff..b0a9879 100644 --- a/poppler/GfxState.cc +++ b/poppler/GfxState.cc @@ -260,25 +260,6 @@ char *GfxColorSpace::getColorSpaceModeName(int idx) { return gfxColorSpaceModeNames[idx]; } -void GfxColorSpace::getRGBLine(Guchar *in, unsigned int *out, int length) { - int i, j, n; - GfxColor color; - GfxRGB rgb; - - n = getNComps(); - for (i = 0; i < length; i++) { - - for (j = 0; j < n; j++) - color.c[j] = in[i * n + j] * 256; - - getRGB (&color, &rgb); - out[i] = - ((int) colToByte(rgb.r) << 16) | - ((int) colToByte(rgb.g) << 8) | - ((int) colToByte(rgb.b) << 0); - } -} - #ifdef USE_CMS cmsHPROFILE loadColorProfile(const char *fileName) { @@ -492,23 +473,6 @@ unsigned int getCMSNChannels(icColorSpaceSignature cs) #endif -void GfxColorSpace::getGrayLine(Guchar *in, unsigned char *out, int length) { - int i, j, n; - GfxColor color; - GfxGray gray; - - n = getNComps(); - for (i = 0; i < length; i++) { - - for (j = 0; j < n; j++) - color.c[j] = in[i * n + j] * 256; - - getGray (&color, &gray); - out[i] = colToByte(gray); - } -} - - //------------------------------------------------------------------------ // GfxDeviceGrayColorSpace //------------------------------------------------------------------------ @@ -1679,6 +1643,14 @@ void GfxICCBasedColorSpace::getCMYK(GfxColor *color, GfxCMYK *cmyk) { #endif } +GBool GfxICCBasedColorSpace::useGetRGBLine() { +#ifdef USE_CMS + return lineTransform != NULL || alt->useGetRGBLine(); +#else + return alt->useGetRGBLine(); +#endif +} + void GfxICCBasedColorSpace::getDefaultColor(GfxColor *color) { int i; @@ -4009,8 +3981,9 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, Object obj; double x[gfxColorMaxComps]; double y[gfxColorMaxComps]; - int i, j, k, byte; + int i, j, k; double mapped; + GBool useByteLookup; ok = gTrue; @@ -4029,6 +4002,7 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, for (k = 0; k < gfxColorMaxComps; ++k) { lookup[k] = NULL; } + byte_lookup = NULL; // get decode map if (decode->isNull()) { @@ -4066,7 +4040,9 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, // rather than component values. colorSpace2 = NULL; nComps2 = 0; - if (colorSpace->getMode() == csIndexed) { + useByteLookup = gFalse; + switch (colorSpace->getMode()) { + case csIndexed: // Note that indexHigh may not be the same as maxPixel -- // Distiller will remove unused palette entries, resulting in // indexHigh < maxPixel. @@ -4076,7 +4052,10 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, nComps2 = colorSpace2->getNComps(); lookup2 = indexedCS->getLookup(); colorSpace2->getDefaultRanges(x, y, indexHigh); - byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps2); + if (colorSpace2->useGetGrayLine() || colorSpace2->useGetRGBLine()) { + byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps2); + useByteLookup = gTrue; + } for (k = 0; k < nComps2; ++k) { lookup[k] = (GfxColorComp *)gmallocn(maxPixel + 1, sizeof(GfxColorComp)); @@ -4090,15 +4069,20 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, mapped = x[k] + (lookup2[j*nComps2 + k] / 255.0) * y[k]; lookup[k][i] = dblToCol(mapped); - byte_lookup[i * nComps2 + k] = (Guchar) (mapped * 255); + if (useByteLookup) + byte_lookup[i * nComps2 + k] = (Guchar) (mapped * 255); } } - } else if (colorSpace->getMode() == csSeparation) { + break; + case csSeparation: sepCS = (GfxSeparationColorSpace *)colorSpace; colorSpace2 = sepCS->getAlt(); nComps2 = colorSpace2->getNComps(); sepFunc = sepCS->getFunc(); - byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps2); + if (colorSpace2->useGetGrayLine() || colorSpace2->useGetRGBLine()) { + byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps2); + useByteLookup = gTrue; + } for (k = 0; k < nComps2; ++k) { lookup[k] = (GfxColorComp *)gmallocn(maxPixel + 1, sizeof(GfxColorComp)); @@ -4106,23 +4090,32 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, x[0] = decodeLow[0] + (i * decodeRange[0]) / maxPixel; sepFunc->transform(x, y); lookup[k][i] = dblToCol(y[k]); - byte_lookup[i*nComps2 + k] = (Guchar) (y[k] * 255); + if (useByteLookup) + byte_lookup[i*nComps2 + k] = (Guchar) (y[k] * 255); } } - } else { - byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps); + break; + default: + if (colorSpace->useGetGrayLine() || colorSpace->useGetRGBLine()) { + byte_lookup = (Guchar *)gmallocn ((maxPixel + 1), nComps); + useByteLookup = gTrue; + } for (k = 0; k < nComps; ++k) { lookup[k] = (GfxColorComp *)gmallocn(maxPixel + 1, sizeof(GfxColorComp)); for (i = 0; i <= maxPixel; ++i) { mapped = decodeLow[k] + (i * decodeRange[k]) / maxPixel; lookup[k][i] = dblToCol(mapped); - byte = (int) (mapped * 255.0 + 0.5); - if (byte < 0) - byte = 0; - else if (byte > 255) - byte = 255; - byte_lookup[i * nComps + k] = byte; + if (useByteLookup) { + int byte; + + byte = (int) (mapped * 255.0 + 0.5); + if (byte < 0) + byte = 0; + else if (byte > 255) + byte = 255; + byte_lookup[i * nComps + k] = byte; + } } } } @@ -4133,7 +4126,6 @@ GfxImageColorMap::GfxImageColorMap(int bitsA, Object *decode, obj.free(); err1: ok = gFalse; - byte_lookup = NULL; } GfxImageColorMap::GfxImageColorMap(GfxImageColorMap *colorMap) { @@ -4221,6 +4213,19 @@ void GfxImageColorMap::getGrayLine(Guchar *in, Guchar *out, int length) { int i, j; Guchar *inp, *tmp_line; + if ((colorSpace2 && !colorSpace2->useGetGrayLine ()) || + (!colorSpace2 && !colorSpace->useGetGrayLine ())) { + GfxGray gray; + + inp = in; + for (i = 0; i < length; i++) { + getGray (inp, &gray); + out[i] = colToByte(gray); + inp += nComps; + } + return; + } + switch (colorSpace->getMode()) { case csIndexed: case csSeparation: @@ -4251,6 +4256,22 @@ void GfxImageColorMap::getRGBLine(Guchar *in, unsigned int *out, int length) { int i, j; Guchar *inp, *tmp_line; + if ((colorSpace2 && !colorSpace2->useGetRGBLine ()) || + (!colorSpace2 && !colorSpace->useGetRGBLine ())) { + GfxRGB rgb; + + inp = in; + for (i = 0; i < length; i++) { + getRGB (inp, &rgb); + out[i] = + ((int) colToByte(rgb.r) << 16) | + ((int) colToByte(rgb.g) << 8) | + ((int) colToByte(rgb.b) << 0); + inp += nComps; + } + return; + } + switch (colorSpace->getMode()) { case csIndexed: case csSeparation: diff --git a/poppler/GfxState.h b/poppler/GfxState.h index b4ebb56..536c86a 100644 --- a/poppler/GfxState.h +++ b/poppler/GfxState.h @@ -183,8 +183,13 @@ public: virtual void getGray(GfxColor *color, GfxGray *gray) = 0; virtual void getRGB(GfxColor *color, GfxRGB *rgb) = 0; virtual void getCMYK(GfxColor *color, GfxCMYK *cmyk) = 0; - virtual void getRGBLine(Guchar *in, unsigned int *out, int length); - virtual void getGrayLine(Guchar *in, Guchar *out, int length); + virtual void getGrayLine(Guchar */*in*/, Guchar */*out*/, int /*length*/) {} + virtual void getRGBLine(Guchar */*in*/, unsigned int */*out*/, int /*length*/) {} + + // Does this ColorSpace use getRGBLine? + virtual GBool useGetRGBLine() { return gFalse; } + // Does this ColorSpace use getGrayLine? + virtual GBool useGetGrayLine() { return gFalse; } // Return the number of color components. virtual int getNComps() = 0; @@ -237,6 +242,9 @@ public: virtual void getGrayLine(Guchar *in, Guchar *out, int length); virtual void getRGBLine(Guchar *in, unsigned int *out, int length); + virtual GBool useGetRGBLine() { return gTrue; } + virtual GBool useGetGrayLine() { return gTrue; } + virtual int getNComps() { return 1; } virtual void getDefaultColor(GfxColor *color); @@ -301,6 +309,9 @@ public: virtual void getGrayLine(Guchar *in, Guchar *out, int length); virtual void getRGBLine(Guchar *in, unsigned int *out, int length); + virtual GBool useGetRGBLine() { return gTrue; } + virtual GBool useGetGrayLine() { return gTrue; } + virtual int getNComps() { return 3; } virtual void getDefaultColor(GfxColor *color); @@ -438,8 +449,10 @@ public: virtual void getGray(GfxColor *color, GfxGray *gray); virtual void getRGB(GfxColor *color, GfxRGB *rgb); virtual void getCMYK(GfxColor *color, GfxCMYK *cmyk); - virtual void getRGBLine(Guchar *in, unsigned int *out, int length); + + virtual GBool useGetRGBLine(); + virtual int getNComps() { return nComps; } virtual void getDefaultColor(GfxColor *color); @@ -483,6 +496,8 @@ public: virtual void getCMYK(GfxColor *color, GfxCMYK *cmyk); virtual void getRGBLine(Guchar *in, unsigned int *out, int length); + virtual GBool useGetRGBLine() { return gTrue; } + virtual int getNComps() { return 1; } virtual void getDefaultColor(GfxColor *color); -- 1.6.0.4
From f48ffc7efcbef5fa656b54ec62027992da698afc Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos <[email protected]> Date: Sun, 19 Jul 2009 16:02:33 +0200 Subject: [PATCH] Copy byte_lookup in copy constructor --- poppler/GfxState.cc | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/poppler/GfxState.cc b/poppler/GfxState.cc index b0a9879..6dc9221 100644 --- a/poppler/GfxState.cc +++ b/poppler/GfxState.cc @@ -4158,6 +4158,12 @@ GfxImageColorMap::GfxImageColorMap(GfxImageColorMap *colorMap) { memcpy(lookup[k], colorMap->lookup[k], n * sizeof(GfxColorComp)); } } + if (colorMap->byte_lookup) { + int nc = colorSpace2 ? nComps2 : nComps; + + byte_lookup = (Guchar *)gmallocn (n, nc); + memcpy(byte_lookup, colorMap->byte_lookup, n * nc); + } for (i = 0; i < nComps; ++i) { decodeLow[i] = colorMap->decodeLow[i]; decodeRange[i] = colorMap->decodeRange[i]; -- 1.6.0.4
signature.asc
Description: Esta parte del mensaje está firmada digitalmente
_______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
