diff --git a/pyWebLayout/abstract/inline.py b/pyWebLayout/abstract/inline.py index d6e3263..e28761b 100644 --- a/pyWebLayout/abstract/inline.py +++ b/pyWebLayout/abstract/inline.py @@ -163,6 +163,18 @@ class Word: """Set the next word in sequence""" self._next = next_word + def with_style(self, style: Font) -> 'Word': + """ + Return a copy of this word carrying a different font. + + Subclasses that hold extra state must override this, or that state is + silently dropped when a caller restyles the word. Sequence links + (previous/next) are deliberately not copied: the copy belongs to a + different word chain, which the new container rebuilds as words are + added to it. + """ + return Word(self._text, style, self._background) + def possible_hyphenation(self, language: str = None) -> bool: """ Hyphenate the word and store the parts. @@ -348,6 +360,19 @@ class LinkedWord(Word): """Get the link title/tooltip""" return self._title + def with_style(self, style: Font) -> 'LinkedWord': + """Return a copy carrying a different font, keeping the link intact.""" + return LinkedWord( + self._text, + style, + self._location, + link_type=self._link_type, + callback=self._callback, + background=self._background, + params=dict(self._params), + title=self._title, + ) + def execute_link(self, context: Optional[Dict[str, Any]] = None) -> Any: """ Execute the link action. diff --git a/pyWebLayout/layout/ereader_layout.py b/pyWebLayout/layout/ereader_layout.py index 7ad470b..0fb46b0 100644 --- a/pyWebLayout/layout/ereader_layout.py +++ b/pyWebLayout/layout/ereader_layout.py @@ -15,7 +15,9 @@ from __future__ import annotations from dataclasses import dataclass, asdict from typing import List, Dict, Tuple, Optional, Any -from pyWebLayout.abstract.block import Block, Paragraph, Heading, HeadingLevel, Table, HList, Image +from pyWebLayout.abstract.block import ( + Block, Paragraph, Heading, HeadingLevel, Table, TableRow, TableCell, + HList, ListItem, Quote, Image) from pyWebLayout.abstract.inline import Word from pyWebLayout.concrete.page import Page from pyWebLayout.concrete.text import Text @@ -320,6 +322,12 @@ class BidirectionalLayouter: self._page_chain: Dict[Tuple[float, Tuple[int, int, int]], RenderingPosition] = {} + # Scaled copies of blocks, keyed by (id(block), font_scale). Rebuilding + # a block's words on every page render allocated a fresh Paragraph and + # Word per word on the hot path. The original block is kept alongside + # the copy so its id cannot be recycled while it is a live key. + self._scaled_block_cache: Dict[Tuple[int, float], Tuple[Block, Block]] = {} + def render_page_forward(self, position: RenderingPosition, font_scale: float = 1.0) -> Tuple[Page, RenderingPosition]: """ @@ -526,29 +534,89 @@ class BidirectionalLayouter: return (position.chapter_index, position.block_index, position.word_index) def _scale_block_fonts(self, block: Block, font_scale: float) -> Block: - """Apply font scaling and font family override to all fonts in a block""" - # Check if we need to do any transformation + """ + Apply font scaling and the font family override to every font in a block. + + Returns the block unchanged when there is nothing to apply. Results are + memoised per (block, scale) for the life of the layouter, so a page + re-render at an unchanged scale costs a dict lookup. + """ if font_scale == 1.0 and self.font_family_override is None: return block - # This is a simplified implementation - # In practice, we'd need to handle each block type appropriately - if isinstance(block, (Paragraph, Heading)): - scaled_block_style = FontScaler.scale_font(block.style, font_scale, self.font_family_override) - if isinstance(block, Heading): - scaled_block = Heading(block.level, scaled_block_style) - else: - scaled_block = Paragraph(scaled_block_style) + key = (id(block), font_scale) + cached = self._scaled_block_cache.get(key) + if cached is not None: + return cached[1] - # words_iter() returns tuples of (position, word) - for position, word in block.words_iter(): + scaled = self._build_scaled_block(block, font_scale) + self._scaled_block_cache[key] = (block, scaled) + return scaled + + def _build_scaled_block(self, block: Block, font_scale: float) -> Block: + """Construct the scaled copy of a block. See _scale_block_fonts.""" + def scale(font: Font) -> Font: + return FontScaler.scale_font(font, font_scale, self.font_family_override) + + if isinstance(block, (Paragraph, Heading)): + if isinstance(block, Heading): + scaled_block = Heading(block.level, scale(block.style)) + else: + scaled_block = Paragraph(scale(block.style)) + + # words_iter() yields (position, word) tuples. with_style() keeps + # the concrete word class, so a LinkedWord stays linked - rebuilding + # these as plain Words silently stripped every hyperlink in the + # document as soon as the reader changed font size. + for _, word in block.words_iter(): if isinstance(word, Word): - scaled_word = Word( - word.text, FontScaler.scale_font( - word.style, font_scale, self.font_family_override)) - scaled_block.add_word(scaled_word) + scaled_block.add_word(word.with_style(scale(word.style))) return scaled_block + if isinstance(block, Quote): + scaled_quote = Quote(scale(block.style) if block.style else None) + for child in block.blocks(): + scaled_quote.add_block(self._scale_block_fonts(child, font_scale)) + return scaled_quote + + if isinstance(block, HList): + scaled_list = HList( + block.style, + scale(block.default_style) if block.default_style else None) + for item in block.items(): + scaled_item = ListItem( + item.term, + scale(item.style) if item.style else None) + for child in item.blocks(): + scaled_item.add_block(self._scale_block_fonts(child, font_scale)) + scaled_list.add_item(scaled_item) + return scaled_list + + if isinstance(block, Table): + scaled_table = Table( + block.caption, + scale(block.style) if block.style else None) + # Rows must go back into the section they came from, or a + # row would be re-added as a body row. + for section, rows in (('header', block.header_rows()), + ('body', block.body_rows()), + ('footer', block.footer_rows())): + for row in rows: + scaled_row = TableRow(scale(row.style) if row.style else None) + for cell in row.cells(): + scaled_cell = TableCell( + is_header=cell.is_header, + colspan=cell.colspan, + rowspan=cell.rowspan, + style=scale(cell.style) if cell.style else None) + for child in cell.blocks(): + scaled_cell.add_block(self._scale_block_fonts(child, font_scale)) + scaled_row.add_cell(scaled_cell) + scaled_table.add_row(scaled_row, section) + return scaled_table + + # Blocks with no fonts of their own (Image, HorizontalRule, PageBreak, + # CodeBlock - which carries raw lines, not styled words) pass through. return block def _layout_block_on_page(self, diff --git a/tests/layout/test_font_scaling.py b/tests/layout/test_font_scaling.py new file mode 100644 index 0000000..319980d --- /dev/null +++ b/tests/layout/test_font_scaling.py @@ -0,0 +1,262 @@ +""" +Tests for font scaling in the ereader layout path (R3). + +_scale_block_fonts rebuilds a block with scaled fonts. It used to construct a +plain Word for every word, which downgraded LinkedWord and silently discarded +every hyperlink in the document as soon as the reader changed font size. It +also handled only Paragraph and Heading, so quotes, lists and tables kept their +original size while the text around them reflowed. +""" + +import tempfile + +import pytest + +from pyWebLayout.abstract.block import Paragraph, Heading, Quote, HList, Table +from pyWebLayout.abstract.inline import LinkedWord, Word +from pyWebLayout.concrete.functional import LinkText +from pyWebLayout.io.readers.html_extraction import parse_html_string +from pyWebLayout.layout.ereader_layout import BidirectionalLayouter +from pyWebLayout.layout.ereader_manager import EreaderLayoutManager +from pyWebLayout.style import Font +from pyWebLayout.style.page_style import PageStyle + + +HTML = """ +

