Async generation fixes. I hope all segfaults and mutex overlocking have

been fixed. If someone finds one of those, please tell me.
Here are thread generator pros/cons:
- generation is ~20% slower
+ thumbnaillist or pageview scrolling is fast (never blocked anymore)
+ all UI feels so better and responsive
So:
o thumbnaillist will be all ASYNC.
o pageview will use ASYNC for _preloading_ next pages and a mixed A/S for
  scrolling/viewing the document
p previewwidget and others (searches, etc) are SYNC as obvious.
Btw now async generation is enabled everywhere.

svn path=/branches/kpdf_experiments/kdegraphics/kpdf/; revision=373810

kpdf/QOutputDev.cpp

@@ -62,17 +62,6 @@ void KPDFOutputDev::setParams( int width, int height, bool genT, bool genL, bool
         m_text = new TextPage( gFalse );
 }
 
-bool KPDFOutputDev::generateTextPage() const
-{
-    return m_generateText;
-}
-
-bool KPDFOutputDev::generateRects() const
-{
-    return m_generateLinks;
-}
-
-
 KPDFLink * KPDFOutputDev::generateLink( LinkAction * a )
 {
     KPDFLink * link = NULL;
@@ -182,11 +171,6 @@ QValueList< KPDFPageRect * > KPDFOutputDev::takeRects()
     return rectsCopy;
 }
 
-void KPDFOutputDev::freeInternalBitmap()
-{
-    // ### hack: unload memory used by internal SplashOutputDev's bitmap
-    SplashOutputDev::startPage( 0, NULL );
-}
 
 void KPDFOutputDev::startPage( int pageNum, GfxState *state )
 {
@@ -201,10 +185,10 @@ void KPDFOutputDev::endPage()
     if ( m_generateText )
         m_text->coalesce( gTrue );
 
-    // create a QImage over the internally generated pixmap
     int bh = getBitmap()->getHeight(),
         bw = getBitmap()->getWidth();
     SplashColorPtr dataPtr = getBitmap()->getDataPtr();
+    // construct a qimage SHARING the raw bitmap data in memory
     QImage * img = new QImage( (uchar*)dataPtr.rgb8, bw, bh, 32, 0, 0, QImage::IgnoreEndian );
 
     // use the QImage or convert it immediately to QPixmap for better
@@ -212,15 +196,12 @@ void KPDFOutputDev::endPage()
     if ( m_qtThreadSafety )
     {
         delete m_image;
-        m_image = img;
         // it may happen (in fact it doesn't) that we need a rescaling
         if ( bw != m_pixmapWidth && bh != m_pixmapHeight )
-        {
             m_image = new QImage( img->smoothScale( m_pixmapWidth, m_pixmapHeight ) );
-            delete img;
-        }
-        m_image->detach();
-        // note: internal bitmap will be freed by PDFGenerator after getting the data
+        else
+            // dereference image from the xpdf memory
+            m_image = new QImage( img->copy() );
     }
     else
     {
@@ -230,10 +211,11 @@ void KPDFOutputDev::endPage()
             m_pixmap = new QPixmap( img->smoothScale( m_pixmapWidth, m_pixmapHeight ) );
         else
             m_pixmap = new QPixmap( *img );
-        delete img;
-        // free internal bitmap immediately
-        freeInternalBitmap();
     }
+
+    // destroy the shared descriptor and ### unload underlying xpdf bitmap
+    delete img;
+    SplashOutputDev::startPage( 0, NULL );
 }
 
 void KPDFOutputDev::drawLink( Link * link, Catalog * catalog )

kpdf/QOutputDev.h

@@ -49,8 +49,6 @@ class KPDFOutputDev : public SplashOutputDev
         // @param qtThreadSafety: use a slow QImage and conversions (for threads only)
         void setParams( int pixmapWidth, int pixmapHeight, bool generateTextPage,
                         bool decodeLinks, bool decodeImages, bool qtThreadSafety = false );
-        bool generateTextPage() const;
-        bool generateRects() const;
 
         // generate a valid KPDFLink subclass (or null) from a xpdf's LinkAction
         KPDFLink * generateLink( LinkAction * );
@@ -61,9 +59,6 @@ class KPDFOutputDev : public SplashOutputDev
         TextPage * takeTextPage();
         QValueList< KPDFPageRect * > takeRects();
 
-        // free internal memory used by SplashOutputDev
-        void freeInternalBitmap();
-
         /** inherited from OutputDev */
         // Start a page.
         virtual void startPage(int pageNum, GfxState *state);

kpdf/TODO

@@ -12,7 +12,6 @@ Things to do in order to merge in HEAD (first item has highest priority):
  and rename internals too (document->kpdfdocument, page->kpdfpage, etc..)) ]
 
 IN PROGRESS on the branch (first item comes first):
--> fix megaLock bug in PDF Threaded Pixmap Generator
 -> sync Memory Management (in Document) with the Generator (exp. undoing requests)
 -> add preloading
 -> FIX: viewport changes the right way when clicking links and TOC items (also
@@ -20,6 +19,7 @@ IN PROGRESS on the branch (first item comes first):
    it inside the synopsis too. [60% done]
 
 More items (first items will enter 'In progress list' first):
+-> REGRESSION: find-as-you-type somewhat broken here
 -> move toolbar view actions in the PageView instead of the part. maybe.. or not...
 -> link thumbnails view with document [first, the 'Viewport' must be defined]
 -> usability: layout 2PPV [1 2,3 4,5 6] -> [1,2 3,4 5]. add option for 'ebook' style alignment. (by Mikolaj)

kpdf/conf/dlgperformance.ui

@@ -8,8 +8,8 @@
         <rect>
             <x>0</x>
             <y>0</y>
-            <width>250</width>
-            <height>192</height>
+            <width>252</width>
+            <height>218</height>
         </rect>
     </property>
     <vbox>
@@ -102,7 +102,15 @@
                         <cstring>kcfg_DisableCompositing</cstring>
                     </property>
                     <property name="text">
-                        <string>Disable &amp;transparency (see help)</string>
+                        <string>Disable &amp;transparency effects</string>
+                    </property>
+                </widget>
+                <widget class="QCheckBox">
+                    <property name="name">
+                        <cstring>kcfg_DisableThreading</cstring>
+                    </property>
+                    <property name="text">
+                        <string>Disable &amp;asyncronous generation</string>
                     </property>
                 </widget>
             </vbox>
@@ -119,8 +127,8 @@
             </property>
             <property name="sizeHint">
                 <size>
-                    <width>20</width>
-                    <height>21</height>
+                    <width>21</width>
+                    <height>20</height>
                 </size>
             </property>
         </spacer>

kpdf/conf/kpdf.kcfg

@@ -86,6 +86,9 @@
   <entry key="DisableCompositing" type="Bool" >
    <default>false</default>
   </entry>
+  <entry key="DisableThreading" type="Bool" >
+   <default>false</default>
+  </entry>
  </group>
  <group name="Temp" >
   <entry key="TempDrawBoundaries" type="Bool" >

kpdf/document.cpp

@@ -241,7 +241,7 @@ bool KPDFDocument::okToPrint() const
 }
 
 
-void KPDFDocument::requestPixmaps( const QValueList< PixmapRequest * > & requests, bool syn )
+void KPDFDocument::requestPixmaps( const QValueList< PixmapRequest * > & requests, bool async )
 {
     if ( !generator )
         return;
@@ -268,7 +268,7 @@ void KPDFDocument::requestPixmaps( const QValueList< PixmapRequest * > & request
         mCleanupMemory( request->id );
 
         // 3. Enqueue to Generator (that takes ownership of request)
-        generator->requestPixmap( request, syn );
+        generator->requestPixmap( request, Settings::disableThreading() ? false : async );
     }
 }
 

kpdf/document.h

@@ -95,7 +95,7 @@ class KPDFDocument : public QObject // only for a private slot..
         bool okToPrint() const;
 
         // perform actions on document / pages
-        void requestPixmaps( const QValueList< PixmapRequest * > & requests, bool syncronous = false );
+        void requestPixmaps( const QValueList< PixmapRequest * > & requests, bool asyncronous );
         void requestTextPage( uint page );
         void setCurrentPage( int page, const QRect & viewport = QRect() );
         void findText( const QString & text = "", bool caseSensitive = false );

kpdf/generator.h

