diff --git a/TODO b/TODO index 653152cec..e6fae4c0d 100644 --- a/TODO +++ b/TODO @@ -11,27 +11,35 @@ Status: next steps: empty the in-progress list and keep it empty until release. In progress: --> memory: check for alloc/dealloc dynamic loops --> new word highlighting for searches / other highlights (on paper - enrico) +-> new word highlighting for searches / other highlights -More items (first items will enter 'In progress list' first): +High priority 3.4 features (deadline is 2k5-Feb-2): -> display current page / total pages (with analog indicator too (progressbar/...)) maybe this can be done on a small widget at the top of the toolbox, displaying 'document' informations (pages, current pg, some metadata, etc..) Tested a 16px ktoolbar in the left-bottom corner.. looks goos and can be used to insert some actions that aren't so useful in the main (and bigger) toolbar --> c00l: add scrollbar marks for bookmarks (like kate) +-> implement history (for actionNamed and restoring previous viewports on navigation) +-> rmb popup for bookmarking pages on thumbnailslist, showing bkmarked pages and + go to next/previous bookmark actions (support for showing bookmarked pages + is already in, but disabled) +-> evaluate wether to change F9 as the default presentation shortcut + +More items (first items will enter 'In progress list' first): +-> presentation: provide a pageX/totalPages indicator in addition to the circle one +-> add scrollbar marks for bookmarks (like kate) -> google search in the page -> cleanup code and update README.png -> find-as-you-type: use shortcut for 'find next' action (not the default one) -> show Viewport in ThumbnailsList (blended/contour) +-> history as a toolbox child (collecting Doc's viewport changes notifications) -> Delay TOC (DocumentSynapsis) generation (and move it on thread) -> refactor ThumbnailsList to do internal rendering as pageview does (way faster than using QScrollView + inserted Widgets and saves 8% on document loading) -> move toolbar view actions in the PageView instead of the part. maybe.. or not... --> usability: layout 2PPV [1 2,3 4,5 6] -> [1,2 3,4 5]. add option for 'ebook' style alignment. (by Mikolaj) --> usability: trigger redraw on 'filter text' on current page (by Mikolaj) +-> usability: layout 2PPV [1 2,3 4,5 6] -> [1,2 3,4 5]. add option for 'ebook' style alignment +-> usability: trigger redraw on 'filter text' on current page -> abstract TextPage generation (the last xpdf dependant class!). then go dancing in the streets. -> better boomark rendering (tested a 'clip overlay' but looks bad actually) @@ -59,8 +67,6 @@ More items (first items will enter 'In progress list' first): -> export all text in plain_text/html -> extract(export?) images (have a look at ImageOutputDev.cc and pdfimages.cc from xpdf (not in our xpdf sources)) -> text selection in wordprocessor style (very hard/impossible) --> implement history (mainly for actionNamed) --> history as a toolbox child (collecting Doc's viewport changes notifications) -> zoom: fit text (with configurable margin) -> open gzipped (.pdf.gz?) files -> kttsd output with menu entries. speech{document/page/selection}. (patch available - enrico) @@ -85,6 +91,8 @@ More items (first items will enter 'In progress list' first): -> export: export to other formats keeping formatting (a dream.. except for PNG :-) Done (newest features come first): +-> FIX: various in memory unallocator, preload with single pages, pageview +-> FIX: optimized pageView (removed 1 waster req on start, lowered reqs) -> FIX: memory unloading order and hard swap avoiding -> CHG: open and open-recent buttons unified in Shell -> CHG: lens icon for the find-ahead messages diff --git a/core/document.cpp b/core/document.cpp index 16cdd70f5..033060c12 100644 --- a/core/document.cpp +++ b/core/document.cpp @@ -81,10 +81,8 @@ KPDFDocument::KPDFDocument() { d->searchPage = -1; d->allocatedPixmapsTotalMemory = 0; - d->memCheckTimer = new QTimer( this ); - connect( d->memCheckTimer, SIGNAL( timeout() ), this, SLOT( slotCheckMemory() ) ); - d->saveBookmarksTimer = new QTimer( this ); - connect( d->saveBookmarksTimer, SIGNAL( timeout() ), this, SLOT( saveDocumentInfo() ) ); + d->memCheckTimer = 0; + d->saveBookmarksTimer = 0; } KPDFDocument::~KPDFDocument() @@ -152,9 +150,19 @@ bool KPDFDocument::openDocument( const QString & docFile ) setViewport( loadedViewport ); // start bookmark saver timer + if ( !d->saveBookmarksTimer ) + { + d->saveBookmarksTimer = new QTimer( this ); + connect( d->saveBookmarksTimer, SIGNAL( timeout() ), this, SLOT( saveDocumentInfo() ) ); + } d->saveBookmarksTimer->start( 5 * 60 * 1000 ); // start memory check timer + if ( !d->memCheckTimer ) + { + d->memCheckTimer = new QTimer( this ); + connect( d->memCheckTimer, SIGNAL( timeout() ), this, SLOT( slotTimedMemoryCheck() ) ); + } d->memCheckTimer->start( 2000 ); return true; @@ -166,8 +174,11 @@ void KPDFDocument::closeDocument() if ( generator && pages_vector.size() > 0 ) saveDocumentInfo(); - // stop memory check timer - d->memCheckTimer->stop(); + // stop timers + if ( d->memCheckTimer ) + d->memCheckTimer->stop(); + if ( d->saveBookmarksTimer ) + d->saveBookmarksTimer->stop(); // delete contents generator delete generator; @@ -190,7 +201,7 @@ void KPDFDocument::closeDocument() delete *pIt; pages_vector.clear(); - // clear memory management data + // clear memory allocation descriptors QValueList< AllocatedPixmap * >::iterator aIt = d->allocatedPixmapsFifo.begin(); QValueList< AllocatedPixmap * >::iterator aEnd = d->allocatedPixmapsFifo.end(); for ( ; aIt != aEnd; ++aIt ) @@ -235,12 +246,27 @@ void KPDFDocument::reparseConfig() // reparse generator config and if something changed clear KPDFPages if ( generator && generator->reparseConfig() ) { - // invalidate pixmaps and send reload signals to observers + // invalidate pixmaps QValueVector::iterator it = pages_vector.begin(), end = pages_vector.end(); for ( ; it != end; ++it ) (*it)->deletePixmapsAndRects(); + + // [MEM] remove allocation descriptors + QValueList< AllocatedPixmap * >::iterator aIt = d->allocatedPixmapsFifo.begin(); + QValueList< AllocatedPixmap * >::iterator aEnd = d->allocatedPixmapsFifo.end(); + for ( ; aIt != aEnd; ++aIt ) + delete *aIt; + d->allocatedPixmapsFifo.clear(); + d->allocatedPixmapsTotalMemory = 0; + + // send reload signals to observers foreachObserver( notifyContentsCleared( DocumentObserver::Pixmap) ); } + + // free memory if in 'low' profile + if ( Settings::memoryLevel() == Settings::EnumMemoryLevel::Low && + !d->allocatedPixmapsFifo.isEmpty() && !pages_vector.isEmpty() ) + cleanupPixmapMemory(); } @@ -707,15 +733,15 @@ void KPDFDocument::sendGeneratorRequest() // [MEM] preventive memory freeing int pixmapBytes = 4 * request->width * request->height; if ( pixmapBytes > (1024 * 1024) ) - cleanupMemory( pixmapBytes ); + cleanupPixmapMemory( pixmapBytes ); // submit the request to the generator generator->generatePixmap( request ); } -void KPDFDocument::cleanupMemory( int /*freeOffset*/ ) +void KPDFDocument::cleanupPixmapMemory( int /*bytesOffset*/ ) { - // choose memory parameters based on configuration profile + // [MEM] choose memory parameters based on configuration profile int clipValue = -1; int memoryToFree = -1; switch ( Settings::memoryLevel() ) @@ -725,50 +751,43 @@ void KPDFDocument::cleanupMemory( int /*freeOffset*/ ) break; case Settings::EnumMemoryLevel::Normal: - memoryToFree = d->allocatedPixmapsTotalMemory - getTotalMemory() / 5; - clipValue = d->allocatedPixmapsTotalMemory - getFreeMemory() / 2; + memoryToFree = d->allocatedPixmapsTotalMemory - getTotalMemory() / 3; + clipValue = (d->allocatedPixmapsTotalMemory - getFreeMemory()) / 2; break; case Settings::EnumMemoryLevel::Aggressive: - clipValue = d->allocatedPixmapsTotalMemory - getFreeMemory() / 2; + clipValue = (d->allocatedPixmapsTotalMemory - getFreeMemory()) / 2; break; } - // p--rintf("T:%d TT:%d FF:%d - c:%d m:%d\n", d->allocatedPixmapsTotalMemory, getTotalMemory(), getFreeMemory(), clipValue, memoryToFree); - if ( clipValue > memoryToFree ) memoryToFree = clipValue; if ( memoryToFree > 0 ) - freeMemory( memoryToFree ); -} - -void KPDFDocument::freeMemory( int bytesToFree ) -{ - //kdDebug() << "Freeing: " << bytesToFree << " bytes" << endl; - // [MEM] free memory starting from older pixmaps - int pagesFreed = 0; - QValueList< AllocatedPixmap * >::iterator pIt = d->allocatedPixmapsFifo.begin(); - QValueList< AllocatedPixmap * >::iterator pEnd = d->allocatedPixmapsFifo.end(); - while ( (pIt != pEnd) && (bytesToFree > 0) ) { - AllocatedPixmap * p = *pIt; - if ( d->observers[ p->id ]->canUnloadPixmap( p->page ) ) + // [MEM] free memory starting from older pixmaps + int pagesFreed = 0; + QValueList< AllocatedPixmap * >::iterator pIt = d->allocatedPixmapsFifo.begin(); + QValueList< AllocatedPixmap * >::iterator pEnd = d->allocatedPixmapsFifo.end(); + while ( (pIt != pEnd) && (memoryToFree > 0) ) { - // update internal variables - pIt = d->allocatedPixmapsFifo.remove( pIt ); - d->allocatedPixmapsTotalMemory -= p->memory; - bytesToFree -= p->memory; - pagesFreed++; - // delete pixmap - pages_vector[ p->page ]->deletePixmap( p->id ); - // delete allocation descriptor - delete p; - } else - ++pIt; + AllocatedPixmap * p = *pIt; + if ( d->observers[ p->id ]->canUnloadPixmap( p->page ) ) + { + // update internal variables + pIt = d->allocatedPixmapsFifo.remove( pIt ); + d->allocatedPixmapsTotalMemory -= p->memory; + memoryToFree -= p->memory; + pagesFreed++; + // delete pixmap + pages_vector[ p->page ]->deletePixmap( p->id ); + // delete allocation descriptor + delete p; + } else + ++pIt; + } + //p--rintf("freeMemory A:[%d -%d = %d] \n", d->allocatedPixmapsFifo.count() + pagesFreed, pagesFreed, d->allocatedPixmapsFifo.count() ); } - - //kdDebug() << "items: " << d->allocatedPixmapsFifo.count() << " [" << (d->allocatedPixmapsTotalMemory/1000) << "] Removed " << pagesFreed << " pages. gap: " << bytesToFree << endl; } int KPDFDocument::getTotalMemory() @@ -1006,12 +1025,12 @@ void KPDFDocument::saveDocumentInfo() const infoFile.close(); } -void KPDFDocument::slotCheckMemory() +void KPDFDocument::slotTimedMemoryCheck() { // [MEM] clean memory (for 'free mem dependant' profiles only) if ( Settings::memoryLevel() != Settings::EnumMemoryLevel::Low && d->allocatedPixmapsTotalMemory > 1024*1024 ) - cleanupMemory(); + cleanupPixmapMemory(); } diff --git a/core/document.h b/core/document.h index e6db72d51..ce33513c2 100644 --- a/core/document.h +++ b/core/document.h @@ -92,8 +92,7 @@ class KPDFDocument : public QObject // only for a private slot.. private: void sendGeneratorRequest(); // memory management related functions - void cleanupMemory( int bytesOffset = 0 ); - void freeMemory( int bytes ); + void cleanupPixmapMemory( int bytesOffset = 0 ); int getTotalMemory(); int getFreeMemory(); // more private functions @@ -108,7 +107,7 @@ class KPDFDocument : public QObject // only for a private slot.. private slots: void saveDocumentInfo() const; - void slotCheckMemory(); + void slotTimedMemoryCheck(); }; diff --git a/core/generator_pdf/generator_pdf.cpp b/core/generator_pdf/generator_pdf.cpp index 717a23172..bf696da7b 100644 --- a/core/generator_pdf/generator_pdf.cpp +++ b/core/generator_pdf/generator_pdf.cpp @@ -39,6 +39,21 @@ // id for DATA_READY PDFPixmapGeneratorThread Event #define TGE_DATAREADY_ID 6969 +/** NOTES on threading: + * internal: thread race prevention is done via the 'docLock' mutex. the + * mutex is needed only because we have the asyncronous thread; else + * the operations are all within the 'gui' thread, scheduled by the + * Qt scheduler and no mutex is needed. + * external: dangerous operations are all locked via mutex internally, and the + * only needed external thing is the 'canGeneratePixmap' method + * that tells if the generator is free (since we don't want an + * internal queue to store PixmapRequests). A generatedPixmap call + * without the 'ready' flag set, results in undefined behavior. + * So, as example, printing while generating a pixmap asyncronously is safe, + * it might only block the gui thread by 1) waiting for the mutex to unlock + * in async thread and 2) doing the 'heavy' print operation. + */ + PDFGenerator::PDFGenerator( KPDFDocument * doc ) : Generator( doc ), pdfdoc( 0 ), kpdfOutputDev( 0 ), ready( true ), pixmapRequest( 0 ), docInfoDirty( true ), docSynopsisDirty( true ) @@ -208,7 +223,7 @@ void PDFGenerator::generatePixmap( PixmapRequest * request ) // be set only to prevent asking a pixmap while the thread is running) ready = false; - // debug message + // debug requests to this (xpdf) generator //kdDebug() << "id: " << request->id << " is requesting " << (request->async ? "ASYNC" : "sync") << " pixmap for page " << request->page->number() << " [" << request->width << " x " << request->height << "]." << endl; /** asyncronous requests (generation in PDFPixmapGeneratorThread::run() **/ diff --git a/ui/pageview.cpp b/ui/pageview.cpp index 788e2e5bf..cc5463143 100644 --- a/ui/pageview.cpp +++ b/ui/pageview.cpp @@ -78,7 +78,8 @@ public: bool typeAheadActivated; QString findString; bool blockViewport; - PageViewMessage * messageWindow; //in pageviewutils.h + bool blockPixmapsRequest; // prevent pixmap requests + PageViewMessage * messageWindow; // in pageviewutils.h // actions KToggleAction * aMouseEdit; @@ -120,6 +121,7 @@ PageView::PageView( QWidget *parent, KPDFDocument *document ) d->scrollIncrement = 0; d->dirtyLayout = false; d->blockViewport = false; + d->blockPixmapsRequest = false; d->messageWindow = new PageViewMessage(this); // widget setup: setup focus, accept drops and track mouse @@ -273,11 +275,11 @@ void PageView::notifyViewportChanged() d->blockViewport = true; // find PageViewItem matching the viewport description - const DocumentViewport & viewport = d->document->viewport(); + const DocumentViewport & vp = d->document->viewport(); PageViewItem * item = 0; QValueVector< PageViewItem * >::iterator iIt = d->items.begin(), iEnd = d->items.end(); for ( ; iIt != iEnd; ++iIt ) - if ( (*iIt)->pageNumber() == viewport.pageNumber ) + if ( (*iIt)->pageNumber() == vp.pageNumber ) { item = *iIt; break; @@ -289,19 +291,21 @@ void PageView::notifyViewportChanged() } // relayout in "Single Pages" mode or if a relayout is pending + d->blockPixmapsRequest = true; if ( !Settings::viewContinous() || d->dirtyLayout ) slotRelayoutPages(); // restore viewport or use default x-center, v-top alignment const QRect & r = item->geometry(); - if ( viewport.reCenter.enabled ) + if ( vp.reCenter.enabled ) { - int xCenter = (int)( viewport.reCenter.normalizedCenterX * (float)r.width() ); - int yCenter = (int)( viewport.reCenter.normalizedCenterY * (float)r.height() ); + int xCenter = (int)( vp.reCenter.normalizedCenterX * (float)r.width() ); + int yCenter = (int)( vp.reCenter.normalizedCenterY * (float)r.height() ); center( r.left() + xCenter, r.top() + yCenter ); } else center( r.left() + r.width() / 2, r.top() + visibleHeight() / 2 - 10 ); + d->blockPixmapsRequest = false; // request visible pixmaps in the current viewport and recompute it slotRequestVisiblePixmaps(); @@ -1339,7 +1343,7 @@ void PageView::slotRelayoutPages() { PageViewItem * item = *iIt; // update internal page size (leaving a little margin in case of Fit* modes) - updateItemSize( item, colWidth[ cIdx ] - 6, viewportHeight - 10 ); + updateItemSize( item, colWidth[ cIdx ] - 6, viewportHeight - 8 ); // find row's maximum height and column's max width if ( item->width() + 6 > colWidth[ cIdx ] ) colWidth[ cIdx ] = item->width() + 6; @@ -1355,7 +1359,7 @@ void PageView::slotRelayoutPages() // 2) arrange widgets inside cells int insertX = 0, - insertY = 10; //TODO take d->zoomFactor into account (2+4*x) + insertY = 4; //TODO take d->zoomFactor into account (2+4*x) cIdx = 0; rIdx = 0; for ( iIt = d->items.begin(); iIt != iEnd; ++iIt ) @@ -1403,12 +1407,12 @@ void PageView::slotRelayoutPages() if ( item == currentItem || (cIdx > 0 && cIdx < nCols) ) { // update internal page size (leaving a little margin in case of Fit* modes) - updateItemSize( item, colWidth[ cIdx ] - 4, viewportHeight - 6 ); + updateItemSize( item, colWidth[ cIdx ] - 6, viewportHeight - 8 ); // find row's maximum height and column's max width - if ( item->width() + 4 > colWidth[ cIdx ] ) - colWidth[ cIdx ] = item->width() + 4; - if ( item->height() + 6 > fullHeight ) - fullHeight = item->height() + 6; + if ( item->width() + 6 > colWidth[ cIdx ] ) + colWidth[ cIdx ] = item->width() + 6; + if ( item->height() + 8 > fullHeight ) + fullHeight = item->height() + 8; cIdx++; } } @@ -1435,7 +1439,9 @@ void PageView::slotRelayoutPages() fullWidth += colWidth[ i ]; delete [] colWidth; - slotRequestVisiblePixmaps(); + + // this can cause a little overhead + slotRequestVisiblePixmaps(); } // 3) reset dirty state @@ -1475,6 +1481,10 @@ void PageView::slotRelayoutPages() void PageView::slotRequestVisiblePixmaps( int newLeft, int newTop ) { + // if requests are blocked (because raised by an unwanted event), exit + if ( d->blockPixmapsRequest ) + return; + // precalc view limits for intersecting with page coords inside the lOOp bool isEvent = newLeft != -1 && newTop != -1 && !d->blockViewport; QRect viewportRect( isEvent ? newLeft : contentsX(), @@ -1543,7 +1553,7 @@ void PageView::slotRequestVisiblePixmaps( int newLeft, int newTop ) { PageViewItem * i = d->items[ headRequest ]; // request the pixmap if not already present - if ( !i->page()->hasPixmap( PAGEVIEW_ID, i->width(), i->height() ) ) + if ( !i->page()->hasPixmap( PAGEVIEW_ID, i->width(), i->height() ) && i->width() > 0 ) requestedPixmaps.push_back( new PixmapRequest( PAGEVIEW_ID, i->pageNumber(), i->width(), i->height(), PAGEVIEW_PRELOAD_PRIO, true ) ); } @@ -1554,7 +1564,7 @@ void PageView::slotRequestVisiblePixmaps( int newLeft, int newTop ) { PageViewItem * i = d->items[ tailRequest ]; // request the pixmap if not already present - if ( !i->page()->hasPixmap( PAGEVIEW_ID, i->width(), i->height() ) ) + if ( !i->page()->hasPixmap( PAGEVIEW_ID, i->width(), i->height() ) && i->width() > 0 ) requestedPixmaps.push_back( new PixmapRequest( PAGEVIEW_ID, i->pageNumber(), i->width(), i->height(), PAGEVIEW_PRELOAD_PRIO, true ) ); }