Go to this link now.

+

Quoted qlink text.

+ + + + +
head hlink
cell clink
+""" + + +def collect_links(block, out=None): + """Every LinkedWord reachable in a block, at any nesting depth.""" + out = [] if out is None else out + if isinstance(block, Paragraph): # covers Heading + for _, word in block.words_iter(): + if isinstance(word, LinkedWord): + out.append(word) + elif isinstance(block, Quote): + for child in block.blocks(): + collect_links(child, out) + elif isinstance(block, HList): + for item in block.items(): + for child in item.blocks(): + collect_links(child, out) + elif isinstance(block, Table): + for rows in (block.header_rows(), block.body_rows(), block.footer_rows()): + for row in rows: + for cell in row.cells(): + for child in cell.blocks(): + collect_links(child, out) + return out + + +def collect_sizes(block, out=None): + """Every font size reachable in a block, at any nesting depth.""" + out = [] if out is None else out + if isinstance(block, Paragraph): + for _, word in block.words_iter(): + out.append(word.style.font_size) + elif isinstance(block, Quote): + for child in block.blocks(): + collect_sizes(child, out) + elif isinstance(block, HList): + for item in block.items(): + for child in item.blocks(): + collect_sizes(child, out) + elif isinstance(block, Table): + for rows in (block.header_rows(), block.body_rows(), block.footer_rows()): + for row in rows: + for cell in row.cells(): + for child in cell.blocks(): + collect_sizes(child, out) + return out + + +@pytest.fixture +def blocks(): + return parse_html_string(HTML) + + +@pytest.fixture +def layouter(blocks): + return BidirectionalLayouter(blocks, PageStyle(), (400, 600)) + + +# ============================================================================ +# Word.with_style +# ============================================================================ + +class TestWithStyle: + def test_word_keeps_its_text_and_takes_the_new_font(self): + word = Word("hello", Font(font_size=16)) + + copy = word.with_style(Font(font_size=24)) + + assert copy.text == "hello" + assert copy.style.font_size == 24 + assert word.style.font_size == 16, "the original must not be mutated" + + def test_linked_word_stays_linked(self): + word = LinkedWord("hello", Font(font_size=16), "http://example.com", + params={"a": "1"}, title="Tooltip") + + copy = word.with_style(Font(font_size=24)) + + assert isinstance(copy, LinkedWord) + assert copy.location == "http://example.com" + assert copy.link_type == word.link_type + assert copy.params == {"a": "1"} + assert copy.link_title == "Tooltip" + assert copy.style.font_size == 24 + + def test_linked_word_params_are_copied_not_shared(self): + word = LinkedWord("hello", Font(), "http://example.com", params={"a": "1"}) + + copy = word.with_style(Font(font_size=24)) + copy.params["b"] = "2" + + assert "b" not in word.params + + +# ============================================================================ +# _scale_block_fonts +# ============================================================================ + +class TestScaleBlockFonts: + def test_links_survive_scaling_in_every_container(self, blocks, layouter): + before = sum(len(collect_links(b)) for b in blocks) + after = sum(len(collect_links(layouter._scale_block_fonts(b, 1.5))) + for b in blocks) + + assert before == 6, "fixture should contain 6 linked words" + assert after == before, "scaling must not discard hyperlinks" + + def test_link_targets_are_preserved_exactly(self, blocks, layouter): + scaled = [layouter._scale_block_fonts(b, 1.5) for b in blocks] + targets = sorted(w.location for b in scaled for w in collect_links(b)) + + assert targets == sorted([ + "http://example.com", "http://example.com", + "http://q.example", "http://l.example", + "http://h.example", "http://c.example", + ]) + + @pytest.mark.parametrize("index,kind", [(0, "paragraph"), (1, "quote"), + (2, "list"), (3, "table")]) + def test_every_container_type_actually_scales(self, blocks, layouter, index, kind): + original = collect_sizes(blocks[index]) + scaled = collect_sizes(layouter._scale_block_fonts(blocks[index], 2.0)) + + assert original, f"fixture {kind} should contain sized words" + assert scaled == [s * 2 for s in original], f"{kind} did not scale" + + def test_table_rows_stay_in_their_section(self, blocks, layouter): + table = next(b for b in blocks if isinstance(b, Table)) + + scaled = layouter._scale_block_fonts(table, 1.5) + + assert len(list(scaled.header_rows())) == len(list(table.header_rows())) + assert len(list(scaled.body_rows())) == len(list(table.body_rows())) + + def test_unscaled_blocks_are_returned_unchanged(self, blocks, layouter): + assert layouter._scale_block_fonts(blocks[0], 1.0) is blocks[0] + + def test_heading_level_is_preserved(self, layouter): + heading = parse_html_string("

Title here

")[0] + + scaled = layouter._scale_block_fonts(heading, 1.5) + + assert isinstance(scaled, Heading) + assert scaled.level == heading.level + + def test_result_is_memoised(self, blocks, layouter): + """Rebuilding a block per page render allocated on the hot path.""" + first = layouter._scale_block_fonts(blocks[0], 1.5) + second = layouter._scale_block_fonts(blocks[0], 1.5) + + assert first is second + + def test_different_scales_are_cached_separately(self, blocks, layouter): + assert (layouter._scale_block_fonts(blocks[0], 1.5) + is not layouter._scale_block_fonts(blocks[0], 2.0)) + + def test_originals_are_never_mutated(self, blocks, layouter): + before = [collect_sizes(b) for b in blocks] + + for b in blocks: + layouter._scale_block_fonts(b, 3.0) + + assert [collect_sizes(b) for b in blocks] == before + + +# ============================================================================ +# End to end +# ============================================================================ + +def rendered_link_texts(page): + """Every LinkText on a rendered page. They live inside Line objects.""" + found = [] + for child in page._children: + for text_obj in getattr(child, '_text_objects', []): + if isinstance(text_obj, LinkText): + found.append(text_obj) + return found + + +class TestLinksRemainClickableAfterFontChange: + """ + The user-visible symptom of R3: increase the font size and links stop + responding to taps. + """ + + @pytest.fixture + def manager(self): + blocks = parse_html_string( + '

Go to this link now.

') + manager = EreaderLayoutManager(blocks, page_size=(400, 600), + bookmarks_dir=tempfile.mkdtemp()) + yield manager + manager.shutdown() + + def test_links_render_at_default_scale(self, manager): + page = manager.get_current_page() + page.render() + + assert [t.link.location for t in rendered_link_texts(page)] == \ + ["http://example.com", "http://example.com"] + + @pytest.mark.parametrize("scale", [0.8, 1.5, 2.0]) + def test_links_survive_a_font_size_change(self, manager, scale): + manager.set_font_scale(scale) + page = manager.get_current_page() + page.render() + + locations = {t.link.location for t in rendered_link_texts(page)} + assert locations == {"http://example.com"} + + @pytest.mark.parametrize("scale", [1.0, 1.5]) + def test_the_link_is_reachable_by_tapping(self, manager, scale): + """ + Scanned rather than probed at the LinkText's own centre: the hit region + query_point reports is offset from LinkText.origin by roughly the + ascent. That misalignment predates this fix and is tracked separately + as R9 - it reproduces identically at scale 1.0. + """ + manager.set_font_scale(scale) + page = manager.get_current_page() + page.render() + + targets = set() + for y in range(0, 120, 2): + for x in range(0, 400, 2): + result = page.query_point((x, y)) + if result is not None and result.object_type == "link": + targets.add(result.link_target) + + assert targets == {"http://example.com"}