search

lumina-desktop / lumina

Lumina Desktop Environment
http://lumina-desktop.org
BSD 3-Clause "New" or "Revised" License
531 stars 116 forks source link

Missing LuminaPDF::LRUCache::contains() method causing compilation error #663

Closed maxsteciuk closed 5 years ago

maxsteciuk commented 5 years ago

Fix for https://github.com/lumina-desktop/lumina/issues/662

q5sys commented 5 years ago

I can confirm that this does fix the compilation error. But pdf do not open. May or may not be related to this patch or another issue.

maxsteciuk commented 5 years ago

Separate issue Seems related to optimization and parallelization of page rendering code such as keeping track of pages.size() vs pagesStillLoading counter

q5sys commented 5 years ago

I just wanted to leave it open for Ken to review and make the decision on.

maxsteciuk commented 5 years ago

Ok I think I figured out the rendering part of issue:

  1. The snippet which uses LRUCache seems to never hitting the branch to push QImage into the cache if it is not there and always returning empty QImage()

Renderer::imageHash(int pagenum) { if(!imageCache.contains(pagenum)) { return QImage(); } ... // push to cache // return from cache }

Proposed solution:

Renderer::imageHash(int pagenum) if(not imageCache.contains(pagenum)){ imageCache.push(pagenum, pages[pagenum].render()); return *imageCache.get(pagenum); } else { std::optional cachedImage = imageCache.get(pagenum);

if (cachedImage.has_value()) return cachedImage; else { // in case image in cache is somehow not rendered do we want to re-render it? imageCache.push(pagenum, pages[pagenum].render()); return imageCache.get(pagenum); } }

imageCache.push(pagenum, pages[pagenum].render()); return *imageCache.get(pagenum);

  1. In mainUI.cpp the ShowPage() which Rendered::imageHash() is only invoked when "Slideshow" is invoked and never after page rendering is finished

  2. In paralell mode there seems to be race codition when updating pagesStillLoading counter which drives checking whether rendering is complete and so the finished condition in MainUI::slotPageLoaded(int page) does not get called

P.S.

I was able test this snippet when running lumina-pdf in sequential mode (i.e. commenting out BACKEND->loadMultiThread() branch).

beanpole135 commented 5 years ago

Closing PR. Overcome by events. The poppler backend has already been redone to ensure rapid loading of pages without overloading the page cache.