@@ -59,7 +59,7 @@ class Generator : public QObject
 
         // generator core
         virtual bool print( KPrinter& /*printer*/ ) { return false; }
-        virtual void requestPixmap( PixmapRequest * request, bool syncronous = false ) = 0;
+        virtual void requestPixmap( PixmapRequest * request, bool asyncronous ) = 0;
         virtual void requestTextPage( KPDFPage * page ) = 0;
 
         // check configuration and return true if something changed

kpdf/generator_pdf.cpp

@@ -265,7 +265,7 @@ bool PDFGenerator::print( KPrinter& printer )
     }
 }
 
-void PDFGenerator::requestPixmap( PixmapRequest * request, bool syncronous )
+void PDFGenerator::requestPixmap( PixmapRequest * request, bool asyncronous )
 {
     //kdDebug() << "id: " << request->id << " is requesting pixmap for page " << request->page->number() << " [" << request->width << " x " << request->height << "]." << endl;
 
@@ -274,7 +274,7 @@ void PDFGenerator::requestPixmap( PixmapRequest * request, bool syncronous )
     if ( page->hasPixmap( request->id, request->width, request->height ) )
         return;
 
-    if ( syncronous )
+    if ( !asyncronous )
     {
         // compute dpi used to get an image with desired width and height
         double fakeDpiX = request->width * 72.0 / page->width(),
@@ -562,21 +562,23 @@ void PDFGenerator::customEvent( QCustomEvent * event )
     if ( event->type() != TGE_DATAREADY_ID )
         return;
 
-    // take data from the OutputDev
+    // put the requested data to the KPDFPage
     const PixmapRequest * request = generatorThread->currentRequest();
-    int reqId = request->id;
-    int reqPage = request->pageNumber;
-    // generate the QPixmap
-    QImage * outImage = kpdfOutputDev->takeImage();
-    QPixmap * pixmap = new QPixmap( *outImage );
-    delete outImage;
-    kpdfOutputDev->freeInternalBitmap();
+    int reqId = request->id,
+        reqPage = request->pageNumber;
+
+    QImage * outImage = generatorThread->takeImage();
+    TextPage * outTextPage = generatorThread->takeTextPage();
+    QValueList< KPDFPageRect * > outRects = generatorThread->takeRects();
+
+    // QImage -> QPixmap
     // attach data to the Page
-    request->page->setPixmap( request->id, pixmap );
-    if ( kpdfOutputDev->generateTextPage() )
-        request->page->setSearchPage( kpdfOutputDev->takeTextPage() );
-    if ( kpdfOutputDev->generateRects() )
-        request->page->setRects( kpdfOutputDev->takeRects() );
+    request->page->setPixmap( request->id, new QPixmap( *outImage ) );
+    delete outImage;
+    if ( outTextPage )
+        request->page->setSearchPage( outTextPage );
+    if ( !outRects.isEmpty() )
+        request->page->setRects( outRects );
 
     // tell generator that contents has been taken and can unlock mutex
     // note: request will be deleted so we can't access it from here on
@@ -593,21 +595,35 @@ void PDFGenerator::customEvent( QCustomEvent * event )
 
 /** The  PDF Pixmap Generator Thread  **/
 
-PDFPixmapGeneratorThread::PDFPixmapGeneratorThread( PDFGenerator * gen )
-    : m_generator( gen ), m_currentRequest( 0 )
+struct PPGThreadPrivate
 {
+    // reference to main objects
+    PDFGenerator * generator;
+    PixmapRequest * currentRequest;
+
+    // internal temp stored items. don't delete this.
+    QImage * m_image;
+    TextPage * m_textPage;
+    QValueList< KPDFPageRect * > m_rects;
+};
+
+PDFPixmapGeneratorThread::PDFPixmapGeneratorThread( PDFGenerator * gen )
+    : d( new PPGThreadPrivate() )
+{
+    d->generator = gen;
+    d->currentRequest = 0;
 }
 
 PDFPixmapGeneratorThread::~PDFPixmapGeneratorThread()
 {
-    delete m_currentRequest;
+    delete d;
 }
 
 void PDFPixmapGeneratorThread::startGeneration( PixmapRequest * request )
 {
 #ifndef NDEBUG
     // check if a generation is already running
-    if ( m_currentRequest )
+    if ( d->currentRequest )
     {
         kdDebug() << "PDFPixmapGeneratorThread: requesting a pixmap "
                   << "when another is being generated." << endl;
@@ -616,36 +632,33 @@ void PDFPixmapGeneratorThread::startGeneration( PixmapRequest * request )
     }
 
     // check if the mutex is already held
-    if ( m_generator->docLock.locked() )
+    if ( d->generator->docLock.locked() )
     {
         kdDebug() << "PDFPixmapGeneratorThread: requesting a pixmap "
                   << "with the mutex already held." << endl;
         return;
     }
 #endif
-    // [LOCK] start locking XPDF thread unsafe classes
-    m_generator->docLock.lock();
-
     // set generation parameters and run thread
-    m_currentRequest = request;
+    d->currentRequest = request;
     start( QThread::LowestPriority );
 }
 
 const PixmapRequest * PDFPixmapGeneratorThread::currentRequest() const
 {
-    return m_currentRequest;
+    return d->currentRequest;
 }
 
 bool PDFPixmapGeneratorThread::isReady() const
 {
-    return !m_currentRequest;
+    return !d->currentRequest;
 }
 
 void PDFPixmapGeneratorThread::endGeneration()
 {
 #ifndef NDEBUG
     // check if a generation is already running
-    if ( !m_currentRequest )
+    if ( !d->currentRequest )
     {
         kdDebug() << "PDFPixmapGeneratorThread: 'end generation' called "
                   << "but generation was not started." << endl;
@@ -653,11 +666,23 @@ void PDFPixmapGeneratorThread::endGeneration()
     }
 #endif
     // reset internal members preparing for a new generation
-    delete m_currentRequest;
-    m_currentRequest = 0;
+    delete d->currentRequest;
+    d->currentRequest = 0;
+}
 
-    // [UNLOCK] mutex
-    m_generator->docLock.unlock();
+QImage * PDFPixmapGeneratorThread::takeImage() const
+{
+    return d->m_image;
+}
+
+TextPage * PDFPixmapGeneratorThread::takeTextPage() const
+{
+    return d->m_textPage;
+}
+
+QValueList< KPDFPageRect * > PDFPixmapGeneratorThread::takeRects() const
+{
+    return d->m_rects;
 }
 
 void PDFPixmapGeneratorThread::run()
@@ -665,27 +690,40 @@ void PDFPixmapGeneratorThread::run()
 // @see PDFGenerator::requestPixmap( .. ) (and be aware to sync the code)
 {
     // compute dpi used to get an image with desired width and height
-    KPDFPage * page = m_currentRequest->page;
-    int width = m_currentRequest->width,
-        height = m_currentRequest->height;
+    KPDFPage * page = d->currentRequest->page;
+    int width = d->currentRequest->width,
+        height = d->currentRequest->height;
     double fakeDpiX = width * 72.0 / page->width(),
            fakeDpiY = height * 72.0 / page->height();
 
     // setup kpdf output device: text page is generated only if we are at 72dpi.
     // since we can pre-generate the TextPage at the right res.. why not?
-    m_genTextPage = !page->hasSearchPage() &&
+    bool genTextPage = !page->hasSearchPage() &&
                     ( width == page->width() ) &&
                     ( height == page->height() );
 
     // generate links and image rects if rendering pages on pageview
-    m_genPageRects = m_currentRequest->id == PAGEVIEW_ID;
+    bool genPageRects = d->currentRequest->id == PAGEVIEW_ID;
 
-    // set OutputDev parameters and Generate contents
-    m_generator->kpdfOutputDev->setParams( width, height, m_genTextPage,
-                                           m_genPageRects, m_genPageRects, TRUE /*thread safety*/ );
-    m_generator->pdfdoc->displayPage( m_generator->kpdfOutputDev, page->number() + 1,
-                                      fakeDpiX, fakeDpiY, 0, true, m_genPageRects );
+    // 0. LOCK s[tart locking XPDF thread unsafe classes]
+    d->generator->docLock.lock();
+
+    // 1. set OutputDev parameters and Generate contents
+    d->generator->kpdfOutputDev->setParams( width, height, genTextPage,
+                                           genPageRects, genPageRects, TRUE /*thread safety*/ );
+    d->generator->pdfdoc->displayPage( d->generator->kpdfOutputDev, page->number() + 1,
+                                      fakeDpiX, fakeDpiY, 0, true, genPageRects );
+
+    // 2. grab data from the OutputDev and store it locally
+    d->m_image = d->generator->kpdfOutputDev->takeImage();
+    d->m_textPage = d->generator->kpdfOutputDev->takeTextPage();
+    d->m_rects = d->generator->kpdfOutputDev->takeRects();
+
+    //d->generator->kpdfOutputDev->freeInternalBitmap();
+
+    // 3. [UNLOCK] mutex
+    d->generator->docLock.unlock();
 
     // notify the GUI thread that data is pending and can be read
-    QApplication::postEvent( m_generator, new QCustomEvent( TGE_DATAREADY_ID ) );
+    QApplication::postEvent( d->generator, new QCustomEvent( TGE_DATAREADY_ID ) );
 }

kpdf/generator_pdf.h

@@ -21,6 +21,8 @@
 
 class PDFDoc;
 class GList;
+class TextPage;
+class KPDFPageRect;
 class KPDFOutputDev;
 class PDFPixmapGeneratorThread;
 
@@ -53,7 +55,7 @@ class PDFGenerator : public Generator
 
         // [INHERITED] perform actions on document / pages
         bool print( KPrinter& printer );
-        void requestPixmap( PixmapRequest * request, bool syncronous = false );
+        void requestPixmap( PixmapRequest * request, bool asyncronous );
         void requestTextPage( KPDFPage * page );
 
         // [INHERITED] reparse configuration
@@ -62,8 +64,6 @@ class PDFGenerator : public Generator
         // used by the KPDFOutputDev child
         KPDFLinkGoto::Viewport decodeLinkViewport( class GString * namedDest, class LinkDest * dest );
 
-    protected:
-
     private:
         // friend class to access private document related variables
         friend class PDFPixmapGeneratorThread;
@@ -116,15 +116,16 @@ class PDFPixmapGeneratorThread : public QThread
         // end generation
         void endGeneration();
 
+        // methods for getting contents from the GUI thread
+        QImage * takeImage() const;
+        TextPage * takeTextPage() const;
+        QValueList< KPDFPageRect * > takeRects() const;
+
     private:
         // can't be called from the outside (but from startGeneration)
         void run();
 
-        // local members
-        PDFGenerator * m_generator;
-        PixmapRequest * m_currentRequest;
-        bool m_genTextPage;
-        bool m_genPageRects;
+        class PPGThreadPrivate * d;
 };
 
 #endif

kpdf/pageview.cpp

@@ -1345,7 +1345,7 @@ void PageView::slotRequestVisiblePixmaps( int newLeft, int newTop )
 
     // actually request pixmaps
     if ( !requestedPixmaps.isEmpty() )
-        d->document->requestPixmaps( requestedPixmaps, true );
+        d->document->requestPixmaps( requestedPixmaps, true /*ASYNC*/  );
 }
 
 void PageView::slotAutoScoll()

