posts/zathura.md: added
This commit is contained in:
parent
28d872435d
commit
ef8d87b716
172
posts/zathura.md
Normal file
172
posts/zathura.md
Normal file
@ -0,0 +1,172 @@
|
||||
# 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.
|
BIN
public/img/zathura/before-after-thumb.gif
Normal file
BIN
public/img/zathura/before-after-thumb.gif
Normal file
Binary file not shown.
After Width: | Height: | Size: 180 KiB |
BIN
public/img/zathura/before-after.mp4
Normal file
BIN
public/img/zathura/before-after.mp4
Normal file
Binary file not shown.
BIN
public/img/zathura/flicker-thumb.gif
Normal file
BIN
public/img/zathura/flicker-thumb.gif
Normal file
Binary file not shown.
After Width: | Height: | Size: 187 KiB |
BIN
public/img/zathura/flicker.mp4
Normal file
BIN
public/img/zathura/flicker.mp4
Normal file
Binary file not shown.
BIN
public/img/zathura/loading.jpg
Normal file
BIN
public/img/zathura/loading.jpg
Normal file
Binary file not shown.
After Width: | Height: | Size: 2.4 KiB |
Loading…
Reference in New Issue
Block a user