ahrm / sioyek

Sioyek is a PDF viewer with a focus on textbooks and research papers
https://sioyek.info/
GNU General Public License v3.0
7.22k stars 236 forks source link

sioyek 2.0: Computer crashes when trying to jump to another chapter #539

Closed BonfireAtNight closed 1 year ago

BonfireAtNight commented 1 year ago

First of all, thanks once more for the amazing work you are doing on Sioyek! It's getting better and better and the new 2.0 release brings some cool new features and changes!

I'm experiencing some strange issue, though. When I open the table of content (t) and choose a chapter to jump to RAM usage suddenly goes through the roof. The computer becomes mostly unusable and I have to reboot. This happens with every PDF file I tried. On the command line I saw that no such column: nan is the last message.

This is my prefs_user.config:

# Size of the UI font
font_size 20
# Size of the GUI elements at the bottom and top of the screen
status_bar_font_size 20

# Synctex
inverse_search_command  nvr --remote-silent %1 -c %2

search_url_g https://www.google.com/search?q=

page_separator_width 1
page_separator_color 0.5 0.5 0.5

# Necessary so that Sioyek opens a new window for a newly opened file
# Otherwise the existing instance would be closed and replaced by new file
should_launch_new_window 1
should_launch_new_instance 1

# The confusing original keybinds are set by default for backward compatibility
use_legacy_keybinds 0

I'm using the i3 window manager and suspect it might have something to do with it. Does anyone have an idea what might cause the issue?

ahrm commented 1 year ago

Can't reproduce the issue. I even tested it on a linux system with i3 and with a 4000 page document with more than 10000 table of contents entries.

Can you share one of the documents which is causing you problems?

ahrm commented 1 year ago

Unfortunately could not reproduce even with the documents. Why do you think i3 might be the issue, does the problem happen with other WMs?

BonfireAtNight commented 1 year ago

Thanks for your effort!

Actually, the AUR package might be at fault. Today I tried the AppImage and the table of content functionality works just fine. In fact, even the user interface looks changed (it has a nice-looking bar in the beginning). I'm not sure if this is the result of some setting of the AppImage or if that is the new standard for Sioyek 2.0 (it would make me wonder why my AUR built doesn't have it). I think I'll work with Appimages for the time being.

Incidentally (sorry for issue hijack), why is Setting $XDG_CONFIG_HOME to /home/username/sioyek-2/Sioyek-x86_64.AppImage.config the default when running the AppImage (well, at least it is in my case)? Shouldn't it default to /home/username/.config/sioyek-2/Sioyek-x86_64.AppImage.config? Can I define that the AppImage uses ~/.config/sioyek/keys_user.config, ~/.config/sioyek/prefs_user.config and ~/.local/share/sioyek/shared.db?

Edit: Thinking about this, could it be that a database file was corrupted or somehow incompatible to how Sioyek 2.0 functions? The AppImage doesn't load any of the databases I had for Sioyek before the 2.0 release. This would also fit with the SQL error cited above.

ahrm commented 1 year ago

