Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux version does not save font sizes and margins #4826

Closed
Fmstrat opened this issue Mar 20, 2019 · 17 comments · Fixed by koreader/koreader-base#873
Closed

Linux version does not save font sizes and margins #4826

Fmstrat opened this issue Mar 20, 2019 · 17 comments · Fixed by koreader/koreader-base#873

Comments

@Fmstrat
Copy link

Fmstrat commented Mar 20, 2019

  • KOReader version: v2019.03.1
  • Device: Ubuntu 18.04 laptop

Issue

When using the bottom menu to set font sizes and margin, the settings are not saved. Restarting the reader resets those settings. The settings show up in the menu where I left them, but the fonts and margins are at their original default.

Steps to reproduce

In Ubuntu 18.04

  • sudo apt install libsdl2-dev
  • sudo apt install ./koreader-2019.03.1-amd64.deb
  • After opening, drag up from the bottom and adjust the font size and margins
  • Close KOReader
  • Open KOReader
  • Note the font size and margins are like they were when the app first started
  • Drag up for menu
  • Note that the settings make it seem like the larger font sizes and margins are selected, even though these settings are not applied.
@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

As an aside, surely there's no dependency on libsdl2-dev?

echo "Depends: libsdl2-2.0-0"

Note that the settings make it seem like the larger font sizes and margins are selected, even though these settings are not applied.

That sounds a whole lot more bothersome than not saved. ;-)

This doesn't seem to be something I can reproduce. This is after closing and reopening the version packaged in the Debubuntu package:

@pazos
Copy link
Member

pazos commented Mar 20, 2019

sudo apt install libsdl2-dev

thanks! that is, in fact, a prerrequisite that I forgot to add to the debian package.

When using the bottom menu to set font sizes and margin, the settings are not saved

I can't reproduce that. I think you are closing the window using the close button.

@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

@pazos But so am I.

@Fmstrat
Copy link
Author

Fmstrat commented Mar 20, 2019

@Frenzie It is, actually, but looks like @pazos pointed that out. Had to debug that one on my own ;)

I can't reproduce that. I think you are closing the window using the close button.

Hrm.. Yes, yes I am. How should I be closing?

@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

Why is libsdl2-dev a prerequisite? That sounds like a bug, not like something to be added to the list of dependencies.

@pazos
Copy link
Member

pazos commented Mar 20, 2019

Why is libsdl2-dev a prerequisite? That sounds like a bug

it seems that libsdl2-2.0-0 is not linked to libSDL2.so unless libsdl2-dev is installed

Without it I get: libSDL2.so: cannot open shared object file: No such file or directory

@Fmstrat
Copy link
Author

Fmstrat commented Mar 20, 2019

I can't reproduce that. I think you are closing the window using the close button.

OK, I can reproduce using the exit button in the menus, too. What's the best way to debug this for you?

@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

Alright, so the bug is that the program only looks for libSDL2.so.

Please don't work around it in the Debian control file. :-)

@pazos
Copy link
Member

pazos commented Mar 20, 2019

OK, I can reproduce using the exit button in the menus, too. What's the best way to debug this for you?

You can get debug messages on your console running koreader -d

@Fmstrat
Copy link
Author

Fmstrat commented Mar 20, 2019

Please see the attached. koreader.log is the first run, and koreader2.log is the second. I did the same thing in each. Open, maximize, change settings.

Search through each for "margins" and you'll see they default to 70 at startup, then I change them to larger later. Hope this helps debug!
koreader.log
koreader2.log

BTW, I also noticed that passwords are logged when using OPDS. Might want to consider masking those, especially if those are getting dumped to logcat in the Android app.

@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

It says CRE WARNING: cached rendering is invalid (style hash mismatch): doing full rendering.

@poire-z could that be responsible?

@poire-z
Copy link
Contributor

poire-z commented Mar 20, 2019

Nope, that's the consequence: the cache was saved with book rendered with the user-set margins. On next load, the margins applied are no more the user-set ones, so saved style hash (with user-set margins) is different from current one (with default margins), so mismatch, so full rendering.

