search

lumina-desktop / lumina

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

FreeBSD 12.0-RELEASE: Build crashes at lumina-pdf #662

Closed matias-pizarro closed 5 years ago

matias-pizarro commented 5 years ago

Hello,

I'd appreciate any help I can get with the build error below. Let me know if you require any additional information.

$ cd lumina
$ git log -1
commit 9ca1d49ef773adae2a26758a61c9db3b029d93b1 (HEAD -> master, origin/master, origin/HEAD)
Author: Ken Moore <ken@ixsystems.com>
Date:   Sun Jan 27 16:11:00 2019 -0500

    Make sure the applauncher panel plugin uses the same LFileInfo class as other tools.
$ qmake
Project MESSAGE: Build OS Info: FreeBSD, amd64, FreeBSD 12.0-RELEASE-p1 0d7ca8055f4(feature/lenovo_t480) CUSTOM
Project MESSAGE:  - Detected Qt Version: 5.12.0
Project MESSAGE: Build Settings Loaded: FreeBSD
$ make
... <lots of output> ...
clang++ -c -pipe -std=c++17 -O2 -std=gnu++1z -Wall -W -pthread -fPIC -DPREFIX=QString\(\"/usr/local\"\) -DL_ETCDIR=QString\(\"/usr/local/etc\"\) -DL_SHAREDIR=QString\(\"/usr/local/share\"\) -DGIT_VERSION="\"9ca1d49e\"" -DQT_NO_DEBUG -DQT_PRINTSUPPORT_LIB -DQT_SVG_LIB -DQT_WIDGETS_LIB -DQT_MULTIMEDIA_LIB -DQT_GUI_LIB -DQT_CONCURRENT_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -I. -I../../core/libLumina -I/usr/local/include/qt5 -I/usr/local -I/usr/local/include -I/usr/home/mpizarro/devel/rebost/lumina/src-qt5/desktop-utils/lumina-pdf -I/usr/local/include/qt5/QtPrintSupport -I/usr/local/include/qt5/QtSvg -I/usr/local/include/qt5/QtWidgets -I/usr/local/include/qt5/QtMultimedia -I/usr/local/include/qt5/QtGui -I/usr/local/include/qt5/QtConcurrent -I/usr/local/include/qt5/QtNetwork -I/usr/local/include/qt5/QtCore -I.build/moc -I/usr/local/include -I/usr/local/include/libdrm -I.build/ui -I/usr/local/include -I/usr/local/lib/qt5/mkspecs/freebsd-clang -o .build/obj/Renderer-poppler.o Renderer-poppler.cpp                                                                                   
Renderer-poppler.cpp:163:60: error: no member named 'contains' in 'LuminaPDF::LRUCache<QImage, int>'
bool Renderer::isDoneLoading(int page) { return imageCache.contains(page); }
                                                ~~~~~~~~~~ ^
Renderer-poppler.cpp:181:18: error: no member named 'contains' in 'LuminaPDF::LRUCache<QImage, int>'
  if(!imageCache.contains(pagenum)){ return QSize(); }
      ~~~~~~~~~~ ^
Renderer-poppler.cpp:186:18: error: no member named 'contains' in 'LuminaPDF::LRUCache<QImage, int>'
  if(!imageCache.contains(pagenum)){ return QImage(); }
      ~~~~~~~~~~ ^
3 errors generated.
*** Error code 1

Stop.
make[3]: stopped in /usr/home/mpizarro/devel/rebost/lumina/src-qt5/desktop-utils/lumina-pdf
*** Error code 1

Stop.
make[2]: stopped in /usr/home/mpizarro/devel/rebost/lumina/src-qt5/desktop-utils
*** Error code 1

Stop.
make[1]: stopped in /usr/home/mpizarro/devel/rebost/lumina/src-qt5
*** Error code 1

Stop.
make: stopped in /usr/home/mpizarro/devel/rebost/lumina
maxsteciuk commented 5 years ago

There is no LuminaPDF::LRUCache::contains() There is LuminaPDF::LRUCache::get()

matias-pizarro commented 5 years ago

good catch!

q5sys commented 5 years ago

I can confirm this bug.

stackyjoe commented 5 years ago

Fixing it isn't too bad,

bool contains(const Key key) { return hashmap.find(key) != hashmap.end();}

Is the implementation I meant.

However even with that the build doesn't result in a functional pdf. At least when compiled with Poppler it doesn't actually display the loaded file. I'm looking at the diffs between the last version I had working at the current one but I haven't figured out the exact change yet. I know the code I wrote up wasn't too easy to square with the muPDF option.

stackyjoe commented 5 years ago

The issue is that the function Renderer::isDoneLoading() was changed from reporting on an atomic int pagesStillLoading to being a function of an integer that tells if a page is done loading. To make it work for both Poppler and muPDF it was changed to

Renderer::isDoneLoading(int page) { return imageCache.contains(page); }

Which isn't what you want. The object imageCache is an LRUCache of the QImages, we should be looking at the pages object instead. However changing that doesn't seem to work, since the isDoneLoading() can actually be called before pages is filled with dummy drawablePage objects, leading to dereferencing a null pointer.

stackyjoe commented 5 years ago

After reading more about Qt5, I think the easiest way to get it working again is to revert to before the PR I submitted, and change the PageItem class in printwidget.h to not own a QImage object, but instead own a small wrapper around the mupdf/poppler page. This wrapper only needs to be able to create the image when requested in the PageItem::paint() function.

As far as I can tell from the documentation, QGraphicsScene already only paints visible items to its viewport. So this simple change would fix the memory issues. The PageItem::paint() function could also easily be linked to a LRUCache to improve responsiveness. There would still be some multi-threading issues, but I think that could be fixed separately without anything too difficult.

I'll play around with it in the next couple of days, and report back.

beanpole135 commented 5 years ago

This is already fixed. The lumina-pdf utility has also been split out into it's own repository to make it easier to update on it's own as needed: https://github.com/lumina-desktop/lumina-pdf