kpdf/presentationwidget.cpp

@@ -472,7 +472,7 @@ void PresentationWidget::slotNextPage()
         {
             QValueList< PixmapRequest * > request;
             request.push_back( new PixmapRequest( PRESENTATION_ID, m_frameIndex, pixW, pixH ) );
-            m_document->requestPixmaps( request, true );
+            m_document->requestPixmaps( request, false );
         }
         else
             generatePage();
@@ -501,7 +501,7 @@ void PresentationWidget::slotPrevPage()
         {
             QValueList< PixmapRequest * > request;
             request.push_back( new PixmapRequest( PRESENTATION_ID, m_frameIndex, pixW, pixH ) );
-            m_document->requestPixmaps( request, true );
+            m_document->requestPixmaps( request, false );
         }
         else
             generatePage();

kpdf/thumbnaillist.cpp

@@ -330,7 +330,7 @@ void ThumbnailList::slotRequestPixmaps( int /*newContentsX*/, int newContentsY )
 
     // actually request pixmaps
     if ( !requestedPixmaps.isEmpty() )
-        m_document->requestPixmaps( requestedPixmaps, false /*use threaded gen*/ );
+        m_document->requestPixmaps( requestedPixmaps, true /*ASYNC*/ );
 }
 //END internal SLOTS