Reader: do not apply font size out of range by hius07 · Pull Request #10295 · koreader/koreader

hius07 · 2023-04-03T06:10:13Z

This change is

poire-z · 2023-04-03T09:35:40Z

 end

 function ReaderCoptListener:onConfigChange(option_name, option_value)
+    if option_name == "font_size" and option_value < 12 then return end


I think all these need a comment.
-- the option name font_size is historically and sadly shared by both reader modules, but fortunately their allowed values have different ranges, so we can distringuish them
and here:
if option_name == "font_size" and option_value < 12 then return end -- aimed at Kopt
and below -- aimed at Cre
(@Frenzie for a proper writing with less adjectives maybe ? :)

If they're this simple to distinguish should we migrate them to their own values?

But as for the comment, maybe shortened slightly like this?
-- font_size is historically and sadly shared by both the filemanager and reader module, but fortunately they can be distinguished by their different ranges

Frenzie · 2023-04-03T09:47:56Z


 local default_font_size = 22
-- the koptreader config goes from 0.1 to 3.0, but we want a regular font size
+-- the koptreader config goes from 0.2 to 2.0, but we want a regular font size


Please don't change this comment. I didn't write the values correctly by mistake. ;-)

This is a code comment, not a UI comment.

Sorry, don't understand.

koreader/frontend/ui/data/koptoptions.lua

Line 10 in 827581a

local FONT_SCALE_FACTORS = {0.2, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0, 1.1, 1.3, 1.6, 2.0}

Left 3, right 2

PS Those larger and smaller sizes are accessible through the UI too through larger/smaller. But they're too large and small respectively to eat up valuable UI space.

Sorry, don't understand.

Those are what we consider the useful options in the UI. They don't reflect the technical limits.

poire-z · 2023-04-03T11:37:30Z

 function ReaderKoptListener:onConfigChange(option_name, option_value)
+    -- font_size is historically and sadly shared by both mupdf and cre reader modules,
+    -- but fortunately they can be distinguished by their different ranges
+    if option_name == "font_size" and option_value > 2 then return end


So, this > 2 should be > 3 if technically, kopt (and may be our fine tuning numberpicker) accepts up to 3.0.
A quick look at crengine and its own m_min_font_size is 6.

(I would personally use > 5 and < 5 just to not have to think too much about them :)

poire-z · 2023-04-04T09:31:18Z

Didn't you mention that something similar could happen with line_spacing ? #10286 (comment)
Should we do something about it, or not needed ?

hius07 · 2023-04-04T09:57:08Z

koreader/frontend/ui/data/koptoptions.lua

Line 369 in b01e1c5

values = {1.0, 1.2, 1.4},

koreader/frontend/ui/data/creoptions.lua

Lines 389 to 390 in b01e1c5

    
           value_min = 50, 
        
           value_max = 300,

koreader/frontend/apps/reader/modules/readerfont.lua

Lines 263 to 264 in b01e1c5

    
           function ReaderFont:onSetLineSpace(space) 
        
               self.line_space_percent = math.min(200, math.max(50, space))

In pdf it does not cause any slowdown or issues, just a bigger spacing than available via the bottom config menu.

poire-z · 2023-04-04T10:14:24Z

Still, shouldn't a value=120 set for cre just have no effect on a PDF ?

hius07 · 2023-04-04T11:18:54Z

The last screenshot is pdf in reflow mode after applying Line spacing: 120%.

poire-z · 2023-04-06T21:52:41Z

^ Not sure what I should make out from these screenshots :/ ?

It still feels that if I make a profile with both "pdf line spacing small" and "cre line spacing large/120%", only the PDF one should have effect on a PDF and vice versa ?

hius07 · 2023-04-07T05:24:46Z

I mean that applying any "pdf action" to a cre document (and vice versa) can cause an effect of three types:
(1) crash or slow down (font_size)
(2) visual effect (line_spacing)
(3) internal effect - garbage in sdr (other settings)

(1) must be fixed no doubt, (2) and (3) are not so critical.

poire-z · 2023-04-07T20:06:16Z

If the fixes are as simple as the < and > you used for font size, it would be sad to not easy fix (2).

@hius07