The appimage should also be using ~/.config/sioyek/* by default. You can put your shared and prefs files there. there is also a shared_database_path option that allows you to specify the path of the shared database file.

Thinking about this, could it be that a database file was corrupted or somehow incompatible to how Sioyek 2.0 functions?

It could be corruption, but not an incompatibility, because we have not changed the database.

The AppImage doesn't load any of the databases I had for Sioyek before the 2.0 release.

How do you try to use the previous database file with the new appimage (and what error do you get?)

mschwld commented 1 year ago

Actually, the AUR package might be at fault. Today I tried the AppImage and the table of content functionality works just fine. [...] could it be that a database file was corrupted or somehow incompatible to how Sioyek 2.0 functions?

I also experienced some issues with 2.0.0 (using bspwm). Various documents didn't load and would only give me a blank page, TOC didn't work and Sioyek randomly crashed, often alongside the error message no such column: nan. None of the mentioned problems occur when using the Appimage instead of the AUR package, so there might be something wrong with it. My shared database file also seems to work fine with the Appimage.

ahrm commented 1 year ago

It might be a good idea to leave a comment in the AUR package. I can't do it myself because apparently registering in the website requires a captcha which involves pasting a terminal commands which requires pacman which I don't have (because I don't use arch).

I genuinely want to know what type of crack archlinux developers are smoking to think that was a good idea.

BonfireAtNight commented 1 year ago

I wrote a comment for the AUR package. Until now I thought that AUR was a convenient way to quickly get packages not (yet) included with the core repositories. Well, I guess there is a price to quick convenience.

raffaem commented 1 year ago

I wrote a comment for the AUR package. Until now I thought that AUR was a convenient way to quickly get packages not (yet) included with the core repositories. Well, I guess there is a price to quick convenience.

Can you post sioyek's stdout when you open a file?

goggle commented 1 year ago

I'm the maintainer of that AUR package. If anyone figures out what I need to change in the PKGBUILD, please let me know.

BonfireAtNight commented 1 year ago

The appimage should also be using ~/.config/sioyek/* by default. You can put your shared and prefs files there. there is also a shared_database_path option that allows you to specify the path of the shared database file.

Oh, the AppImage does load the '~/.configfiles. I think I was mislead by theSetting $XDG_CONFIG_HOME to /home/username/sioyek-2/Sioyek-x86_64.AppImage.config`, which I guess I took to say that configurations would be in that directory. I realize now that this is not what it's saying.

How do you try to use the previous database file with the new appimage (and what error do you get?)

I defined the shared_database_path value and now everything works fine. Thanks!

Incidentally, do you prefer ~/.locale/share/Sioyek or ~/.locale/share/sioyek? I somehow have both directories. Previously, I had been using the latter, but I think the former is what newer releases use?

In any case, thanks a lot for your help! I'm happy with the AppImage now.

ahrm commented 1 year ago

I think we use ~/.locale/share/sioyek now.

BonfireAtNight commented 1 year ago

The issues experienced here might be indicative of a more general problem with Pacman. I don't understand these things well enough to assess the scope of the problem, but I thought I would mention this as (part of) the reason why the AUR package is acting up (the PKGBUILD file might be fine).

In case you are interested, the problem is discussed in a recent video from YouTube's DistroTube channel. I think it's fair to say that Arch users should revert to the AppImage, at least for the time being.

xulongwu4 commented 1 year ago

I am wondering whether this is caused by the mupdf version on arch linux being different than that expected by sioyek. Currently the arch linux's mupdf library is at version 1.21.1 while sioyek uses mupdf 1.20.0.

linwaytin commented 1 year ago

I use the AUR version, and have the same issue.

asakura42 commented 1 year ago

@ahrm Why not to change page fetching from TOC as regular gg works? I have such problems (I use Arch btw), but page jumping works perfectly. Is it hard to make such fix?

ahrm commented 1 year ago

I think the problem is that the mupdf function that parses table of content entries takes different number of arguments in the mupdf used in sioyek vs the mupdf linked in archlinux. Which creates nasty bugs and crashes when it is called. So the fix is not as simple as you are suggesting. You either have to link the correct version of mupdf or create a patch for sioyek or use for example this patch: https://github.com/ahrm/sioyek/pull/831 . (it will be included in the next version of sioyek)

asakura42 commented 1 year ago

I installed sioyek-git package which includes this patch. Also PKGBUILD has some additional lines:

sed -i 's/-lmupdf-threads/-lfreetype -lgumbo -ljbig2dec -lopenjp2 -ljpeg/' pdf_viewer_build_config.pro
sed -i '/#define LINUX_STANDARD_PATHS/s/\/\///' pdf_viewer/main.cpp
sed -i 's/-lmupdf-third//' pdf_viewer_build_config.pro

I don't know if it breaks TOC function or not, but latest version of PKGBUILD was pushed 10 hours ago and should be up-to-date.

Maybe kind maintainers may help with it? @goggle @hrdl-github

hrdl-github commented 1 year ago

The line that mentions LINUX_STANDARD_PATHS has been obsolete for a long time. I'll remove it next time I update the AUR sioyek-git packages. The TOC function works fine for me using AUR's sioyek-git.

asakura42 commented 1 year ago

@hrdl-github Sadly, but even after full system upgrade and rebuilding sioyek-git, TOC still doesn't work. All goes blank and that's all.

hrdl-github commented 1 year ago

Could you share one of the documents that causes this and possibly the output of pacman -Q; readelf -d /usr/bin/sioyek /usr/lib/libmupdf.so as well as ~/.config/sioyek/*.config ~/.local/share/sioyek/auto.config, @asakura42?

asakura42 commented 1 year ago

@hrdl-github

PDF: https://annas-archive.gs/md5/6834d7936fce8c766a6fb12b175cb644 (but basically any PDF with TOC)

pacman -Q; readelf -d /usr/bin/sioyek /usr/lib/libmupdf.so: https://c-v.sh/ramstamlichis.txt

~/.config/sioyek/*.config are empty

~/.local/share/sioyek/auto.config: https://c-v.sh/stringentmainstreams.txt

hrdl-github commented 1 year ago

With mupdf 1.23.4 (I haven't tested other versions) this document creates TOC nodes that have intra-page offsets x=y=nan. I would add some checks to Document::convert_toc_tree. What default offsets would you use, @ahrm ?

ahrm commented 1 year ago

Probably 0 for offset_x and the offset_y of that page.

asakura42 commented 1 year ago

@hrdl-github patch from your PR works! Thank you. Finally the last piece of my frustration went away.