This is the first post of a 3-part series where I will go into technical details of my summer of code project. I will give a high-level overview about the current state of my written code, show you what’s supposed to work and how you can try that; and what still needs to be done.
Then I will write about some “Gotcha!” moments I have experienced in the past few months and what I could learn from them. Those aren’t in any way ordered and are there to explain why my project turned out like it did - and hopefully even help some future GSoC students along the way.
Enough with the introduction, let’s get into the details…
Current status
Nearly finished!
The review request is up on phabricator and currently in the 4th revision.
By and large there are no more showstoppers present and the code is in mergeable quality. Although there are still a few smaller issues present, I think those could be fixed independently in a later revision.
You are encouraged to test my patch, I’m happy about every feedback I can get!
What’s left to do?
Not much. There are still some rendering bugs in obscure places (e.g. when drawing on the screen while in presentation mode).
I also stumbled upon other bugs (more on that later), but those were out of scope.
Difficulties & Lessons learned
Tracking pixmaps
I’ve described in the examples
how enabling HiDPI support basically works. So at first, I thought it was a
good idea to track down any place where a pixmap is created, increase its size
and set its device pixel ratio to QPaintDevice::devicePixelRatio()
.
This had several shortcomings: First, I had to track down where every single pixmap of the application got created. Secondly, I had to introduce getters and setters or extend existing method signatures, so I knew the DPR value of the current screen in every class a pixmap is generated. This was not only cumbersome, but also broke the ABI (application binary interface) - one mustn’t extend functions of the library with another parameter.
Therefore I had to look for another approach: Instead at setting the DPR directly
after creating, I noticed it was sufficient to do so just before painting the pixmap,
because its value is only used for the layout size. I also filled the pixmap cache
with images enlarged by the factor QApplication::devicePixelRatio()
- this
means that the window can be freely moved between screens with differing scaling
factors while the cache stays valid and the documents are correctly rendered. Also,
the code was much more concise: The only changes needed were in the
PixmapRequest
, where the larger images were automatically requested; and in the rendering code - pageview.cpp, pagepainter.cpp, pageviewannotator.cpp and presentationwidget.cpp.
Scaling icons
Okular includes some app-specific icons, which weren’t yet HiDPI-ready.
So I tried to fix it by generating higher resolution PNGs from the SVG source, and install them via cmake, similar to that:
ecm_install_icons(ICONS
32-actions-tool-base-okular.png
64-actions-tool-base-okular.png
[...]
)
Afterwards I should be able to just load the correct version of the icon via QIcon::fromTheme(QStringLiteral("tool-base-okular"))
, right?
Yes, that works… for simple, square icons. Which the annotation icons weren’t.
So David showed me another nifty trick:
If there is a tool-base-okular@2x.png
file next to tool-base-okular.png
,
Qt recognizes this and will automatically load the HiDPI version with the right
scaling factor set when requesting this icon.
But when dealing with QPixmap
s - instead of icons -, one must directly load
this version:
// Load HiDPI variant on HiDPI screen
QString imageVariant;
if ( qApp->devicePixelRatio() > 1.05 ) {
imageVariant = "@2x";
pixmap.load( QStandardPaths::locate(QStandardPaths::GenericDataLocation, QString("okular/pics/tool-base-okular" + imageVariant + ".png") ) );
Complexity
As already mentioned in my previous post the rendering code is really complex and, in my opinion, difficult to understand in the beginning.
Besides the many different codepaths - depending on the availability of a tile manager, the need for for back buffering or the cases where images are not yet ready - some of the code is also already showing symptom of old age.
It was written for Qt 3.3, and then ported over to newer versions. Nowadays some functions could be much simpler and cleaner. Whenever possible, I tried to modernize the code, but a complete overhaul was out of scope. Thankfully, I was not the only one trying to solve this technical debt though.
Chasing ghosts
A few weeks ago, I was at a state where I could open PDF documents, zoom into them and the rendering worked. I tried to open a PNG images, and that behaved also as it should have. I was pleased with myself.
But for testing, I wanted to make sure every filetype worked. And they all did reasonably well - until I got to multipage TIFF documents.
Those were completely broken, shown in a wrong aspect ratio and generally bahaving quite strange. I was puzzled, because everything else up to that point had worked so well.
I quickly checked the Okular release included in my distribution, noticed that the image got rendered there correctly and then tried to find the bug in my code. If I had been a bit more careful at this step, I could have saved myself several hours of scratching my head and wondering what the hell I got wrong there.
Because, as I noticed after those frustrating hours, my distribution rendered the image using the general “Image Backend”, not the specialized one for TIFFs. As I’ve already checked before the “Image Backend” was working fine, but the TIFF generator had an already known bug - it looks like this hasn’t worked since 2006.
Fractional scaling
As already mentioned, getting fractional scaling correct is not easy. I had to fix many off-by-one and rounding errors and until the rendering finally looked good.
Development setup
For development, I set up a new user account and built the workspace, frameworks
and applications with kdesrc-build
. But this also had one downside: I once spent
many hours narrowing down a crash of the application for seemingly random reasons
when calling QAction::setEnabled
. Turns out there was a bug in the master branch
of KXmlGui
.
A good tip for future students is to get acquaint with gdb
soon, at least I had
to use it heavily.
My setup also included two screens, which also meant a great deal of configuration hassle.
Side trip to the Qt source code
Sometimes, I also had to look directly into the source of the Qt Framework to better understand some things. Luckily, I had compiled it myself, and therefore this was easy.
On one such instance I had to check that the copy constructor of QPixmap
s
doesn’t perform a deep copy of the whole pixmap data after changing some data.
The setDevicePixelRatio
method does call detach()
when changing the value,
but luckily the underlying QPlatformPixmap
stays the same.
Testing
I set up a test plan to check that i didn’t break things along the way.
In hindsight, this was a clever thing to do at the start, because it also allowed me to see my progress - which functionality already worked and what still needed to be changed.
You made it to the end of the post, here is a cake for you! If you’re still reading, the future of Okular has to matter for you. Therefore I would be happy if you could try out my patch and give me a quick feedback how it works for you.