wb5/posts/zathura.md

176 lines
8.5 KiB
Markdown
Raw Permalink Normal View History

2023-10-31 19:16:12 -04:00
# 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
2024-04-30 17:29:57 -04:00
~~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`.~~
(Since 0.5.5, `smooth-reload` [is no longer an option,](https://github.com/pwmt/zathura/commit/ef6e7e295c9c046368a3202e8c82efd8b9d24a92)
and it is the default behaviour.)
2023-10-31 19:16:12 -04:00
2024-04-30 17:29:57 -04:00
As of version 0.5.3, my code for flicker-free reload is enabled by default for all users of Zathura.
2023-10-31 19:16:12 -04:00
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.