KOReader 2023.04 "Solar Panel" ![koreader-2023-04-fs8](https://user-images.githubusercontent.com/202757/234975122-d7739c71-74aa-4cb0-a086-72a4bba70f52.png) It's been another busy month squashing many bugs. Our Mac users will be happy to hear that I told macOS we've supported HiDPI since long before anyone came up with such terminology (koreader#10341), and that the program can now natively build on M1 devices (koreader#10291). Solar panel credit: <https://openclipart.org/detail/294030/solar-energy> by gnokii We'd like to thank all contributors for their efforts. Some highlights since the previous release include: * Readerzooming: fix use of default settings (koreader#10205) @hius07 * ButtonDialog/ButtonDialogTitle: consistent 'width' handling (koreader#10230) @poire-z * MovableContainer: add support for anchoring initial position (koreader#10230) @poire-z * Book map, Page browser: add top left menu (koreader#10230) @poire-z * Book style tweak: add button with CSS suggestions (koreader#10230) @poire-z * crengine: fix parsing of multibytes encodings (koreader#10230) @poire-z * readerstyletweak: update profiles on unregistering in dispatcher (koreader#10247) @hius07 * Filesearcher: add search in book metadata (koreader#10198) @hius07 * Fix: Updated legacy directory, which crashed the program (koreader#10260) @Mochitto * ReaderLink: allow a forward location stack (koreader#10228) @yparitcher * BookInfo: add page information (koreader#10255) @hius07 * Center pdf manual zoom mode (koreader#10246) @nairyosangha * File browser: add Folder Menu (koreader#10275) @hius07 * Calendar view: add options to change start time of days (koreader#10254) @weijiuqiao * filechooser: fix crash on "unreadable content" (koreader#10283) @hius07 * Sync book statistics: add to dispatcher (koreader#10285) @ptrm * [plugin] Exporter: use util.getSafeFilename() to remove illegal characters from output filename (koreader#10282) @Mochitto * readerbookmark: fix writing pdf annotation (koreader#10287) @hius07 * Folder Menu: sign for Home folder (koreader#10288) @hius07 * ListMenu: show mark for books with highlights (koreader#10276) @hius07 * Calendar view's day view: thicker separator at 00:00 (koreader#10289) @poire-z * PageBrowser: tweak scrolling behaviour at book start/end (koreader#10289) @poire-z * Make `kodev check` feature complete (koreader#8682) @yparitcher * macOS: support for M1 building (koreader#10291) @ptrm * Reader: do not apply font size and spacing out of range (koreader#10295, koreader#10307) @hius07 * File browser: show Folder Menu on long-press on Home icon (koreader#10298) @hius07 * SSH.koplugin: fix cant stop SSH server bug when pid file's stale (koreader#10300) @weijiuqiao * PM: Optimize task queue handling around standby (koreader#10203) @zwim * statistic.koplugin: fix today's timeline showing next day when within custom offset (koreader#10299) @weijiuqiao * ReaderThumbnails: update cached page thumbnail on bookmark note change (koreader#10303) @hius07 * SDL: add multitouch support (koreader#10334) @Frenzie * SDL: add HiDPI support (koreader#10341) @Frenzie * BookInfo: fix crash on show cover (koreader#10315) @hius07 * Deal with table.pack corner-cases properly (koreader#10350) @NiLuJe * Android: add Tagus Gea support (<koreader/android-luajit-launcher#412>) @Alfedi [Full changelog](koreader/koreader@v2023.03...v2023.04) — [closed milestone issues](https://github.com/koreader/koreader/milestone/63?closed=1) --- Installation instructions: [Android](https://github.com/koreader/koreader/wiki/Installation-on-Android-devices) • [Cervantes](https://github.com/koreader/koreader/wiki/Installation-on-BQ-devices) • [ChromeOS](https://github.com/koreader/koreader/wiki/Installation-on-Chromebook-devices) • [Kindle](https://github.com/koreader/koreader/wiki/Installation-on-Kindle-devices) • [Kobo](https://github.com/koreader/koreader/wiki/Installation-on-Kobo-devices) • [PocketBook](https://github.com/koreader/koreader/wiki/Installation-on-PocketBook-devices) • [ReMarkable](https://github.com/koreader/koreader/wiki/Installation-on-ReMarkable) • [Desktop Linux](https://github.com/koreader/koreader/wiki/Installation-on-desktop-linux) • [MacOS](https://github.com/koreader/koreader/wiki/Installation-on-MacOS)

hius07 added 5 commits April 3, 2023 09:05

readercoptlistener: fix font size out of range

cccb25d

readerkoptlistener: fix font size out of range

0c49e46

readerview: ReZoom for paging only

70b4f15

pdfdocument: fix comment about font_size

6cbdc91

creoptions: fix font size limit

cb37b36

poire-z reviewed Apr 3, 2023

View reviewed changes

Frenzie reviewed Apr 3, 2023

View reviewed changes

hius07 added 3 commits April 3, 2023 13:08

pdfdocument revert

9ad3501

readerkoptlistener: comment

da9d8d8

readercoptlistener: comment

2522953

poire-z reviewed Apr 3, 2023

View reviewed changes

hius07 added 2 commits April 3, 2023 14:40

readerkoptlistener: > 5

00b7c2f

readerkoptlistener: < 5

a63ce89

Frenzie reviewed Apr 3, 2023

View reviewed changes

Comment thread frontend/ui/data/creoptions.lua

Frenzie approved these changes Apr 3, 2023

View reviewed changes

Frenzie added this to the 2023.04 milestone Apr 3, 2023

Frenzie added the bug label Apr 3, 2023

poire-z approved these changes Apr 3, 2023

View reviewed changes

hius07 merged commit b01e1c5 into koreader:master Apr 4, 2023

hius07 deleted the fix-font-size branch April 4, 2023 05:11

hius07 mentioned this pull request Apr 8, 2023

Reader: do not apply line spacing out of range #10307

Merged

0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026

Reader: do not apply font size out of range (koreader#10295)

b3a4d38

Uh oh!

Uh oh!

Conversation

hius07 commented Apr 3, 2023 • edited by Frenzie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poire-z Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frenzie Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

poire-z commented Apr 4, 2023

Uh oh!

hius07 commented Apr 4, 2023

Uh oh!

poire-z commented Apr 4, 2023

Uh oh!

hius07 commented Apr 4, 2023

Uh oh!

poire-z commented Apr 6, 2023

Uh oh!

hius07 commented Apr 7, 2023

Uh oh!

poire-z commented Apr 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hius07 commented Apr 3, 2023 •

edited by Frenzie

Loading

poire-z Apr 3, 2023 •

edited

Loading

Frenzie Apr 3, 2023 •

edited

Loading