Port qpageview to PyQt6 #24
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Right now this is just barely able to load and display a document. I still need to fix the page background and scaling logic, as well as a number of crashes in other modules that I was only able to find after getting this working.
The corresponding Poppler code is much more complex; it seems like a lot of that is to handle things like high-DPI screens and printing. Those will need to be addressed eventually, but I'm not sure I understand that code well enough yet to tackle it myself.
This is a substantial rewrite now that I have a better understanding of both QPdfLinkModel and the original Poppler code. Links are now placed in the correct areas on the page. In addition, the Link.url property now returns a QString, as it does under the Poppler backend, rather than a QUrl. So far as I've been able to test, this brings the QtPdf backend's link support up to par with the Poppler backend's.
These have moved from QtWidgets to QtGui in version 6.
This lets us keep the old Poppler code for reference and possible re-use without breaking things on systems with popplerqt5 installed.
Poppler will be used if the (currently hypothetical) popplerqt6 package is available, since the user presumably went out of their way to install it. Otherwise, the QtPdf renderer is selected.
This lets the QtPdf backend load documents exactly the same way the Poppler backend does, without having to know its parent QObject.
This rewrites that method as a wrapper around AbstractRenderer's default implementation with oversampling support added, eliminating a lot of code duplicated from the draw() method. It also fixes the grayscale background bug reported in frescobaldi#29.
The render() method does in fact call draw() since 329f344.
The new version is better commented, and hopefully more intelligible to those like me who have trouble keeping up with all the different coordinate systems in play here. This also moves the oversampling logic into draw() where it has the information it needs to correctly calculate the effective resolution at the current zoom level.
This ensures we get the correct output resolution when printing.
This fixes Frescobaldi's "copy to image" feature at 72dpi.
The definition of oversampleThreshold = 96 goes back to the very first commit of poppler.py back when qpageview was still part of Frescobaldi. No explanation is given, but I'm presuming it's because 96 dpi is the traditional standard pixel density for PC screens. In practice, painter.device().logicalDpi[XY]() seems to return 96 anyway on both standard and high-DPI screens, but based on my understanding of the scaling logic this is more semantically correct.
The threshold is determined by readability, not device capabilities.
Capitalizing the drive letter here fixes Frescobaldi opening a separate copy of the document when using point-and-click. While the actual bug seems to involve code elsewhere assuming case sensitivity, working around it this way is easier and has much less chance of breaking things on platforms that do use case-sensitive paths. Since I was reworking this section anyway, I also rewrote the logic to restore a missing colon after the drive letter to make it easier to understand.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a port of qpageview to PyQt6 as discussed under #17.
The Qt 6 port adds a new PDF rendering backend based on QtPdf (requires Qt 6.6 or higher to support point-and-click). This should help considerably with the packaging issues caused by python-poppler-qt5. I have left the existing Poppler code in place for now in case there is any future need for it.
This port seems pretty stable, but there are probably still some bugs, and I suspect you all may want to keep it on a separate branch until more folks have had a chance to test it. Any suggestions, questions, patches, or other feedback would be welcome.