8.5 KiB
contributing to zathura as a novice
2023-10-24
Earlier this year I made a patch for Zathura, the open-source PDF reader. In this post, I'll document the journey of how I diagnosed the root cause of a bug in Zathura, then wrote a bugfix for it. As this is my first "real" patch that got accepted, the most important take-away for me is that it's actually not that hard to contribute to open-source.
the bug
First of all, as seen in my last blog post, I make use of Zathura extensively in my note-taking setup. On one half of my window, I write markup in Neovim, and in the other half Zathura displays the rendered result. Every time I save the markup, it is automatically re-compiled, then Zathura reloads the document. Normally, this works really well, but if you're reloading documents often, you'll notice that there's flicker every time:
Of course, this is not a deal-breaking issue. However, it is definitely annoying if you save often, and even more so if your editor automatically saves on change.
Upon finding this bug, I did what any normal person does and searched online to see if there was any fixes for it. I stumbled upon this GitLab issue: Prevent flickering when reloading document. Unfortunately for me, the thread was nearly empty, and the last comment was from another user seeking a solution.
Another thread on Stack Exchange was also a dead end, with no real answer to the problem.
At this point, there was clearly no easy solution to remove flicker from Zathura. Since this software is open source, I decided to take a look at the source code in an attempt to diagnose the issue.
investigating source code
Zathura's source code is available at their git repository. I cloned it to my projects folder:
git clone https://git.pwmt.org/pwmt/zathura
At the time, the latest commit was c7baa2f6
("Make icon visible on dark backgrounds").
If you want to follow along with the exact same code as I saw, run this command:
git checkout c7baa2f6
At first glance, it definitely can seem daunting to navigate an unfamiliar codebase. However, these days, we have language servers, which makes it much faster. Essentially, language servers are plugins for text editors that give language "intelligence". With them, you can, for example, jump to the definition of a variable, or get completions when writing code. I find that language servers are useful especially in situations like these, where you might have no idea where a symbol comes from. For C, which Zathura is written in, I use clangd, which integrates easily with Neovim.
A necessary step to use clangd is to have a compile_commands.json
file.
Thankfully, with Zathura's build system, Meson, this is automatically generated during the build.
I followed the build instructions at the bottom of the README:
meson build
cd build
ninja
Then, I copied the resulting file back into the root directory.
cp compile_commands.json ../
Now, Neovim will automatically have language intelligence.
Still, we have no idea where the code responsible for the bug lies.
Considering the bug happens upon reload, it would make sense to search for code related to reloading documents.
To do this, I searched for the term "reload" using ripgrep (you could use git grep
too):
rg "reload"
From here, I saw an interesting function signature in zathura/shortcuts.c
:
sc_reload(girara_session_t* session, girara_argument_t* UNUSED(argument)
This looks like it could be the code responsible for reloading. Looking at it, this is the function body (heavily abridged):
bool
sc_reload()
{
/* close current document */
document_close(zathura, true);
/* reopen document */
document_open();
}
Surprisingly, reloading a document is implemented by closing and reopening the document, rather than a separate in-place refresh feature. This would explain why the screen flickers during reloads: the document disappears entirely before the new one is loaded in its place. It seems like there might be a moment between closing and reopening where there is nothing on screen, which is what causes jarring flicker.
fixing the bug
So now, we've successfully diagnosed the root cause of the flickering. However, this provides no clear path towards solving it.
At this point, I decided to check out the document_close
and document_open
functions mentioned above.
After studying these two functions, something caught my eye:
during opening, we create zathura->pages
, and during closing, we free it.
Looking at the definition, zathura->pages
is an array of pointers to GtkWidgets.
As an experiment, I commented out a bunch of code from document_close
to see what happens if the pages are not freed.
In my head, I thought it might leave the old pages on screen and let the new ones replace it.
Obviously, that didn't happen, and I got multiple segmentation faults, crashes and memory leaks (womp womp).
Now, I was practically at a dead end again.
However, I remembered something the author of the Stack Exchange thread tried regarding this issue:
they attempted to set render-loading
to false to fix the flicker.
On default settings, when the document is reloaded, a "Loading..." prompt flickers on screen:
Turning it off just makes the flickering a blank screen.
I decided to look for code related to this prompt with rg "loading"
.
Doing that, I found an interesting function in zathura/page-widget.c
called zathura_page_widget_draw
.
Skimming the code, it looks like it handles drawing pages to screen.
This was a promising new direction to look into.
Near the end, there is a section that takes care of drawing the loading screen. Reading above, we see that the loading screen only renders if the page has no Cairo surface. Essentially, if the page hasn't fully loaded, put up a loading screen.
Here, I had an idea: what if we replace the loading screen with the original page from before the reload? That way, it would be a seamless transition from the original document to the new document.
To implement this, I added two extra pointers to the zathura
struct:
the predecessor_document
, and the predecessor_pages
.
When Zathura closes the document for a reload, it preserves the current document and pages as "predecessors".
Zathura will not free the predecessor document and pages immediately.
Then, in zathura_page_widget_draw
, instead of drawing a loading screen, it will draw the predecessor pages.
Since having an extra buffer also uses more memory, I added a toggle option smooth-reload
that switches this feature on and off.
Of course, I'm skimming over many specific details here. To see the exact code of my patch, you can look at the commit on Zathura's git repository.
Anyways, here is the end result of the bug-fix, where the bottom window is patched and the top one isn't:
I was really ecstatic when Zathura first smoothly reloaded a document. It finally worked! After this initial success, I collected all these changes into a merge request on GitLab. Finally, after a month of waiting, I got the maintainer to merge my patch. All in all, it took a weekend in total to get familiar with the codebase and create this patch.
conclusion
At the time of writing, this patch is still only available in the bleeding-edge/git build of Zathura.
To check that my code has made it into your version of Zathura, you can check in the man page
(Since 0.5.5, zathurarc(5)
for the option smooth-reload
.smooth-reload
is no longer an option,
and it is the default behaviour.)
As of version 0.5.3, my code for flicker-free reload is enabled by default for all users of Zathura. Hopefully, this small contribution will improve the experience of future users of Zathura. Again, what I want to show in this blog post is that contributing to open-source is actually not that daunting. When you encounter odd behaviour in your favourite software, or you want improve a feature, do not be afraid to just open it up and start tinkering.