@poire-z
Copy link
Contributor

poire-z commented Mar 20, 2019

Between the 2 runs, you should look at the content of /home/fmstrat/Books/Cory Doctorow - Walkaway.sdr/metadata.epub.lua : which margins are saved in it.
So we know if it's a save issue, or a load issue.

Also, there are tons of

03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/multiswipes.lua  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/multiswipes.lua.old.1  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/multiswipes.lua.old.2  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/lookup_history.lua.old.8  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/lookup_history.lua.old.9  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/wikipedia_history.lua  is invalid, remove.
03/20/19-18:02:56 DEBUG /home/fmstrat/.config/koreader/settings/wikipedia_history.lua.old.1  is invalid, remove.

So, I don't know, is this directory writable with enough freespace?

@Frenzie
Copy link
Member

Frenzie commented Mar 20, 2019

Those are (presumably) some noise that needs fixing, i.e., differentiate between "invalid" and "doesn't exist".

@Fmstrat
Copy link
Author

Fmstrat commented Mar 21, 2019

@Frenzie I don't believe this should be closed. The SDL issue has been fixed but I haven't had a chance to check the cached margin issue.

@Frenzie Frenzie reopened this Mar 21, 2019
@Frenzie
Copy link
Member

Frenzie commented Mar 21, 2019

I didn't, GH did. Apparently it thinks fixes comment means fixes issue too.

Frenzie added a commit to Frenzie/koreader that referenced this issue Mar 21, 2019
See koreader#4826 (comment)

Also some Makefile/CMake sanity/cleanup/style/refactoring.

Includes:

* [build] Move Android ZLIB_LDFLAGS to thirdparty/zlib koreader/koreader-base#870
* [chore] Update CMake to modern style with lowercase commands koreader/koreader-base#872
* [fix] Add util.ffiLoadCandidates to try multiple SDL2 library names koreader/koreader-base#873
* [fix] Return fail status for SDL koreader/koreader-base#874
* [chore] Makefile: declare all phonies koreader/koreader-base#875
* [chore] Remove luajit-clean target koreader/koreader-base#876
* [chore] CMake kpvcrlib convert remaining tabs to four spaces koreader/koreader-base#877
Frenzie added a commit that referenced this issue Mar 21, 2019
See #4826 (comment)

Also some Makefile/CMake sanity/cleanup/style/refactoring.

Includes:

* [build] Move Android ZLIB_LDFLAGS to thirdparty/zlib koreader/koreader-base#870
* [chore] Update CMake to modern style with lowercase commands koreader/koreader-base#872
* [fix] Add util.ffiLoadCandidates to try multiple SDL2 library names koreader/koreader-base#873
* [fix] Return fail status for SDL koreader/koreader-base#874
* [chore] Makefile: declare all phonies koreader/koreader-base#875
* [chore] Remove luajit-clean target koreader/koreader-base#876
* [chore] CMake kpvcrlib convert remaining tabs to four spaces koreader/koreader-base#877
@pazos
Copy link
Member

pazos commented Aug 11, 2019

@Fmstrat: could you still reproduce on latest stable?

@pazos pazos closed this as completed Aug 16, 2019
mwoz123 pushed a commit to mwoz123/koreader that referenced this issue Mar 29, 2020
See koreader#4826 (comment)

Also some Makefile/CMake sanity/cleanup/style/refactoring.

Includes:

* [build] Move Android ZLIB_LDFLAGS to thirdparty/zlib koreader/koreader-base#870
* [chore] Update CMake to modern style with lowercase commands koreader/koreader-base#872
* [fix] Add util.ffiLoadCandidates to try multiple SDL2 library names koreader/koreader-base#873
* [fix] Return fail status for SDL koreader/koreader-base#874
* [chore] Makefile: declare all phonies koreader/koreader-base#875
* [chore] Remove luajit-clean target koreader/koreader-base#876
* [chore] CMake kpvcrlib convert remaining tabs to four spaces koreader/koreader-base#877
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants