iLiad Enhancing Poppler for iLiad

[scotty1024]

I pulled these four patches from the Poppler project that apply to the iLiad Poppler but have not been applied by iRex.

I'm running them along with a highlighter enhancement. I'll be posting a patched poppler library shortly.

That fix:
* splash/Splash.cc:
* splash/SplashErrorCodes.h: Do not crash on documents that report a
0x0 mask for an image, like
http://bugs.kde.org/attachment.cgi?id=18083&action=view
* splash/Splash.cc: Initialize the values of nClipRes
* poppler/Function.cc: Initialize PostScriptFunction::codeString to
NULL so that it can safely deleted if initialization fails.
Fixes #9263.

--- SplashErrorCodes.h	2005/03/03 19:45:59	1.1
+++ SplashErrorCodes.h	2006/10/11 23:09:24	1.2
@@ -27,4 +27,6 @@
 
 #define splashErrSingularMatrix  8	// matrix is singular
 
+#define splashErrZeroImage       9      // image of 0x0
+
 #endif

--- Splash.cc	2006/05/31 20:14:04	1.8
+++ Splash.cc	2006/10/11 23:09:24	1.9
@@ -2515,6 +2515,8 @@
 	   (double)mat[3], (double)mat[4], (double)mat[5]);
   }
 
+  if (w == 0 && h == 0) return splashErrZeroImage;
+
   // check for singular matrix
   if (splashAbs(mat[0] * mat[3] - mat[1] * mat[2]) < 0.000001) {
     return splashErrSingularMatrix;

--- Splash.cc	2006/10/11 23:09:24	1.9
+++ Splash.cc	2006/11/19 12:59:48	1.10
@@ -445,7 +445,7 @@
   int x0, x1, x2, x3, y0, y1, x, y, t;
   SplashCoord dx, dy, dxdy;
   SplashClipResult clipRes;
-  int nClipRes[3];
+  int nClipRes[3] = {0, 0, 0};
   int i;
 
   for (i = 0, seg = xPath->segs; i < xPath->length; ++i, ++seg) {

--- Function.cc	2006/05/05 20:51:01	1.5
+++ Function.cc	2006/12/10 05:24:56	1.6
@@ -978,6 +978,7 @@
   GooString *tok;
 
   code = NULL;
+  codeString = NULL;
   codeSize = 0;
   ok = gFalse;

[scotty1024]

I wondered why I was chasing my tail so vigorously with the highlighter enhancement, then I found this bug...

In SplashOutputDev.cc in method void SplashOutputDev::setFillColor(int r, int g, int b)

  gray = (GfxColorComp)(0.299 * rgb.r + 0.587 * rgb.g + 0.114 * rgb.g + 0.5);

Should be

  gray = (GfxColorComp)(0.299 * rgb.r + 0.587 * rgb.g + 0.114 * rgb.b + 0.5);

Building a better iLiad, one Poppler bug at at time.

Entered as Poppler bug 9386.

I've also built a splashModeMono4 into Poppler and I'm testing it to make sure the error dispersion works the way iRex likes it. This will require a new poppler and a new ipdf to use it. The new ipdf won't work with an older poppler since the older poppler doesn't grok splashModeMono4. But an old ipdf will work with the new poppler.

[henkvdg]

Originally Posted by scotty1024

I wondered why I was chasing my tail so vigorously with the highlighter enhancement, then I found this bug...

In SplashOutputDev.cc in method void SplashOutputDev::setFillColor(int r, int g, int b)

  gray = (GfxColorComp)(0.299 * rgb.r + 0.587 * rgb.g + 0.114 * rgb.g + 0.5);

Should be

  gray = (GfxColorComp)(0.299 * rgb.r + 0.587 * rgb.g + 0.114 * rgb.b + 0.5);

Building a better iLiad, one Poppler bug at at time.

I've also built a splashModeMono4 into Poppler and I'm testing it to make sure the error dispersion works the way iRex likes it. This will require a new poppler and a new ipdf to use it. The new ipdf won't work with an older poppler since the older poppler doesn't grok splashModeMono4. But an old ipdf will work with the new poppler.

Good work, Scotty !

[tribble]

hehe, if you get that commenting functionality (even just doing highlights with the pen) then you are my personal hero

Well, you are already.

[Matthijs]

Nice spot. In the current code this bug is not normally encountered, but for highlighting it indeed may

Anyhow: we've been modifying poppler on our end to get the dithering only on images, attached is the current diff with the CVS version of poppler that was posted before.

Remember this is work in progress.

[Matthijs]

BTW: I've just synced our internal poppler repo with the latest CVS version of poppler, which includes the fixes in the first post. So if you use the poppler CVS version of today and apply the patch above, you get the same bleeding-edge we are currently using

Next public release of iPDF I will also include poppler in the public repository.

[scotty1024]

Originally Posted by Matthijs

Remember this is work in progress.

Adding a Mono4 mode solves some of the issues you all are encountering, after all, with the fixes to the gray calculation Mono8 is now back to being "perfect". The issue with image quality is in converting it to Mono4 for the iLiad.

You need to be careful with adding Mono4 as to not break binary compatibility, it must be added to the end of the enums.

splash/SplashTypes.h

 splashModeBGRA8               // 1 byte per component, 4 bytes per pixel:     
                                //   BGRABGRA...                                
#if SPLASH_CMYK
  ,
  splashModeCMYK8,              // 1 byte per component, 4 bytes per pixel:     
                                //   CMYKCMYK...                                
  splashModeACMYK8              // 1 byte per component, 5 bytes per pixel:     
                                //   ACMYKACMYK                                 
#endif
  ,splashModeMono4              // 1 byte per component, 1 pixel per byte,      
                                // most significant 4 bits contain pixel value  
};

Adding this means you can do the gamma adjustment for the fonts etc...

An example of splashModeMono4 in action in my Poppler.

struct SplashOutImageData {
  ImageStream *imgStr;
  GfxImageColorMap *colorMap;
  SplashColorPtr lookup;
  int *maskColors;
  SplashColorMode colorMode;
  int width, height, y;
};

 int *_ed_errors = NULL;

GBool SplashOutputDev::imageSrc(void *data, SplashColorPtr line) {
  SplashOutImageData *imgData = (SplashOutImageData *)data;
  Guchar *p;
  SplashColorPtr q, col;
  GfxRGB rgb;
  GfxGray gray;
#if SPLASH_CMYK
  GfxCMYK cmyk;
#endif
  int nComps, x;

  if ((imgData->colorMode == splashModeMono4) &&
      (imgData->y == 0)) {
    // If we have an error distribution array, free it (must have had an        
    // error with previous image...)                                            
    if (_ed_errors) {
      gfree(_ed_errors);
    }

    // Need to allocate error distribution array and zero it.                   
    _ed_errors = (int *)gmalloc((imgData->width + 2) * 4);
    memset(_ed_errors, 0, (imgData->width + 2) * 4);
  }

  if (imgData->y == imgData->height) {
    // If we have an error distribution array, free it                          
    if (_ed_errors) {
      gfree(_ed_errors);
      _ed_errors = NULL;
    }
    return gFalse;
  }

  nComps = imgData->colorMap->getNumPixelComps();

  if (imgData->lookup) {
    switch (imgData->colorMode) {
  case splashModeMono4:
    {
      int origGrey = 0;
      int grey = 0;
      int error = 0;
      int errorRight = 0;
      int errorDownRight = 0;

      for (x = 0, p = imgData->imgStr->getLine(), q = line;
           x < imgData->width;
           ++x, p += nComps) {
        origGrey = imgData->lookup[*p] + errorRight + _ed_errors[x+1];

        // Convert pixel to Mono 4, clip < 0 and > Mono 4                       
        if (origGrey <= 0) {
          grey = 0;
        } else if (origGrey  >= 0x00ff) {
          grey = 0x00ff; // No error for white of 255                           
        } else {
          grey = origGrey & 0x00f0;
        }

        // Put Mono 4 pixel with distributed error back                         
        *q++ = grey;

        // Calculate error                                                      
        error = origGrey - grey;

        // Distribute error                                                     
        errorRight = (error * 7) >> 4;
        _ed_errors[x] += (error * 3) >> 4;
        _ed_errors[x+1] = ((error * 5) >> 4) + errorDownRight;
        errorDownRight = error >> 4;
      }
    }
    break;
  case splashModeMono1:
  case splashModeMono8:

[scotty1024]

I've tweaked Poppler's Makefile's to use these optimization switches and it looks better on the benchmarks here...

CXXFLAGS = -Wall -Wno-unused -g -O2 -mcpu=xscale -mtune=xscale

[scotty1024]

Poppler bug 9386 has been fixed.

[Matthijs]

The optimization you suggest is used in OpenEmbedded by default, so the poppler that comes with a release uses "-fexpensive-optimizations -fomit-frame-pointer -frename-registers -O2" and also the -mcpu and -mtune options (everything is automatically compiled with that).
For stand-alone compilation this is a very valid remark though.

[scotty1024]

Bug 9405 submitted to Poppler.

Enhance PSTokenizer::getToken 

1. Reserve space for terminating 0 by decrementing size in advance (rather than always having to account for it)
2. Utilize new method consumeChar() to indicate we've used the char returned from lookChar().
3. Made placement of calls to consumeChar() consistent inside parsing code.

Add new method PSTokenizer::consumeChar()
This method is more efficient than getChar() for consuming character returned by lookChar().

Enhance PSTokenizer::getChar()
Changed to reduce writes to charBuffer.

*** PSTokenizer.h.~1.1.1.1.~    Thu Mar  3 11:46:01 2005
--- PSTokenizer.h       Wed Dec 20 08:37:09 2006
***************
*** 29,34 ****
--- 29,35 ----
  private:
  
    int lookChar();
+   void consumeChar();
    int getChar();
  
    int (*getCharFunc)(void *);

*** PSTokenizer.cc.~1.1.1.1.~   Thu Mar  3 11:46:03 2005
--- PSTokenizer.cc      Wed Dec 20 08:32:02 2006
***************
*** 55,61 ****
    int c;
    int i;
  
!   // skip whitespace and comments
    comment = gFalse;
    while (1) {
      if ((c = getChar()) == EOF) {
--- 55,61 ----
    int c;
    int i;
  
!   // skip leading whitespace and comments
    comment = gFalse;
    while (1) {
      if ((c = getChar()) == EOF) {
***************
*** 74,89 ****
      }
    }
  
    // read a token
    i = 0;
    buf[i++] = c;
    if (c == '(') {
      backslash = gFalse;
      while ((c = lookChar()) != EOF) {
!       if (i < size - 1) {
        buf[i++] = c;
        }
-       getChar();
        if (c == '\\') {
        backslash = gTrue;
        } else if (!backslash && c == ')') {
--- 74,93 ----
      }
    }
  
+   // Reserve room for terminating '\0'
+   size--;
+ 
    // read a token
    i = 0;
    buf[i++] = c;
+ 
    if (c == '(') {
      backslash = gFalse;
      while ((c = lookChar()) != EOF) {
!       consumeChar();
!       if (i < size) {
        buf[i++] = c;
        }
        if (c == '\\') {
        backslash = gTrue;
        } else if (!backslash && c == ')') {
***************
*** 94,101 ****
      }
    } else if (c == '<') {
      while ((c = lookChar()) != EOF) {
!       getChar();
!       if (i < size - 1) {
        buf[i++] = c;
        }
        if (c == '>') {
--- 98,105 ----
      }
    } else if (c == '<') {
      while ((c = lookChar()) != EOF) {
!       consumeChar();
!       if (i < size) {
        buf[i++] = c;
        }
        if (c == '>') {
***************
*** 104,116 ****
      }
    } else if (c != '[' && c != ']') {
      while ((c = lookChar()) != EOF && !specialChars[c]) {
!       getChar();
!       if (i < size - 1) {
        buf[i++] = c;
        }
      }
    }
    buf[i] = '\0';
    *length = i;
  
    return gTrue;
--- 108,124 ----
      }
    } else if (c != '[' && c != ']') {
      while ((c = lookChar()) != EOF && !specialChars[c]) {
!       consumeChar();
!       if (i < size) {
        buf[i++] = c;
        }
      }
    }
+ 
+   // Zero terminate token string
    buf[i] = '\0';
+ 
+   // Return length of token
    *length = i;
  
    return gTrue;
***************
*** 123,135 ****
    return charBuf;
  }
  
  int PSTokenizer::getChar() {
!   int c;
  
!   if (charBuf < 0) {
!     charBuf = (*getCharFunc)(data);
    }
!   c = charBuf;
!   charBuf = -1;
    return c;
  }
--- 131,148 ----
    return charBuf;
  }
  
+ void PSTokenizer::consumeChar() {
+   charBuf = -1;
+ }
+ 
  int PSTokenizer::getChar() {
!   int c = charBuf;
  
!   if (c < 0) {
!     c = (*getCharFunc)(data);
!   } else {
!     charBuf = -1;
    }
! 
    return c;
  }