173 lines
8.3 KiB
Markdown
173 lines
8.3 KiB
Markdown
|
# contributing to zathura as a novice
|
||
|
|
||
|
2023-10-24
|
||
|
|
||
|
Earlier this year I made a [patch](https://git.pwmt.org/pwmt/zathura/-/merge_requests/80)
|
||
|
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](../typst-notes/), 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:
|
||
|
|
||
|
[ ![](/public/img/zathura/flicker-thumb.gif) ](/public/img/zathura/flicker.mp4)
|
||
|
|
||
|
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_](https://git.pwmt.org/pwmt/zathura/-/issues/268).
|
||
|
Unfortunately for me, the thread was nearly empty, and the last comment was from another user seeking a solution.
|
||
|
|
||
|
[Another thread](https://superuser.com/questions/1459927/zathura-flicker-when-updating-pdf)
|
||
|
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](https://github.com/clangd/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:
|
||
|
|
||
|
![Zathura loading prompt](/public/img/zathura/loading.jpg)
|
||
|
|
||
|
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](https://git.pwmt.org/pwmt/zathura/-/commit/257a2c968bcf67cf814aeab325800d4889d8df21) 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:
|
||
|
|
||
|
[ ![Before and after the bugfix](/public/img/zathura/before-after-thumb.gif) ](/public/img/zathura/before-after.mp4)
|
||
|
|
||
|
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](https://git.pwmt.org/pwmt/zathura/-/merge_requests/80) 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 `zathurarc(5)` for the option `smooth-reload`.
|
||
|
|
||
|
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.
|