From 5541d5f5d335d447c779600dac07754dc1739d06 Mon Sep 17 00:00:00 2001 From: poire-z Date: Thu, 28 Nov 2019 22:39:06 +0100 Subject: [PATCH] bump crengine: word spacing and hyphenation tweaks Includes: - New option to tune word spacing: space width scale percent - Text: look for hyphenation in more words if needed - CSS: fix "hyphens:none" should override inherited "hyphens:auto" - getHtml(): grab dir= and lang= attributes from upper nodes Replace our Word Gap/Space condensing toggle/setting with a new Word Spacing toggle/setting, made of 2 values: - 1st number scales the normal width of spaces in all font (100% uses the font space width untouched) - 2nd number applies after the 1st has been applied, and tells how much these spaces can additionally be condensed to make more text fit on a line. --- .luacheckrc | 6 +- base | 2 +- defaults.lua | 17 +++-- frontend/apps/reader/modules/readerfont.lua | 14 ++-- frontend/document/credocument.lua | 15 +++-- frontend/ui/data/creoptions.lua | 31 +++++---- frontend/ui/data/optionsutil.lua | 73 +++++++++++++-------- frontend/ui/data/settings_migration.lua | 15 +++++ frontend/ui/data/strings.lua | 1 + spec/unit/defaults_spec.lua | 12 +++- spec/unit/readerfooter_spec.lua | 4 +- spec/unit/readerrolling_spec.lua | 10 +-- 12 files changed, 128 insertions(+), 72 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index 17e961b01..4f636d6ce 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -106,9 +106,9 @@ read_globals = { "DCREREADER_CONFIG_LINE_SPACE_PERCENT_LARGE", "DCREREADER_CONFIG_LINE_SPACE_PERCENT_X_LARGE", "DCREREADER_CONFIG_LINE_SPACE_PERCENT_XX_LARGE", - "DCREREADER_CONFIG_WORD_GAP_SMALL", - "DCREREADER_CONFIG_WORD_GAP_MEDIUM", - "DCREREADER_CONFIG_WORD_GAP_LARGE", + "DCREREADER_CONFIG_WORD_SPACING_SMALL", + "DCREREADER_CONFIG_WORD_SPACING_MEDIUM", + "DCREREADER_CONFIG_WORD_SPACING_LARGE", "DMINIBAR_TOC_MARKER_WIDTH", "DMINIBAR_CONTAINER_HEIGHT", "DMINIBAR_FONT_SIZE", diff --git a/base b/base index 91ecbd255..ad5510633 160000 --- a/base +++ b/base @@ -1 +1 @@ -Subproject commit 91ecbd255a27cd373cabf3f161d2d599221f36fb +Subproject commit ad55106331a726e29c630ba09f75c998b94b4708 diff --git a/defaults.lua b/defaults.lua index af8cd1b54..395ff7c44 100644 --- a/defaults.lua +++ b/defaults.lua @@ -168,10 +168,19 @@ DCREREADER_CONFIG_LINE_SPACE_PERCENT_LARGE = 120 DCREREADER_CONFIG_LINE_SPACE_PERCENT_X_LARGE = 125 DCREREADER_CONFIG_LINE_SPACE_PERCENT_XX_LARGE = 130 --- word gap percentage -DCREREADER_CONFIG_WORD_GAP_SMALL = 50 -DCREREADER_CONFIG_WORD_GAP_MEDIUM = 75 -DCREREADER_CONFIG_WORD_GAP_LARGE = 100 +-- word spacing percentages +-- 1st number scales the normal width of spaces in all font +-- (100% uses the font space width untouched) +-- 2nd number applies after the 1st has been applied, and +-- tells how much these spaces can additionally be condensed +-- to make more text fit on a line. +-- So, {80,50} can reduce the width of a space up to 40% of its +-- regular width. {99, 100} allows reducing it by at least 1px. +-- (These replace the old settings DCREREADER_CONFIG_WORD_GAP_*, +-- with the equivalence: new_option = { 100, old_option }.) +DCREREADER_CONFIG_WORD_SPACING_SMALL = {75, 50} +DCREREADER_CONFIG_WORD_SPACING_MEDIUM = {95, 75} +DCREREADER_CONFIG_WORD_SPACING_LARGE = {100, 90} -- crereader progress bar (no longer needed) -- 0 for top "full" progress bar diff --git a/frontend/apps/reader/modules/readerfont.lua b/frontend/apps/reader/modules/readerfont.lua index 8c1d61371..214e82d5c 100644 --- a/frontend/apps/reader/modules/readerfont.lua +++ b/frontend/apps/reader/modules/readerfont.lua @@ -130,9 +130,9 @@ function ReaderFont:onReadSettings(config) or G_reader_settings:readSetting("copt_font_kerning") or 3 -- harfbuzz (slower, but needed for proper arabic) self.ui.document:setFontKerning(self.font_kerning) - self.space_condensing = config:readSetting("space_condensing") - or G_reader_settings:readSetting("copt_space_condensing") or 75 - self.ui.document:setSpaceCondensing(self.space_condensing) + self.word_spacing = config:readSetting("word_spacing") + or G_reader_settings:readSetting("copt_word_spacing") or {95, 75} + self.ui.document:setWordSpacing(self.word_spacing) self.line_space_percent = config:readSetting("line_space_percent") or G_reader_settings:readSetting("copt_line_spacing") @@ -241,9 +241,9 @@ function ReaderFont:onSetFontKerning(mode) return true end -function ReaderFont:onSetSpaceCondensing(space) - self.space_condensing = space - self.ui.document:setSpaceCondensing(space) +function ReaderFont:onSetWordSpacing(values) + self.word_spacing = values + self.ui.document:setWordSpacing(values) self.ui:handleEvent(Event:new("UpdatePos")) return true end @@ -267,7 +267,7 @@ function ReaderFont:onSaveSettings() self.ui.doc_settings:saveSetting("font_embolden", self.font_embolden) self.ui.doc_settings:saveSetting("font_hinting", self.font_hinting) self.ui.doc_settings:saveSetting("font_kerning", self.font_kerning) - self.ui.doc_settings:saveSetting("space_condensing", self.space_condensing) + self.ui.doc_settings:saveSetting("word_spacing", self.word_spacing) self.ui.doc_settings:saveSetting("line_space_percent", self.line_space_percent) self.ui.doc_settings:saveSetting("gamma_index", self.gamma_index) end diff --git a/frontend/document/credocument.lua b/frontend/document/credocument.lua index c6b534ce4..b438ae6e2 100644 --- a/frontend/document/credocument.lua +++ b/frontend/document/credocument.lua @@ -703,11 +703,16 @@ function CreDocument:setFontKerning(mode) self._document:setIntProperty("font.kerning.mode", mode) end --- min space condensing percent (how much we can decrease a space width to --- make text fit on a line) 25...100% -function CreDocument:setSpaceCondensing(value) - logger.dbg("CreDocument: set space condensing", value) - self._document:setIntProperty("crengine.style.space.condensing.percent", value) +function CreDocument:setWordSpacing(values) + -- values should be a table of 2 numbers (e.g.: { 90, 75 }) + -- - space width scale percent (hard scale the width of each space char in + -- all fonts - 100 to use the normal font space glyph width unchanged). + -- - min space condensing percent (how much we can additionally decrease + -- a space width to make text fit on a line). + logger.dbg("CreDocument: set space width scale", values[1]) + self._document:setIntProperty("crengine.style.space.width.scale.percent", values[1]) + logger.dbg("CreDocument: set space condensing", values[2]) + self._document:setIntProperty("crengine.style.space.condensing.percent", values[2]) end function CreDocument:setStyleSheet(new_css_file, appended_css_content ) diff --git a/frontend/ui/data/creoptions.lua b/frontend/ui/data/creoptions.lua index 8dc3e0768..8b9e38054 100644 --- a/frontend/ui/data/creoptions.lua +++ b/frontend/ui/data/creoptions.lua @@ -277,8 +277,10 @@ Note that your selected font size is not affected by this setting.]]), }, name_text_hold_callback = optionsutil.showValues, -- used by showValues - name_text_suffix = "%", name_text_true_values = true, + show_true_value_func = function(val) -- add "%" + return string.format("%d%%", val) + end, }, } }, @@ -399,26 +401,27 @@ Note that your selected font size is not affected by this setting.]]), (Font Hinting may need to be adjusted for the best result with either kerning implementation.)]]), }, { - name = "space_condensing", - name_text = S.WORD_GAP, + name = "word_spacing", + name_text = S.WORD_SPACING, toggle = {S.SMALL, S.MEDIUM, S.LARGE}, values = { - DCREREADER_CONFIG_WORD_GAP_SMALL, - DCREREADER_CONFIG_WORD_GAP_MEDIUM, - DCREREADER_CONFIG_WORD_GAP_LARGE, + DCREREADER_CONFIG_WORD_SPACING_SMALL, + DCREREADER_CONFIG_WORD_SPACING_MEDIUM, + DCREREADER_CONFIG_WORD_SPACING_LARGE, }, - default_value = DCREREADER_CONFIG_WORD_GAP_MEDIUM, + default_value = DCREREADER_CONFIG_WORD_SPACING_MEDIUM, args = { - DCREREADER_CONFIG_WORD_GAP_SMALL, - DCREREADER_CONFIG_WORD_GAP_MEDIUM, - DCREREADER_CONFIG_WORD_GAP_LARGE, + DCREREADER_CONFIG_WORD_SPACING_SMALL, + DCREREADER_CONFIG_WORD_SPACING_MEDIUM, + DCREREADER_CONFIG_WORD_SPACING_LARGE, }, - event = "SetSpaceCondensing", + event = "SetWordSpacing", + help_text = _([[Tells the rendering engine how much to scale the width of each 'space' character in the text from its regular width, and how much it can additionally reduce them to make more words fit on a line (100% means no reduction).]]), name_text_hold_callback = optionsutil.showValues, - -- used by showValues - name_text_suffix = "%", name_text_true_values = true, - help_text = _([[Tells the rendering engine how much each 'space' character in the text can be reduced from its regular width to make words fit on a line (100% means no reduction).]]), + show_true_value_func = function(val) + return string.format("%d%%, +%d%%", val[1], val[2]) + end, } } }, diff --git a/frontend/ui/data/optionsutil.lua b/frontend/ui/data/optionsutil.lua index 5150b2807..028912ef3 100644 --- a/frontend/ui/data/optionsutil.lua +++ b/frontend/ui/data/optionsutil.lua @@ -19,32 +19,40 @@ function optionsutil.showValues(configurable, option, prefix) local default = G_reader_settings:readSetting(prefix.."_"..option.name) local current = configurable[option.name] local value_default, value_current - local suffix = option.name_text_suffix or "" if option.name == "screen_mode" then current = Screen:getScreenMode() end - local arg_table = {} if option.toggle and option.values then + -- build a table so we can see if current/default settings map + -- to a known setting with a name (in option.toggle) + local arg_table = {} for i=1,#option.values do - arg_table[option.values[i]] = option.toggle[i] + local val = option.values[i] + -- flatten table to a string for easy lookup via arg_table + if type(val) == "table" then val = table.concat(val, ",") end + arg_table[val] = option.toggle[i] end - end - if not default then - default = _("not set") - if option.toggle and option.values then - value_current = current - current = arg_table[current] - if not current then current = value_current end - end - elseif option.toggle and option.values then value_current = current - value_default = default - default = arg_table[default] + if type(current) == "table" then current = table.concat(current, ",") end current = arg_table[current] - if not default then default = value_default end - if not current then current = value_current end - end - if option.labels and option.values then + if not current then + current = option.name_text_true_values and _("custom") or value_current + end + if option.show_true_value_func then + value_current = option.show_true_value_func(value_current) + end + if default then + value_default = default + if type(default) == "table" then default = table.concat(default, ",") end + default = arg_table[default] + if not default then + default = option.name_text_true_values and _("custom") or value_default + end + if option.show_true_value_func then + value_default = option.show_true_value_func(value_default) + end + end + elseif option.labels and option.values then if option.more_options_param and option.more_options_param.value_table then if option.more_options_param.args_table then for k,v in pairs(option.more_options_param.args_table) do @@ -55,7 +63,7 @@ function optionsutil.showValues(configurable, option, prefix) end end current = option.more_options_param.value_table[current] - if default ~= _("not set") then + if default then if option.more_options_param.args_table then for k,v in pairs(option.more_options_param.args_table) do if v == default then @@ -67,7 +75,7 @@ function optionsutil.showValues(configurable, option, prefix) default = option.more_options_param.value_table[default] end else - if default ~= _("not set") then + if default then for i=1,#option.labels do if default == option.values[i] then default = option.labels[i] @@ -82,21 +90,30 @@ function optionsutil.showValues(configurable, option, prefix) end end end + elseif option.show_true_value_func and option.values then + current = option.show_true_value_func(current) + if default then + default = option.show_true_value_func(default) + end + end + if not default then + default = _("not set") end local help_text = "" if option.help_text then help_text = T("\n%1\n", option.help_text) end local text - if option.name_text_true_values and option.toggle and option.values and value_default then - text = T(_("%1:\n%2\nCurrent value: %3 (%6%5)\nDefault value: %4 (%7%5)"), option.name_text, help_text, - current, default, suffix, value_current, value_default) - elseif option.name_text_true_values and option.toggle and option.values and not value_default then - text = T(_("%1\n%2\nCurrent value: %3 (%6%5)\nDefault value: %4"), option.name_text, help_text, - current, default, suffix, value_current) + if option.name_text_true_values and option.toggle and option.values then + if value_default then + text = T(_("%1\n%2\nCurrent value: %3 (%4)\nDefault value: %5 (%6)"), option.name_text, help_text, + current, value_current, default, value_default) + else + text = T(_("%1\n%2\nCurrent value: %3 (%4)\nDefault value: %5"), option.name_text, help_text, + current, value_current, default) + end else - text = T(_("%1\n%2\nCurrent value: %3%5\nDefault value: %4%5"), option.name_text, help_text, - current, default, suffix) + text = T(_("%1\n%2\nCurrent value: %3\nDefault value: %4"), option.name_text, help_text, current, default) end UIManager:show(InfoMessage:new{ text=text }) end diff --git a/frontend/ui/data/settings_migration.lua b/frontend/ui/data/settings_migration.lua index 66d931242..739c645f3 100644 --- a/frontend/ui/data/settings_migration.lua +++ b/frontend/ui/data/settings_migration.lua @@ -35,6 +35,21 @@ function SettingsMigration:migrateSettings(config) -- Wipe it config:delSetting("copt_page_margins") end + + -- Space condensing to Word spacing + -- From a single number (space condensing) to a table of 2 numbers ({space width scale, space condensing}). + -- Be conservative and don't change space width scale: use 100% + if not config:readSetting("copt_word_spacing") and config:readSetting("copt_space_condensing") then + local space_condensing = config:readSetting("copt_space_condensing") + logger.info("Migrating old", cfg_class, "CRe space condensing:", space_condensing) + config:saveSetting("copt_word_spacing", { 100, space_condensing }) + if cfg_class == "book" then + -- a bit messy that some settings are saved twice in DocSettings, with + -- and without a copt_ prefix, and they must be in sync + config:saveSetting("word_spacing", { 100, space_condensing }) + end + end + end return SettingsMigration diff --git a/frontend/ui/data/strings.lua b/frontend/ui/data/strings.lua index c7721c831..5d0304c17 100644 --- a/frontend/ui/data/strings.lua +++ b/frontend/ui/data/strings.lua @@ -33,6 +33,7 @@ S.FONT_WEIGHT = _("Font Weight") S.GAMMA = _("Gamma") S.FONT_HINT = _("Font Hinting") S.FONT_KERNING = _("Font Kerning") +S.WORD_SPACING = _("Word Spacing") S.VIEW_MODE = _("View Mode") S.EMBEDDED_STYLE = _("Embedded Style") S.EMBEDDED_FONTS = _("Embedded Fonts") diff --git a/spec/unit/defaults_spec.lua b/spec/unit/defaults_spec.lua index 5fe0e2bbc..bc5f66b5f 100644 --- a/spec/unit/defaults_spec.lua +++ b/spec/unit/defaults_spec.lua @@ -27,11 +27,15 @@ describe("defaults module", function() assert.is_same("DFULL_SCREEN", Defaults.defaults_name[54]) assert.is_same("SEARCH_LIBRARY_PATH", Defaults.defaults_name[103]) assert.is_same("DTAP_ZONE_BACKWARD", Defaults.defaults_name[87]) - assert.is_same("DCREREADER_CONFIG_WORD_GAP_LARGE", Defaults.defaults_name[47]) + assert.is_same("DCREREADER_CONFIG_WORD_SPACING_LARGE", Defaults.defaults_name[47]) assert.is_same("DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE", Defaults.defaults_name[20]) local fd = io.open(persistent_filename, "r") assert.Equals( [[-- For configuration changes that persists between updates +DCREREADER_CONFIG_WORD_SPACING_LARGE = { + [1] = 100, + [2] = 90 +} SEARCH_LIBRARY_PATH = "" DTAP_ZONE_BACKWARD = { ["y"] = 0, @@ -39,7 +43,6 @@ DTAP_ZONE_BACKWARD = { ["h"] = 1, ["w"] = 0.25 } -DCREREADER_CONFIG_WORD_GAP_LARGE = 100 DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE = { [1] = 50, [2] = 50 @@ -64,6 +67,10 @@ DFULL_SCREEN = 1 fd = io.open(persistent_filename) assert.Equals( [[-- For configuration changes that persists between updates +DCREREADER_CONFIG_WORD_SPACING_LARGE = { + [2] = 90, + [1] = 100 +} SEARCH_LIBRARY_PATH = "" DTAP_ZONE_BACKWARD = { ["y"] = 10, @@ -71,7 +78,6 @@ DTAP_ZONE_BACKWARD = { ["h"] = 20.25, ["w"] = 20.75 } -DCREREADER_CONFIG_WORD_GAP_LARGE = 100 DCREREADER_CONFIG_H_MARGIN_SIZES_XXX_LARGE = { [2] = 50, [1] = 50 diff --git a/spec/unit/readerfooter_spec.lua b/spec/unit/readerfooter_spec.lua index b57ef44dd..d0910cdef 100644 --- a/spec/unit/readerfooter_spec.lua +++ b/spec/unit/readerfooter_spec.lua @@ -317,8 +317,8 @@ describe("Readerfooter module", function() assert.are.same(362, footer.text_width) footer:onPageUpdate(100) - assert.are.same(186, footer.progress_bar.width) - assert.are.same(394, footer.text_width) + assert.are.same(194, footer.progress_bar.width) + assert.are.same(386, footer.text_width) end) it("should support chapter markers", function() diff --git a/spec/unit/readerrolling_spec.lua b/spec/unit/readerrolling_spec.lua index eab0f1403..f670ec337 100644 --- a/spec/unit/readerrolling_spec.lua +++ b/spec/unit/readerrolling_spec.lua @@ -184,15 +184,15 @@ describe("Readerrolling module", function() describe("test changing word gap - space condensing", function() it("should show pages for different word gap", function() - readerui.document:setSpaceCondensing(100) + readerui.document:setWordSpacing({100, 90}) readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) assert.are.same(252, readerui.document:getPageCount()) - readerui.document:setSpaceCondensing(75) + readerui.document:setWordSpacing({95, 75}) readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) - assert.are.same(248, readerui.document:getPageCount()) - readerui.document:setSpaceCondensing(50) + assert.are.same(241, readerui.document:getPageCount()) + readerui.document:setWordSpacing({75, 50}) readerui:handleEvent(Event:new("ChangeScreenMode", "portrait")) - assert.are.same(235, readerui.document:getPageCount()) + assert.are.same(231, readerui.document:getPageCount()) end) end)