Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for poppler-qt6 pdf backend #356

Merged
merged 4 commits into from
Nov 20, 2022

Conversation

selmf
Copy link
Member

@selmf selmf commented Nov 13, 2022

This PR adds support for the Poppler Qt6 backend

@jbeich feel free to give it a test, feedback is always welcome.

@jbeich
Copy link
Contributor

jbeich commented Nov 18, 2022

Fails to build on FreeBSD 13.1. Clang is default on FreeBSD, OpenBSD, macOS, Android, OpenMandriva, ChimeraLinux. libc++ is often used together with Clang and generally cannot be mixed with libstdc++ due to conflict between libc++abi (or libcxxrt) and libsupc++. Given YACReader depends on C++-based libraries (Qt6, Poppler) it cannot be built with different C++ library than the default (unless those are bundled).

$ pkg info -x qt6
poppler-qt6-22.11.0_1
qt6-5compat-6.4.1
qt6-base-6.4.1
qt6-declarative-6.4.1
qt6-multimedia-6.4.1
qt6-shadertools-6.4.1
qt6-svg-6.4.1
qt6-tools-6.4.1

Clang/libc++

YACReaderLibrary/initial_comic_info_extractor.cpp:41:25: error: functional-style cast from 'std::unique_ptr<Poppler::Document>' to 'std::unique_ptr<Poppler::Document>' uses deleted function
        auto pdfComic = std::unique_ptr<Poppler::Document>(_pdfComic);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/v1/__memory/unique_ptr.h:121:59: note: candidate constructor (the implicit copy constructor) has been implicitly deleted
class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
                                                          ^

GCC/libc++

YACReaderLibrary/initial_comic_info_extractor.cpp: In member function 'void YACReader::InitialComicInfoExtractor::extract()':
YACReaderLibrary/initial_comic_info_extractor.cpp:41:69: error: use of deleted function 'constexpr std::__1::unique_ptr<Poppler::Document>::unique_ptr(const std::__1::unique_ptr<Poppler::Document>&)'
   41 |         auto pdfComic = std::unique_ptr<Poppler::Document>(_pdfComic);
      |                                                                     ^
In file included from /usr/include/c++/v1/__memory/shared_ptr.h:25,
                 from /usr/include/c++/v1/__functional/function.h:20,
                 from /usr/include/c++/v1/functional:500,
                 from /usr/include/c++/v1/optional:154,
                 from /usr/local/include/qt6/QtCore/qtypeinfo.h:8,
                 from /usr/local/include/qt6/QtCore/qglobal.h:1397,
                 from /usr/local/include/qt6/QtCore/QtCore:4,
                 from /usr/local/include/qt6/QtGui/QtGuiDepends:3,
                 from /usr/local/include/qt6/QtGui/QtGui:3,
                 from ../YACReaderLibrary/initial_comic_info_extractor.h:4,
                 from ../YACReaderLibrary/initial_comic_info_extractor.cpp:1:
/usr/include/c++/v1/__memory/unique_ptr.h:121:59: note: 'constexpr std::__1::unique_ptr<Poppler::Document>::unique_ptr(const std::__1::unique_ptr<Poppler::Document>&)' is implicitly declared as deleted because 'std::__1::unique_ptr<Poppler::Document>' declares a move constructor or move assignment operator
  121 | class _LIBCPP_UNIQUE_PTR_TRIVIAL_ABI _LIBCPP_TEMPLATE_VIS unique_ptr {
      |                                                           ^~~~~~~~~~

@selmf
Copy link
Member Author

selmf commented Nov 18, 2022

Well I am glad I asked you to test this now.

What is happening here is not a compiler or libstdc++ problem but a mistake on my side.

The compile error is caused by the compiler hitting a line it shouldn't. What is missing here is the #ifdef guard and the poppler-qt6 specific logic for the unique pointer, which is different from poppler-qt5.

@selmf selmf force-pushed the std_unique branch 2 times, most recently from 032457a to fad24a5 Compare November 18, 2022 13:04
@selmf
Copy link
Member Author

selmf commented Nov 18, 2022

Ok, I pushed a fix for the problem and I did a test build with clang, qt6 and poppler-qt6 on my Linux machine.

@jbeich
Copy link
Contributor

jbeich commented Nov 18, 2022

725cf65 builds fine with both poppler-qt6 and poppler-qt5. Tested runtime via a PDF version of FreeBSD Journal: dssim for poppler-qt6 (fad24a5) vs. poppler-qt5 (9.10.0) returns 0.00000000 (identical) on a screenshot of a single page.

@luisangelsm
Copy link
Member

@selmf maybe we should add a new job(s) in Azure Pipelines for poppler-qt6 to catch problems like the one @jbeich found (thanks for testing btw).

@luisangelsm
Copy link
Member

@selmf could you rebase this on top of develop please?

@selmf
Copy link
Member Author

selmf commented Nov 20, 2022

@luisangelsm rebase is up.

Adding a test build for poppler-qt6 to catch errors is absolutely something we need to do. The problem here is that Ubuntu does not package poppler-qt6 yet, so I could not add a test to our build pipelines.

@luisangelsm luisangelsm merged commit 6f358e4 into YACReader:develop Nov 20, 2022
@selmf selmf deleted the std_unique branch November 20, 2022 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants