Compare commits
7
Commits
1924cc234d
...
3761e00398
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3761e00398 | ||
|
|
0ce1aeaa87 | ||
|
|
8746d3f549 | ||
|
|
bcae45a023 | ||
|
|
e81ba48f6d | ||
|
|
62ca15159a | ||
|
|
f0dc67541b |
+81
-13
@@ -22,7 +22,8 @@ finding is already specced, it is cross-referenced rather than restated.
|
|||||||
| [R5](#r5--monkey-patched-page-methods-with-a-conflicting-signature) | Monkey-patched `Page` methods with a conflicting signature | Medium | New |
|
| [R5](#r5--monkey-patched-page-methods-with-a-conflicting-signature) | Monkey-patched `Page` methods with a conflicting signature | Medium | New |
|
||||||
| [R6](#r6--dead-duck-typing-cluster-in-pagepy) | Dead duck-typing cluster in `page.py` | Low | New; extends S10.3 |
|
| [R6](#r6--dead-duck-typing-cluster-in-pagepy) | Dead duck-typing cluster in `page.py` | Low | New; extends S10.3 |
|
||||||
| [R7](#r7--two-orphaned-subsystems) | Two orphaned subsystems | Low | New |
|
| [R7](#r7--two-orphaned-subsystems) | Two orphaned subsystems | Low | New |
|
||||||
| [R8](#r8--backward-pagination-is-guesswork) | Backward pagination is guesswork | Medium | Partially noted in S11 |
|
| [R8](#r8--backward-pagination-is-guesswork) | Backward pagination is guesswork | Medium | Resolved by S16 |
|
||||||
|
| [R9](#r9--query_points-hit-region-is-offset-from-the-glyphs) | `query_point`'s hit region is offset from the glyphs | Medium | New, open |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -494,22 +495,89 @@ positions round-trip through tables correctly (S8 already notes this dependency)
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Recommended order
|
## Status
|
||||||
|
|
||||||
|
All findings in this document are resolved. What remains is the existing
|
||||||
|
remediation spec: **S4 → S5 → S6 → S7 → S8 → S9**, plus **S10.1**, unchanged.
|
||||||
|
|
||||||
|
| ID | Resolution | Commit |
|
||||||
|
|----|-----------|--------|
|
||||||
|
| R1 | Fixed with S12 — the pool that raised is gone | `1924cc2` |
|
||||||
|
| R2 | Fixed with S12 — no executor, no blocking finaliser | `1924cc2` |
|
||||||
|
| R3 | `Word.with_style` keeps subclasses; all container blocks scale | `f0dc675` |
|
||||||
|
| R4 | Consolidated on `pyproject.toml`; 7 runtime deps → 4 | `767e4c1` |
|
||||||
|
| R5 | Monkey patch deleted | `62ca151` |
|
||||||
|
| R6 | 138 dead lines deleted; contract hardening deferred to S10.1 | `e81ba48` |
|
||||||
|
| R7 | Both subsystems wired into `EreaderLayoutManager` | `8746d3f`, `0ce1aea` |
|
||||||
|
| R8 | Superseded by S16 (anchor replay); dead estimators removed | `bcae45a` |
|
||||||
|
| R9 | Open — see below | — |
|
||||||
|
|
||||||
|
Two things worth carrying forward:
|
||||||
|
|
||||||
|
- **S12's measurement stands as the argument against prefetch.** A page render
|
||||||
|
is 9–56 ms. Any future proposal to render ahead should have to beat that
|
||||||
|
number first.
|
||||||
|
- **Wiring an orphan found a bug.** R7's interaction handler had a crash on
|
||||||
|
every hovered or pressed link (`0ce1aea`). Unreachable code is not
|
||||||
|
neutral — it is untested code that looks tested.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## R9 — query_point's hit region is offset from the glyphs
|
||||||
|
|
||||||
|
**Severity: medium.** Found while verifying R3; not part of the original review.
|
||||||
|
|
||||||
|
### Problem
|
||||||
|
|
||||||
|
The region `Page.query_point` reports for a text object does not line up with
|
||||||
|
where that object says it is. Probing a `LinkText` at the centre of its own
|
||||||
|
`origin`/`size` box returns `object_type="empty"`.
|
||||||
|
|
||||||
|
### Evidence
|
||||||
|
|
||||||
|
A single-link page at 400×600, default scale:
|
||||||
|
|
||||||
```
|
```
|
||||||
R4 ── packaging; independent, minutes, unblocks clean CI [done]
|
'this' origin=(68.3, 35.0) size=(29.2, 19.0) centre=(82, 44) -> empty
|
||||||
S12 ── delete the process pool; resolves R1 and R2 with it
|
'link' origin=(102.5, 35.0) size=(28.3, 19.0) centre=(116, 44) -> empty
|
||||||
R3 ── font scaling loses links; independent, user-visible
|
|
||||||
R5 ── delete the monkey patch; minutes
|
grid scan: link is detected across y≈20–39
|
||||||
R6 ── delete the dead cluster (with S10.1's render contract)
|
LinkText claims: y≈35–54
|
||||||
R7 ── decide the two orphans; no code risk either way
|
|
||||||
S4 → S5 → S6 → S7 → S8 → S9 (existing spec, unchanged)
|
|
||||||
R8 ── after S8
|
|
||||||
```
|
```
|
||||||
|
|
||||||
R4, R5 and R6 are an afternoon and carry no design risk. S12 is the largest
|
The two bands overlap by about four pixels. The offset is close to the font
|
||||||
single removal and fixes two defects at once. R3 is the one users would notice
|
ascent, which points at a baseline-versus-top mismatch between the coordinates
|
||||||
today. Everything after that is the existing spec, which needs no revision.
|
`Text` stores and the ones `in_object` tests.
|
||||||
|
|
||||||
|
This reproduces identically at scale 1.0 and 1.5, so it predates the R3 fix.
|
||||||
|
|
||||||
|
### Why it matters
|
||||||
|
|
||||||
|
Taps land through the grid because the region is only shifted, not absent — but
|
||||||
|
it is shifted by most of a line height. Near the top or bottom of a page, or
|
||||||
|
between tightly spaced lines, a tap can hit the neighbouring line instead of the
|
||||||
|
one under the finger. It also makes `LinkText.origin`/`size` unusable for
|
||||||
|
drawing selection or highlight overlays, which is what R7's highlighting now
|
||||||
|
depends on.
|
||||||
|
|
||||||
|
### Action
|
||||||
|
|
||||||
|
Establish which of the two is authoritative — almost certainly the drawn
|
||||||
|
position — and make the other agree. This sits close to S2 (page geometry) and
|
||||||
|
S3 (draw/canvas lifecycle), both already landed, so the conventions to match
|
||||||
|
are in place.
|
||||||
|
|
||||||
|
### Acceptance criteria
|
||||||
|
|
||||||
|
- `page.query_point(centre_of(obj))` returns `obj` for every text object on a
|
||||||
|
rendered page, at scales 0.8, 1.0, 1.5 and 2.0.
|
||||||
|
- The end-to-end test in `tests/layout/test_font_scaling.py` probes the centre
|
||||||
|
directly instead of scanning a grid.
|
||||||
|
|
||||||
|
### Files
|
||||||
|
|
||||||
|
`pyWebLayout/concrete/text.py`, `pyWebLayout/concrete/page.py`,
|
||||||
|
`pyWebLayout/core/base.py`
|
||||||
|
|
||||||
## Reproducing the findings
|
## Reproducing the findings
|
||||||
|
|
||||||
|
|||||||
@@ -163,6 +163,18 @@ class Word:
|
|||||||
"""Set the next word in sequence"""
|
"""Set the next word in sequence"""
|
||||||
self._next = next_word
|
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:
|
def possible_hyphenation(self, language: str = None) -> bool:
|
||||||
"""
|
"""
|
||||||
Hyphenate the word and store the parts.
|
Hyphenate the word and store the parts.
|
||||||
@@ -348,6 +360,19 @@ class LinkedWord(Word):
|
|||||||
"""Get the link title/tooltip"""
|
"""Get the link title/tooltip"""
|
||||||
return self._title
|
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:
|
def execute_link(self, context: Optional[Dict[str, Any]] = None) -> Any:
|
||||||
"""
|
"""
|
||||||
Execute the link action.
|
Execute the link action.
|
||||||
|
|||||||
@@ -99,15 +99,20 @@ class LinkText(Text, Interactable, Queriable):
|
|||||||
self._origin,
|
self._origin,
|
||||||
np.ndarray) else self._origin
|
np.ndarray) else self._origin
|
||||||
|
|
||||||
# Draw background based on state (before text is rendered)
|
# Draw background based on state (before text is rendered).
|
||||||
if self._pressed:
|
# PIL wants a flat sequence of four scalars; handing it a list of two
|
||||||
# Pressed state - stronger, darker highlight
|
# numpy arrays raises "coordinate list must contain exactly 2
|
||||||
bg_color = (180, 180, 255, 180) # Stronger blue with more opacity
|
# coordinates".
|
||||||
self._draw.rectangle([origin, origin + size], fill=bg_color)
|
if self._pressed or self._hovered:
|
||||||
elif self._hovered:
|
far = origin + size
|
||||||
# Hover state - subtle highlight
|
box = (int(origin[0]), int(origin[1]), int(far[0]), int(far[1]))
|
||||||
bg_color = (220, 220, 255, 100) # Light blue with alpha
|
if self._pressed:
|
||||||
self._draw.rectangle([origin, origin + size], fill=bg_color)
|
# Pressed state - stronger, darker highlight
|
||||||
|
bg_color = (180, 180, 255, 180)
|
||||||
|
else:
|
||||||
|
# Hover state - subtle highlight
|
||||||
|
bg_color = (220, 220, 255, 100)
|
||||||
|
self._draw.rectangle(box, fill=bg_color)
|
||||||
|
|
||||||
# Call the parent Text render method with parameters
|
# Call the parent Text render method with parameters
|
||||||
super().render(next_text, spacing)
|
super().render(next_text, spacing)
|
||||||
|
|||||||
@@ -258,69 +258,6 @@ class Page(Renderable, Queriable):
|
|||||||
"""Get a copy of the children list"""
|
"""Get a copy of the children list"""
|
||||||
return self._children.copy()
|
return self._children.copy()
|
||||||
|
|
||||||
def _get_child_property(self, child: Renderable, private_attr: str,
|
|
||||||
public_attr: str, index: Optional[int] = None,
|
|
||||||
default: Optional[int] = None) -> Optional[int]:
|
|
||||||
"""
|
|
||||||
Generic helper to extract properties from child objects with multiple fallback strategies.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
child: The child object
|
|
||||||
private_attr: Name of the private attribute (e.g., '_size')
|
|
||||||
public_attr: Name of the public property (e.g., 'size')
|
|
||||||
index: Optional index for array-like properties (0 for width, 1 for height)
|
|
||||||
default: Default value if property cannot be determined
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Property value or default
|
|
||||||
"""
|
|
||||||
# Try private attribute first
|
|
||||||
if hasattr(child, private_attr):
|
|
||||||
value = getattr(child, private_attr)
|
|
||||||
if value is not None:
|
|
||||||
if isinstance(value, (list, tuple, np.ndarray)):
|
|
||||||
if index is not None and len(value) > index:
|
|
||||||
return int(value[index])
|
|
||||||
elif index is None:
|
|
||||||
return value
|
|
||||||
|
|
||||||
# Try public property
|
|
||||||
if hasattr(child, public_attr):
|
|
||||||
value = getattr(child, public_attr)
|
|
||||||
if value is not None:
|
|
||||||
if isinstance(value, (list, tuple, np.ndarray)):
|
|
||||||
if index is not None and len(value) > index:
|
|
||||||
return int(value[index])
|
|
||||||
elif index is None:
|
|
||||||
return value
|
|
||||||
else:
|
|
||||||
return int(value)
|
|
||||||
|
|
||||||
return default
|
|
||||||
|
|
||||||
def _get_child_height(self, child: Renderable) -> int:
|
|
||||||
"""
|
|
||||||
Get the height of a child object.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
child: The child to measure
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Height in pixels
|
|
||||||
"""
|
|
||||||
# Try to get height from size property (index 1)
|
|
||||||
height = self._get_child_property(child, '_size', 'size', index=1)
|
|
||||||
if height is not None:
|
|
||||||
return height
|
|
||||||
|
|
||||||
# Try direct height attribute
|
|
||||||
height = self._get_child_property(child, '_height', 'height')
|
|
||||||
if height is not None:
|
|
||||||
return height
|
|
||||||
|
|
||||||
# Default fallback height
|
|
||||||
return 20
|
|
||||||
|
|
||||||
def render_children(self):
|
def render_children(self):
|
||||||
"""
|
"""
|
||||||
Call render on all children in the list.
|
Call render on all children in the list.
|
||||||
@@ -379,23 +316,6 @@ class Page(Renderable, Queriable):
|
|||||||
|
|
||||||
return canvas
|
return canvas
|
||||||
|
|
||||||
def _get_child_position(self, child: Renderable) -> Tuple[int, int]:
|
|
||||||
"""
|
|
||||||
Get the position where a child should be rendered.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
child: The child object
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Tuple of (x, y) coordinates
|
|
||||||
"""
|
|
||||||
# Try to get x coordinate
|
|
||||||
x = self._get_child_property(child, '_origin', 'position', index=0, default=0)
|
|
||||||
# Try to get y coordinate
|
|
||||||
y = self._get_child_property(child, '_origin', 'position', index=1, default=0)
|
|
||||||
|
|
||||||
return (x, y)
|
|
||||||
|
|
||||||
def query_point(self, point: Tuple[int, int]) -> Optional[QueryResult]:
|
def query_point(self, point: Tuple[int, int]) -> Optional[QueryResult]:
|
||||||
"""
|
"""
|
||||||
Query a point to find the deepest object at that location.
|
Query a point to find the deepest object at that location.
|
||||||
@@ -432,64 +352,6 @@ class Page(Renderable, Queriable):
|
|||||||
bounds=(int(point[0]), int(point[1]), 0, 0)
|
bounds=(int(point[0]), int(point[1]), 0, 0)
|
||||||
)
|
)
|
||||||
|
|
||||||
def _point_in_child(self, point: np.ndarray, child: Renderable) -> bool:
|
|
||||||
"""
|
|
||||||
Check if a point is within a child's bounds.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
point: The point to check
|
|
||||||
child: The child to check against
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
True if the point is within the child's bounds
|
|
||||||
"""
|
|
||||||
# If child implements Queriable interface, use it
|
|
||||||
if isinstance(child, Queriable) and hasattr(child, 'in_object'):
|
|
||||||
try:
|
|
||||||
return child.in_object(point)
|
|
||||||
except BaseException:
|
|
||||||
pass # Fall back to bounds checking
|
|
||||||
|
|
||||||
# Get child position and size for bounds checking
|
|
||||||
child_pos = self._get_child_position(child)
|
|
||||||
child_size = self._get_child_size(child)
|
|
||||||
|
|
||||||
if child_size is None:
|
|
||||||
return False
|
|
||||||
|
|
||||||
# Check if point is within child bounds
|
|
||||||
return (
|
|
||||||
child_pos[0] <= point[0] < child_pos[0] + child_size[0] and
|
|
||||||
child_pos[1] <= point[1] < child_pos[1] + child_size[1]
|
|
||||||
)
|
|
||||||
|
|
||||||
def _get_child_size(self, child: Renderable) -> Optional[Tuple[int, int]]:
|
|
||||||
"""
|
|
||||||
Get the size of a child object.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
child: The child to measure
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Tuple of (width, height) or None if size cannot be determined
|
|
||||||
"""
|
|
||||||
# Try to get width and height from size property
|
|
||||||
width = self._get_child_property(child, '_size', 'size', index=0)
|
|
||||||
height = self._get_child_property(child, '_size', 'size', index=1)
|
|
||||||
|
|
||||||
# If size property worked, return it
|
|
||||||
if width is not None and height is not None:
|
|
||||||
return (width, height)
|
|
||||||
|
|
||||||
# Try direct width/height attributes
|
|
||||||
width = self._get_child_property(child, '_width', 'width')
|
|
||||||
height = self._get_child_property(child, '_height', 'height')
|
|
||||||
|
|
||||||
if width is not None and height is not None:
|
|
||||||
return (width, height)
|
|
||||||
|
|
||||||
return None
|
|
||||||
|
|
||||||
def _make_query_result(self, obj, point: Tuple[int, int]) -> QueryResult:
|
def _make_query_result(self, obj, point: Tuple[int, int]) -> QueryResult:
|
||||||
"""
|
"""
|
||||||
Package an object into a QueryResult with metadata.
|
Package an object into a QueryResult with metadata.
|
||||||
|
|||||||
@@ -6,12 +6,16 @@ managing highlight collections, and rendering highlights on pages.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
import logging
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from typing import List, Tuple, Optional, Dict, Any
|
from typing import List, Tuple, Optional, Dict, Any
|
||||||
from enum import Enum
|
from enum import Enum
|
||||||
import json
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from pyWebLayout.core.persistence import ensure_dir, read_json, write_json
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class HighlightColor(Enum):
|
class HighlightColor(Enum):
|
||||||
"""Predefined highlight colors with RGBA values"""
|
"""Predefined highlight colors with RGBA values"""
|
||||||
@@ -44,6 +48,12 @@ class Highlight:
|
|||||||
start_word_index: Optional[int] = None # Word index in document (if available)
|
start_word_index: Optional[int] = None # Word index in document (if available)
|
||||||
end_word_index: Optional[int] = None
|
end_word_index: Optional[int] = None
|
||||||
|
|
||||||
|
# Where in the document this highlight lives, as a serialized
|
||||||
|
# RenderingPosition. `bounds` are pixel coordinates on one particular
|
||||||
|
# rendering, so they stop matching as soon as the font scale or page size
|
||||||
|
# changes; this survives repagination and is what page association uses.
|
||||||
|
position: Optional[Dict[str, Any]] = None
|
||||||
|
|
||||||
# Metadata
|
# Metadata
|
||||||
note: Optional[str] = None # Optional annotation
|
note: Optional[str] = None # Optional annotation
|
||||||
tags: List[str] = None # Optional categorization tags
|
tags: List[str] = None # Optional categorization tags
|
||||||
@@ -63,6 +73,7 @@ class Highlight:
|
|||||||
'text': self.text,
|
'text': self.text,
|
||||||
'start_word_index': self.start_word_index,
|
'start_word_index': self.start_word_index,
|
||||||
'end_word_index': self.end_word_index,
|
'end_word_index': self.end_word_index,
|
||||||
|
'position': self.position,
|
||||||
'note': self.note,
|
'note': self.note,
|
||||||
'tags': self.tags,
|
'tags': self.tags,
|
||||||
'timestamp': self.timestamp
|
'timestamp': self.timestamp
|
||||||
@@ -78,6 +89,7 @@ class Highlight:
|
|||||||
text=data['text'],
|
text=data['text'],
|
||||||
start_word_index=data.get('start_word_index'),
|
start_word_index=data.get('start_word_index'),
|
||||||
end_word_index=data.get('end_word_index'),
|
end_word_index=data.get('end_word_index'),
|
||||||
|
position=data.get('position'),
|
||||||
note=data.get('note'),
|
note=data.get('note'),
|
||||||
tags=data.get('tags', []),
|
tags=data.get('tags', []),
|
||||||
timestamp=data.get('timestamp')
|
timestamp=data.get('timestamp')
|
||||||
@@ -100,12 +112,9 @@ class HighlightManager:
|
|||||||
highlights_dir: Directory to store highlight data
|
highlights_dir: Directory to store highlight data
|
||||||
"""
|
"""
|
||||||
self.document_id = document_id
|
self.document_id = document_id
|
||||||
self.highlights_dir = Path(highlights_dir)
|
self.highlights_dir = ensure_dir(highlights_dir)
|
||||||
self.highlights: Dict[str, Highlight] = {} # id -> Highlight
|
self.highlights: Dict[str, Highlight] = {} # id -> Highlight
|
||||||
|
|
||||||
# Create directory if it doesn't exist
|
|
||||||
self.highlights_dir.mkdir(parents=True, exist_ok=True)
|
|
||||||
|
|
||||||
# Load existing highlights
|
# Load existing highlights
|
||||||
self._load_highlights()
|
self._load_highlights()
|
||||||
|
|
||||||
@@ -178,34 +187,22 @@ class HighlightManager:
|
|||||||
|
|
||||||
def _save_highlights(self) -> None:
|
def _save_highlights(self) -> None:
|
||||||
"""Persist highlights to disk"""
|
"""Persist highlights to disk"""
|
||||||
try:
|
write_json(self._get_filepath(), {
|
||||||
filepath = self._get_filepath()
|
'document_id': self.document_id,
|
||||||
data = {
|
'highlights': [h.to_dict() for h in self.highlights.values()]
|
||||||
'document_id': self.document_id,
|
})
|
||||||
'highlights': [h.to_dict() for h in self.highlights.values()]
|
|
||||||
}
|
|
||||||
|
|
||||||
with open(filepath, 'w') as f:
|
|
||||||
json.dump(data, f, indent=2)
|
|
||||||
except Exception as e:
|
|
||||||
print(f"Error saving highlights: {e}")
|
|
||||||
|
|
||||||
def _load_highlights(self) -> None:
|
def _load_highlights(self) -> None:
|
||||||
"""Load highlights from disk"""
|
"""Load highlights from disk"""
|
||||||
|
data = read_json(self._get_filepath(), {})
|
||||||
try:
|
try:
|
||||||
filepath = self._get_filepath()
|
|
||||||
if not filepath.exists():
|
|
||||||
return
|
|
||||||
|
|
||||||
with open(filepath, 'r') as f:
|
|
||||||
data = json.load(f)
|
|
||||||
|
|
||||||
self.highlights = {
|
self.highlights = {
|
||||||
h['id']: Highlight.from_dict(h)
|
h['id']: Highlight.from_dict(h)
|
||||||
for h in data.get('highlights', [])
|
for h in data.get('highlights', [])
|
||||||
}
|
}
|
||||||
except Exception as e:
|
except (AttributeError, TypeError, KeyError):
|
||||||
print(f"Error loading highlights: {e}")
|
logger.warning("Highlight file %s is not in the expected shape; ignoring it",
|
||||||
|
self._get_filepath(), exc_info=True)
|
||||||
self.highlights = {}
|
self.highlights = {}
|
||||||
|
|
||||||
|
|
||||||
@@ -213,16 +210,18 @@ def create_highlight_from_query_result(
|
|||||||
result,
|
result,
|
||||||
color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
|
color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
|
||||||
note: Optional[str] = None,
|
note: Optional[str] = None,
|
||||||
tags: Optional[List[str]] = None
|
tags: Optional[List[str]] = None,
|
||||||
|
position: Optional[Dict[str, Any]] = None
|
||||||
) -> Highlight:
|
) -> Highlight:
|
||||||
"""
|
"""
|
||||||
Create a highlight from a QueryResult.
|
Create a highlight from a QueryResult.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
result: QueryResult from query_pixel or query_range
|
result: QueryResult from query_point or query_range
|
||||||
color: RGBA color tuple
|
color: RGBA color tuple
|
||||||
note: Optional annotation
|
note: Optional annotation
|
||||||
tags: Optional categorization tags
|
tags: Optional categorization tags
|
||||||
|
position: Serialized RenderingPosition of the page the result came from
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Highlight instance
|
Highlight instance
|
||||||
@@ -243,6 +242,7 @@ def create_highlight_from_query_result(
|
|||||||
bounds=bounds,
|
bounds=bounds,
|
||||||
color=color,
|
color=color,
|
||||||
text=text,
|
text=text,
|
||||||
|
position=position,
|
||||||
note=note,
|
note=note,
|
||||||
tags=tags or [],
|
tags=tags or [],
|
||||||
timestamp=time()
|
timestamp=time()
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
"""
|
||||||
|
Small JSON-file helpers shared by the per-document stores.
|
||||||
|
|
||||||
|
BookmarkManager and HighlightManager both keep a JSON file per document under a
|
||||||
|
directory, and both had their own copy of "make the directory, try to read it,
|
||||||
|
swallow and print on failure". The duplication is the point of this module; the
|
||||||
|
file formats themselves stay owned by each store.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
import logging
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Any
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
def ensure_dir(path: str | Path) -> Path:
|
||||||
|
"""Return `path` as a Path, creating it and any missing parents."""
|
||||||
|
directory = Path(path)
|
||||||
|
directory.mkdir(parents=True, exist_ok=True)
|
||||||
|
return directory
|
||||||
|
|
||||||
|
|
||||||
|
def read_json(path: Path, default: Any) -> Any:
|
||||||
|
"""
|
||||||
|
Read JSON from `path`, returning `default` if it is missing or unreadable.
|
||||||
|
|
||||||
|
A corrupt store must not stop a book from opening, so failures are logged
|
||||||
|
and swallowed. `default` is returned as given, so pass a fresh mutable if
|
||||||
|
the caller intends to mutate it.
|
||||||
|
"""
|
||||||
|
if not path.exists():
|
||||||
|
return default
|
||||||
|
|
||||||
|
try:
|
||||||
|
with open(path, 'r', encoding='utf-8') as handle:
|
||||||
|
return json.load(handle)
|
||||||
|
except (OSError, ValueError):
|
||||||
|
logger.warning("Could not read %s; ignoring its contents", path, exc_info=True)
|
||||||
|
return default
|
||||||
|
|
||||||
|
|
||||||
|
def write_json(path: Path, data: Any) -> bool:
|
||||||
|
"""
|
||||||
|
Write `data` to `path` as JSON.
|
||||||
|
|
||||||
|
Returns True on success. Failures are logged rather than raised: losing a
|
||||||
|
bookmark is not a reason to take down the reader.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
with open(path, 'w', encoding='utf-8') as handle:
|
||||||
|
json.dump(data, handle, indent=2)
|
||||||
|
return True
|
||||||
|
except (OSError, TypeError, ValueError):
|
||||||
|
logger.error("Could not write %s", path, exc_info=True)
|
||||||
|
return False
|
||||||
@@ -15,7 +15,9 @@ from __future__ import annotations
|
|||||||
from dataclasses import dataclass, asdict
|
from dataclasses import dataclass, asdict
|
||||||
from typing import List, Dict, Tuple, Optional, Any
|
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.abstract.inline import Word
|
||||||
from pyWebLayout.concrete.page import Page
|
from pyWebLayout.concrete.page import Page
|
||||||
from pyWebLayout.concrete.text import Text
|
from pyWebLayout.concrete.text import Text
|
||||||
@@ -320,6 +322,12 @@ class BidirectionalLayouter:
|
|||||||
self._page_chain: Dict[Tuple[float, Tuple[int, int, int]],
|
self._page_chain: Dict[Tuple[float, Tuple[int, int, int]],
|
||||||
RenderingPosition] = {}
|
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,
|
def render_page_forward(self, position: RenderingPosition,
|
||||||
font_scale: float = 1.0) -> Tuple[Page, 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)
|
return (position.chapter_index, position.block_index, position.word_index)
|
||||||
|
|
||||||
def _scale_block_fonts(self, block: Block, font_scale: float) -> Block:
|
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:
|
if font_scale == 1.0 and self.font_family_override is None:
|
||||||
return block
|
return block
|
||||||
|
|
||||||
# This is a simplified implementation
|
key = (id(block), font_scale)
|
||||||
# In practice, we'd need to handle each block type appropriately
|
cached = self._scaled_block_cache.get(key)
|
||||||
if isinstance(block, (Paragraph, Heading)):
|
if cached is not None:
|
||||||
scaled_block_style = FontScaler.scale_font(block.style, font_scale, self.font_family_override)
|
return cached[1]
|
||||||
if isinstance(block, Heading):
|
|
||||||
scaled_block = Heading(block.level, scaled_block_style)
|
|
||||||
else:
|
|
||||||
scaled_block = Paragraph(scaled_block_style)
|
|
||||||
|
|
||||||
# words_iter() returns tuples of (position, word)
|
scaled = self._build_scaled_block(block, font_scale)
|
||||||
for position, word in block.words_iter():
|
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):
|
if isinstance(word, Word):
|
||||||
scaled_word = Word(
|
scaled_block.add_word(word.with_style(scale(word.style)))
|
||||||
word.text, FontScaler.scale_font(
|
|
||||||
word.style, font_scale, self.font_family_override))
|
|
||||||
scaled_block.add_word(scaled_word)
|
|
||||||
return scaled_block
|
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 <thead>
|
||||||
|
# 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
|
return block
|
||||||
|
|
||||||
def _layout_block_on_page(self,
|
def _layout_block_on_page(self,
|
||||||
@@ -725,60 +793,6 @@ class BidirectionalLayouter:
|
|||||||
# Keep same position so it will be attempted on the next page
|
# Keep same position so it will be attempted on the next page
|
||||||
return False, position
|
return False, position
|
||||||
|
|
||||||
def _estimate_page_start(
|
|
||||||
self,
|
|
||||||
end_position: RenderingPosition,
|
|
||||||
font_scale: float) -> RenderingPosition:
|
|
||||||
"""Estimate where a page should start to end at the given position"""
|
|
||||||
# This is a simplified heuristic - a full implementation would be more
|
|
||||||
# sophisticated
|
|
||||||
estimated_start = end_position.copy()
|
|
||||||
|
|
||||||
# Move back by an estimated number of blocks that would fit on a page
|
|
||||||
estimated_blocks_per_page = max(1, int(10 / font_scale)) # Rough estimate
|
|
||||||
estimated_start.block_index = max(
|
|
||||||
0, end_position.block_index - estimated_blocks_per_page)
|
|
||||||
estimated_start.word_index = 0
|
|
||||||
|
|
||||||
return estimated_start
|
|
||||||
|
|
||||||
def _adjust_start_estimate(
|
|
||||||
self,
|
|
||||||
current_start: RenderingPosition,
|
|
||||||
target_end: RenderingPosition,
|
|
||||||
actual_end: RenderingPosition) -> RenderingPosition:
|
|
||||||
"""
|
|
||||||
Adjust start position estimate based on overshoot/undershoot.
|
|
||||||
Uses proportional adjustment to converge faster.
|
|
||||||
"""
|
|
||||||
adjusted = current_start.copy()
|
|
||||||
|
|
||||||
# Calculate the difference between actual and target end positions
|
|
||||||
block_diff = actual_end.block_index - target_end.block_index
|
|
||||||
|
|
||||||
comparison = self._position_compare(actual_end, target_end)
|
|
||||||
|
|
||||||
if comparison < 0: # Undershot - rendered to block X but need to reach block Y where X < Y
|
|
||||||
# We didn't render far enough forward
|
|
||||||
# Need to start at a LATER block (higher index) so the page includes more content
|
|
||||||
adjustment = max(1, abs(block_diff) // 2)
|
|
||||||
new_index = adjusted.block_index + adjustment
|
|
||||||
# Clamp to valid range
|
|
||||||
if len(self.blocks) > 0:
|
|
||||||
adjusted.block_index = min(len(self.blocks) - 1, max(0, new_index))
|
|
||||||
else:
|
|
||||||
adjusted.block_index = max(0, new_index)
|
|
||||||
elif comparison > 0: # Overshot - rendered past the target
|
|
||||||
# We rendered too far forward
|
|
||||||
# Need to start at an EARLIER block (lower index) so the page doesn't go as far
|
|
||||||
adjustment = max(1, abs(block_diff) // 2)
|
|
||||||
adjusted.block_index = max(0, adjusted.block_index - adjustment)
|
|
||||||
|
|
||||||
# Reset word index when adjusting blocks
|
|
||||||
adjusted.word_index = 0
|
|
||||||
|
|
||||||
return adjusted
|
|
||||||
|
|
||||||
def _position_compare(self, pos1: RenderingPosition,
|
def _position_compare(self, pos1: RenderingPosition,
|
||||||
pos2: RenderingPosition) -> int:
|
pos2: RenderingPosition) -> int:
|
||||||
"""Compare two positions (-1: pos1 < pos2, 0: equal, 1: pos1 > pos2)"""
|
"""Compare two positions (-1: pos1 < pos2, 0: equal, 1: pos1 > pos2)"""
|
||||||
@@ -789,27 +803,3 @@ class BidirectionalLayouter:
|
|||||||
if pos1.word_index != pos2.word_index:
|
if pos1.word_index != pos2.word_index:
|
||||||
return 1 if pos1.word_index > pos2.word_index else -1
|
return 1 if pos1.word_index > pos2.word_index else -1
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
|
||||||
# Add can_fit_line method to Page class if it doesn't exist
|
|
||||||
def _add_page_methods():
|
|
||||||
"""Add missing methods to Page class"""
|
|
||||||
if not hasattr(Page, 'can_fit_line'):
|
|
||||||
def can_fit_line(self, line_height: int) -> bool:
|
|
||||||
"""Check if a line of given height can fit on the page"""
|
|
||||||
available_height = self.content_size[1] - self._current_y_offset
|
|
||||||
return available_height >= line_height
|
|
||||||
|
|
||||||
Page.can_fit_line = can_fit_line
|
|
||||||
|
|
||||||
if not hasattr(Page, 'available_width'):
|
|
||||||
@property
|
|
||||||
def available_width(self) -> int:
|
|
||||||
"""Get available width for content"""
|
|
||||||
return self.content_size[0]
|
|
||||||
|
|
||||||
Page.available_width = available_width
|
|
||||||
|
|
||||||
|
|
||||||
# Apply the page methods
|
|
||||||
_add_page_methods()
|
|
||||||
|
|||||||
@@ -8,9 +8,7 @@ into a unified, easy-to-use API.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
from typing import List, Dict, Optional, Tuple, Any, Callable
|
from typing import List, Dict, Optional, Tuple, Any, Callable
|
||||||
import json
|
|
||||||
import logging
|
import logging
|
||||||
from pathlib import Path
|
|
||||||
|
|
||||||
from .ereader_layout import RenderingPosition, ChapterNavigator, ChapterInfo
|
from .ereader_layout import RenderingPosition, ChapterNavigator, ChapterInfo
|
||||||
from .page_buffer import BufferedPageRenderer
|
from .page_buffer import BufferedPageRenderer
|
||||||
@@ -20,6 +18,11 @@ from pyWebLayout.concrete.image import RenderableImage
|
|||||||
from pyWebLayout.style.page_style import PageStyle
|
from pyWebLayout.style.page_style import PageStyle
|
||||||
from pyWebLayout.style.fonts import BundledFont
|
from pyWebLayout.style.fonts import BundledFont
|
||||||
from pyWebLayout.layout.document_layouter import image_layouter
|
from pyWebLayout.layout.document_layouter import image_layouter
|
||||||
|
from pyWebLayout.core.highlight import Highlight, HighlightColor, HighlightManager, \
|
||||||
|
create_highlight_from_query_result
|
||||||
|
from pyWebLayout.core.persistence import ensure_dir, read_json, write_json
|
||||||
|
from pyWebLayout.concrete.interaction_handler import InteractionStateManager
|
||||||
|
from PIL import Image as Image_
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -38,8 +41,7 @@ class BookmarkManager:
|
|||||||
bookmarks_dir: Directory to store bookmark files
|
bookmarks_dir: Directory to store bookmark files
|
||||||
"""
|
"""
|
||||||
self.document_id = document_id
|
self.document_id = document_id
|
||||||
self.bookmarks_dir = Path(bookmarks_dir)
|
self.bookmarks_dir = ensure_dir(bookmarks_dir)
|
||||||
self.bookmarks_dir.mkdir(exist_ok=True)
|
|
||||||
|
|
||||||
self.bookmarks_file = self.bookmarks_dir / f"{document_id}_bookmarks.json"
|
self.bookmarks_file = self.bookmarks_dir / f"{document_id}_bookmarks.json"
|
||||||
self.position_file = self.bookmarks_dir / f"{document_id}_position.json"
|
self.position_file = self.bookmarks_dir / f"{document_id}_position.json"
|
||||||
@@ -49,29 +51,23 @@ class BookmarkManager:
|
|||||||
|
|
||||||
def _load_bookmarks(self):
|
def _load_bookmarks(self):
|
||||||
"""Load bookmarks from file"""
|
"""Load bookmarks from file"""
|
||||||
if self.bookmarks_file.exists():
|
data = read_json(self.bookmarks_file, {})
|
||||||
try:
|
try:
|
||||||
with open(self.bookmarks_file, 'r') as f:
|
self._bookmarks = {
|
||||||
data = json.load(f)
|
name: RenderingPosition.from_dict(pos_data)
|
||||||
self._bookmarks = {
|
for name, pos_data in data.items()
|
||||||
name: RenderingPosition.from_dict(pos_data)
|
}
|
||||||
for name, pos_data in data.items()
|
except (AttributeError, TypeError, KeyError):
|
||||||
}
|
logger.warning("Bookmark file %s is not in the expected shape; ignoring it",
|
||||||
except Exception as e:
|
self.bookmarks_file, exc_info=True)
|
||||||
print(f"Failed to load bookmarks: {e}")
|
self._bookmarks = {}
|
||||||
self._bookmarks = {}
|
|
||||||
|
|
||||||
def _save_bookmarks(self):
|
def _save_bookmarks(self):
|
||||||
"""Save bookmarks to file"""
|
"""Save bookmarks to file"""
|
||||||
try:
|
write_json(self.bookmarks_file, {
|
||||||
data = {
|
name: position.to_dict()
|
||||||
name: position.to_dict()
|
for name, position in self._bookmarks.items()
|
||||||
for name, position in self._bookmarks.items()
|
})
|
||||||
}
|
|
||||||
with open(self.bookmarks_file, 'w') as f:
|
|
||||||
json.dump(data, f, indent=2)
|
|
||||||
except Exception as e:
|
|
||||||
print(f"Failed to save bookmarks: {e}")
|
|
||||||
|
|
||||||
def add_bookmark(self, name: str, position: RenderingPosition):
|
def add_bookmark(self, name: str, position: RenderingPosition):
|
||||||
"""
|
"""
|
||||||
@@ -128,11 +124,7 @@ class BookmarkManager:
|
|||||||
Args:
|
Args:
|
||||||
position: Current reading position
|
position: Current reading position
|
||||||
"""
|
"""
|
||||||
try:
|
write_json(self.position_file, position.to_dict())
|
||||||
with open(self.position_file, 'w') as f:
|
|
||||||
json.dump(position.to_dict(), f, indent=2)
|
|
||||||
except Exception as e:
|
|
||||||
print(f"Failed to save reading position: {e}")
|
|
||||||
|
|
||||||
def load_reading_position(self) -> Optional[RenderingPosition]:
|
def load_reading_position(self) -> Optional[RenderingPosition]:
|
||||||
"""
|
"""
|
||||||
@@ -141,14 +133,15 @@ class BookmarkManager:
|
|||||||
Returns:
|
Returns:
|
||||||
Last reading position or None if not found
|
Last reading position or None if not found
|
||||||
"""
|
"""
|
||||||
if self.position_file.exists():
|
data = read_json(self.position_file, None)
|
||||||
try:
|
if data is None:
|
||||||
with open(self.position_file, 'r') as f:
|
return None
|
||||||
data = json.load(f)
|
try:
|
||||||
return RenderingPosition.from_dict(data)
|
return RenderingPosition.from_dict(data)
|
||||||
except Exception as e:
|
except (TypeError, KeyError):
|
||||||
print(f"Failed to load reading position: {e}")
|
logger.warning("Position file %s is not in the expected shape; ignoring it",
|
||||||
return None
|
self.position_file, exc_info=True)
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
class EreaderLayoutManager:
|
class EreaderLayoutManager:
|
||||||
@@ -171,7 +164,8 @@ class EreaderLayoutManager:
|
|||||||
document_id: str = "default",
|
document_id: str = "default",
|
||||||
buffer_size: int = 5,
|
buffer_size: int = 5,
|
||||||
page_style: Optional[PageStyle] = None,
|
page_style: Optional[PageStyle] = None,
|
||||||
bookmarks_dir: str = "bookmarks"):
|
bookmarks_dir: str = "bookmarks",
|
||||||
|
highlights_dir: Optional[str] = None):
|
||||||
"""
|
"""
|
||||||
Initialize the ereader layout manager.
|
Initialize the ereader layout manager.
|
||||||
|
|
||||||
@@ -182,6 +176,8 @@ class EreaderLayoutManager:
|
|||||||
buffer_size: Number of pages to cache in each direction
|
buffer_size: Number of pages to cache in each direction
|
||||||
page_style: Custom page styling (uses default if None)
|
page_style: Custom page styling (uses default if None)
|
||||||
bookmarks_dir: Directory to store bookmark files
|
bookmarks_dir: Directory to store bookmark files
|
||||||
|
highlights_dir: Directory to store highlights. Defaults to
|
||||||
|
bookmarks_dir, so a document's reading state lives in one place.
|
||||||
"""
|
"""
|
||||||
self.blocks = blocks
|
self.blocks = blocks
|
||||||
self.page_size = page_size
|
self.page_size = page_size
|
||||||
@@ -196,6 +192,8 @@ class EreaderLayoutManager:
|
|||||||
self.renderer = BufferedPageRenderer(blocks, page_style, buffer_size, page_size)
|
self.renderer = BufferedPageRenderer(blocks, page_style, buffer_size, page_size)
|
||||||
self.chapter_navigator = ChapterNavigator(blocks)
|
self.chapter_navigator = ChapterNavigator(blocks)
|
||||||
self.bookmark_manager = BookmarkManager(document_id, bookmarks_dir)
|
self.bookmark_manager = BookmarkManager(document_id, bookmarks_dir)
|
||||||
|
self.highlight_manager = HighlightManager(
|
||||||
|
document_id, highlights_dir if highlights_dir is not None else bookmarks_dir)
|
||||||
|
|
||||||
# Current state
|
# Current state
|
||||||
self.current_position = RenderingPosition()
|
self.current_position = RenderingPosition()
|
||||||
@@ -216,6 +214,10 @@ class EreaderLayoutManager:
|
|||||||
self.current_position = saved_position
|
self.current_position = saved_position
|
||||||
self._on_cover_page = False # If we have a saved position, we're past the cover
|
self._on_cover_page = False # If we have a saved position, we're past the cover
|
||||||
|
|
||||||
|
# Pointer interaction state, rebound whenever the displayed page changes
|
||||||
|
self._interaction_state_manager: Optional[InteractionStateManager] = None
|
||||||
|
self._interaction_page: Optional[Page] = None
|
||||||
|
|
||||||
# Callbacks for UI updates
|
# Callbacks for UI updates
|
||||||
self.position_changed_callback: Optional[Callable[[
|
self.position_changed_callback: Optional[Callable[[
|
||||||
RenderingPosition], None]] = None
|
RenderingPosition], None]] = None
|
||||||
@@ -848,6 +850,165 @@ class EreaderLayoutManager:
|
|||||||
"""
|
"""
|
||||||
return self.bookmark_manager.list_bookmarks()
|
return self.bookmark_manager.list_bookmarks()
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Highlights
|
||||||
|
#
|
||||||
|
# A Highlight carries pixel bounds, which belong to the one rendering it
|
||||||
|
# was taken from: change the font scale or page size and they no longer
|
||||||
|
# describe anything. Each highlight therefore also records the
|
||||||
|
# RenderingPosition of the page it was made on, and page association goes
|
||||||
|
# through that rather than through the bounds.
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
def highlight_point(self,
|
||||||
|
point: Tuple[int, int],
|
||||||
|
color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
|
||||||
|
note: Optional[str] = None,
|
||||||
|
tags: Optional[List[str]] = None) -> Optional[Highlight]:
|
||||||
|
"""
|
||||||
|
Highlight whatever is at a point on the current page.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
point: (x, y) in page coordinates, as delivered by a tap
|
||||||
|
color: RGBA fill, e.g. one of HighlightColor
|
||||||
|
note: Optional annotation
|
||||||
|
tags: Optional categorization tags
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The stored Highlight, or None if nothing was at that point.
|
||||||
|
"""
|
||||||
|
result = self.get_current_page().query_point(point)
|
||||||
|
if result is None or result.object_type == "empty":
|
||||||
|
return None
|
||||||
|
|
||||||
|
return self._store_highlight(result, color, note, tags)
|
||||||
|
|
||||||
|
def highlight_range(self,
|
||||||
|
start: Tuple[int, int],
|
||||||
|
end: Tuple[int, int],
|
||||||
|
color: Tuple[int, int, int, int] = HighlightColor.YELLOW.value,
|
||||||
|
note: Optional[str] = None,
|
||||||
|
tags: Optional[List[str]] = None) -> Optional[Highlight]:
|
||||||
|
"""
|
||||||
|
Highlight the text between two points on the current page.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
start: (x, y) where the selection began
|
||||||
|
end: (x, y) where the selection ended
|
||||||
|
color: RGBA fill, e.g. one of HighlightColor
|
||||||
|
note: Optional annotation
|
||||||
|
tags: Optional categorization tags
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The stored Highlight, or None if the range selected no text.
|
||||||
|
"""
|
||||||
|
selection = self.get_current_page().query_range(start, end)
|
||||||
|
if not selection.results:
|
||||||
|
return None
|
||||||
|
|
||||||
|
return self._store_highlight(selection, color, note, tags)
|
||||||
|
|
||||||
|
def _store_highlight(self, result, color, note, tags) -> Highlight:
|
||||||
|
"""Build a Highlight from a query result and persist it."""
|
||||||
|
highlight = create_highlight_from_query_result(
|
||||||
|
result, color=color, note=note, tags=tags,
|
||||||
|
position=self.current_position.to_dict())
|
||||||
|
self.highlight_manager.add_highlight(highlight)
|
||||||
|
return highlight
|
||||||
|
|
||||||
|
def remove_highlight(self, highlight_id: str) -> bool:
|
||||||
|
"""
|
||||||
|
Remove a highlight.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
highlight_id: ID of the highlight to remove
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True if it existed and was removed
|
||||||
|
"""
|
||||||
|
return self.highlight_manager.remove_highlight(highlight_id)
|
||||||
|
|
||||||
|
def list_highlights(self) -> List[Highlight]:
|
||||||
|
"""Get every highlight in this document."""
|
||||||
|
return self.highlight_manager.list_highlights()
|
||||||
|
|
||||||
|
def get_highlights_for_current_page(self) -> List[Highlight]:
|
||||||
|
"""
|
||||||
|
Get the highlights made on the page currently being displayed.
|
||||||
|
|
||||||
|
Matched on the recorded RenderingPosition, so this stays correct across
|
||||||
|
font changes; highlights saved before the position field existed have
|
||||||
|
no position and are never matched.
|
||||||
|
"""
|
||||||
|
current = self.current_position.to_dict()
|
||||||
|
return [h for h in self.highlight_manager.list_highlights()
|
||||||
|
if h.position == current]
|
||||||
|
|
||||||
|
def clear_highlights(self) -> None:
|
||||||
|
"""Remove every highlight in this document."""
|
||||||
|
self.highlight_manager.clear_all()
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# Pointer interaction
|
||||||
|
#
|
||||||
|
# Press/hover feedback is state that belongs to one rendered page, so the
|
||||||
|
# state machine is rebound whenever the displayed page changes. Callers get
|
||||||
|
# a fresh frame back when something changed visually, and None when nothing
|
||||||
|
# did - so a UI can skip a redraw it does not need.
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
|
def _interaction_state(self) -> InteractionStateManager:
|
||||||
|
"""The state machine for the page currently displayed."""
|
||||||
|
page = self.get_current_page()
|
||||||
|
if self._interaction_page is not page:
|
||||||
|
if self._interaction_state_manager is not None:
|
||||||
|
self._interaction_state_manager.reset()
|
||||||
|
self._interaction_state_manager = InteractionStateManager(page)
|
||||||
|
self._interaction_page = page
|
||||||
|
return self._interaction_state_manager
|
||||||
|
|
||||||
|
def handle_hover(self, point: Tuple[int, int]) -> Optional[Image_.Image]:
|
||||||
|
"""
|
||||||
|
Update hover feedback for a pointer at `point`.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
point: (x, y) in page coordinates
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A re-rendered frame if the hover state changed, else None.
|
||||||
|
"""
|
||||||
|
return self._interaction_state().update_hover(point)
|
||||||
|
|
||||||
|
def handle_touch_down(self, point: Tuple[int, int]) -> Optional[Image_.Image]:
|
||||||
|
"""
|
||||||
|
Show pressed feedback for whatever interactive element is at `point`.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
point: (x, y) in page coordinates
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A frame showing the pressed state, or None if nothing interactive
|
||||||
|
is there.
|
||||||
|
"""
|
||||||
|
return self._interaction_state().handle_mouse_down(point)
|
||||||
|
|
||||||
|
def handle_touch_up(self, point: Tuple[int, int]) -> Tuple[Optional[Image_.Image], Any]:
|
||||||
|
"""
|
||||||
|
Release the pressed element and run its action.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
point: (x, y) in page coordinates
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
(frame, callback_result). Both are None if no element was pressed.
|
||||||
|
"""
|
||||||
|
return self._interaction_state().handle_mouse_up(point)
|
||||||
|
|
||||||
|
def reset_interaction_state(self) -> None:
|
||||||
|
"""Clear any hover or press feedback, e.g. when the pointer leaves."""
|
||||||
|
if self._interaction_state_manager is not None:
|
||||||
|
self._interaction_state_manager.reset()
|
||||||
|
|
||||||
def get_reading_progress(self) -> float:
|
def get_reading_progress(self) -> float:
|
||||||
"""
|
"""
|
||||||
Get reading progress as a percentage.
|
Get reading progress as a percentage.
|
||||||
|
|||||||
@@ -0,0 +1,168 @@
|
|||||||
|
"""
|
||||||
|
Tests for the highlight API on EreaderLayoutManager (R7).
|
||||||
|
|
||||||
|
core/highlight.py was fully implemented and tested but unreachable: the manager
|
||||||
|
had no highlight API, so highlighting could not be used through the library's
|
||||||
|
own interface. These tests cover the wiring, not the dataclass - that is
|
||||||
|
tests/core/test_highlight.py.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from pyWebLayout.core.highlight import Highlight, HighlightColor
|
||||||
|
from pyWebLayout.io.readers.html_extraction import parse_html_string
|
||||||
|
from pyWebLayout.layout.ereader_manager import EreaderLayoutManager
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def manager(tmp_path):
|
||||||
|
blocks = parse_html_string(
|
||||||
|
"<p>" + " ".join(f"word{i}" for i in range(300)) + "</p>")
|
||||||
|
manager = EreaderLayoutManager(blocks, page_size=(400, 600),
|
||||||
|
document_id="highlights",
|
||||||
|
bookmarks_dir=str(tmp_path))
|
||||||
|
yield manager
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
|
||||||
|
def text_points(page, limit=None):
|
||||||
|
"""Points on the rendered page that land on a text object."""
|
||||||
|
found = []
|
||||||
|
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 == "text" and result.text:
|
||||||
|
found.append((x, y))
|
||||||
|
if limit and len(found) >= limit:
|
||||||
|
return found
|
||||||
|
return found
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def point_on_text(manager):
|
||||||
|
page = manager.get_current_page()
|
||||||
|
page.render()
|
||||||
|
return text_points(page, limit=1)[0]
|
||||||
|
|
||||||
|
|
||||||
|
class TestHighlightPoint:
|
||||||
|
def test_highlighting_a_word_returns_a_stored_highlight(self, manager, point_on_text):
|
||||||
|
highlight = manager.highlight_point(point_on_text)
|
||||||
|
|
||||||
|
assert isinstance(highlight, Highlight)
|
||||||
|
assert highlight.text
|
||||||
|
assert manager.list_highlights() == [highlight]
|
||||||
|
|
||||||
|
def test_colour_note_and_tags_are_kept(self, manager, point_on_text):
|
||||||
|
highlight = manager.highlight_point(
|
||||||
|
point_on_text, color=HighlightColor.GREEN.value,
|
||||||
|
note="a note", tags=["review"])
|
||||||
|
|
||||||
|
assert highlight.color == HighlightColor.GREEN.value
|
||||||
|
assert highlight.note == "a note"
|
||||||
|
assert highlight.tags == ["review"]
|
||||||
|
|
||||||
|
def test_highlighting_empty_space_returns_none(self, manager):
|
||||||
|
manager.get_current_page().render()
|
||||||
|
|
||||||
|
assert manager.highlight_point((399, 599)) is None
|
||||||
|
assert manager.list_highlights() == []
|
||||||
|
|
||||||
|
def test_the_originating_position_is_recorded(self, manager, point_on_text):
|
||||||
|
highlight = manager.highlight_point(point_on_text)
|
||||||
|
|
||||||
|
assert highlight.position == manager.current_position.to_dict()
|
||||||
|
|
||||||
|
|
||||||
|
class TestHighlightRange:
|
||||||
|
def test_a_selection_spans_multiple_words(self, manager):
|
||||||
|
page = manager.get_current_page()
|
||||||
|
page.render()
|
||||||
|
points = text_points(page)
|
||||||
|
|
||||||
|
highlight = manager.highlight_range(points[0], points[-1])
|
||||||
|
|
||||||
|
assert highlight is not None
|
||||||
|
assert len(highlight.text.split()) > 1
|
||||||
|
assert len(highlight.bounds) > 1
|
||||||
|
|
||||||
|
def test_a_selection_hitting_no_text_returns_none(self, manager):
|
||||||
|
manager.get_current_page().render()
|
||||||
|
|
||||||
|
assert manager.highlight_range((398, 596), (399, 599)) is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestHighlightsAreScopedToTheirPage:
|
||||||
|
def test_current_page_highlights_do_not_leak_across_pages(self, manager, point_on_text):
|
||||||
|
manager.highlight_point(point_on_text)
|
||||||
|
assert len(manager.get_highlights_for_current_page()) == 1
|
||||||
|
|
||||||
|
manager.next_page()
|
||||||
|
|
||||||
|
assert manager.get_highlights_for_current_page() == []
|
||||||
|
assert len(manager.list_highlights()) == 1, "still in the document, just not here"
|
||||||
|
|
||||||
|
def test_returning_to_the_page_finds_it_again(self, manager, point_on_text):
|
||||||
|
highlight = manager.highlight_point(point_on_text)
|
||||||
|
manager.next_page()
|
||||||
|
manager.previous_page()
|
||||||
|
|
||||||
|
assert manager.get_highlights_for_current_page() == [highlight]
|
||||||
|
|
||||||
|
|
||||||
|
class TestPersistence:
|
||||||
|
def test_highlights_survive_a_restart(self, manager, point_on_text, tmp_path):
|
||||||
|
highlight = manager.highlight_point(point_on_text, note="kept")
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
reopened = EreaderLayoutManager(
|
||||||
|
manager.blocks, page_size=(400, 600), document_id="highlights",
|
||||||
|
bookmarks_dir=str(tmp_path))
|
||||||
|
try:
|
||||||
|
restored = reopened.list_highlights()
|
||||||
|
assert len(restored) == 1
|
||||||
|
assert restored[0].id == highlight.id
|
||||||
|
assert restored[0].note == "kept"
|
||||||
|
assert restored[0].position == highlight.position
|
||||||
|
finally:
|
||||||
|
reopened.shutdown()
|
||||||
|
|
||||||
|
def test_highlights_share_the_bookmarks_directory_by_default(self, manager,
|
||||||
|
point_on_text, tmp_path):
|
||||||
|
manager.highlight_point(point_on_text)
|
||||||
|
|
||||||
|
assert (tmp_path / "highlights_highlights.json").exists()
|
||||||
|
|
||||||
|
def test_removing_a_highlight_persists(self, manager, point_on_text, tmp_path):
|
||||||
|
highlight = manager.highlight_point(point_on_text)
|
||||||
|
|
||||||
|
assert manager.remove_highlight(highlight.id) is True
|
||||||
|
assert manager.remove_highlight(highlight.id) is False
|
||||||
|
|
||||||
|
reopened = EreaderLayoutManager(
|
||||||
|
manager.blocks, page_size=(400, 600), document_id="highlights",
|
||||||
|
bookmarks_dir=str(tmp_path))
|
||||||
|
try:
|
||||||
|
assert reopened.list_highlights() == []
|
||||||
|
finally:
|
||||||
|
reopened.shutdown()
|
||||||
|
|
||||||
|
def test_clear_removes_everything(self, manager, point_on_text):
|
||||||
|
manager.highlight_point(point_on_text)
|
||||||
|
|
||||||
|
manager.clear_highlights()
|
||||||
|
|
||||||
|
assert manager.list_highlights() == []
|
||||||
|
|
||||||
|
def test_a_corrupt_store_does_not_stop_the_book_opening(self, tmp_path):
|
||||||
|
(tmp_path / "broken_highlights.json").write_text("{not json")
|
||||||
|
blocks = parse_html_string("<p>hello world</p>")
|
||||||
|
|
||||||
|
manager = EreaderLayoutManager(blocks, page_size=(400, 600),
|
||||||
|
document_id="broken",
|
||||||
|
bookmarks_dir=str(tmp_path))
|
||||||
|
try:
|
||||||
|
assert manager.list_highlights() == []
|
||||||
|
assert manager.get_current_page() is not None
|
||||||
|
finally:
|
||||||
|
manager.shutdown()
|
||||||
@@ -0,0 +1,129 @@
|
|||||||
|
"""
|
||||||
|
Tests for pointer interaction on EreaderLayoutManager (R7).
|
||||||
|
|
||||||
|
concrete/interaction_handler.py was 310 lines reachable only from
|
||||||
|
examples/07_pressed_state_demo.py - no library code, no tests. These cover the
|
||||||
|
wiring; the press/hover state on the elements themselves lives in
|
||||||
|
tests/concrete/.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from PIL import Image
|
||||||
|
|
||||||
|
from pyWebLayout.io.readers.html_extraction import parse_html_string
|
||||||
|
from pyWebLayout.layout.ereader_manager import EreaderLayoutManager
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def manager(tmp_path):
|
||||||
|
blocks = parse_html_string(
|
||||||
|
'<p>Tap <a href="action:go">this link</a> please.</p>'
|
||||||
|
'<p>' + " ".join(f"w{i}" for i in range(400)) + '</p>')
|
||||||
|
manager = EreaderLayoutManager(blocks, page_size=(400, 600),
|
||||||
|
document_id="interaction",
|
||||||
|
bookmarks_dir=str(tmp_path))
|
||||||
|
yield manager
|
||||||
|
manager.shutdown()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def link_point(manager):
|
||||||
|
"""A page coordinate that lands on the interactive link."""
|
||||||
|
page = manager.get_current_page()
|
||||||
|
page.render()
|
||||||
|
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.is_interactive:
|
||||||
|
return (x, y)
|
||||||
|
pytest.fail("fixture document rendered no interactive element")
|
||||||
|
|
||||||
|
|
||||||
|
EMPTY_POINT = (399, 599)
|
||||||
|
|
||||||
|
|
||||||
|
class TestHover:
|
||||||
|
def test_hovering_an_element_produces_a_frame(self, manager, link_point):
|
||||||
|
assert isinstance(manager.handle_hover(link_point), Image.Image)
|
||||||
|
|
||||||
|
def test_hovering_the_same_element_again_reports_no_change(self, manager, link_point):
|
||||||
|
manager.handle_hover(link_point)
|
||||||
|
|
||||||
|
assert manager.handle_hover(link_point) is None, \
|
||||||
|
"an unchanged hover should not force the caller to redraw"
|
||||||
|
|
||||||
|
def test_moving_off_the_element_clears_the_hover(self, manager, link_point):
|
||||||
|
manager.handle_hover(link_point)
|
||||||
|
|
||||||
|
assert isinstance(manager.handle_hover(EMPTY_POINT), Image.Image)
|
||||||
|
|
||||||
|
|
||||||
|
class TestPress:
|
||||||
|
def test_pressing_an_element_produces_a_frame(self, manager, link_point):
|
||||||
|
assert isinstance(manager.handle_touch_down(link_point), Image.Image)
|
||||||
|
|
||||||
|
def test_pressing_empty_space_does_nothing(self, manager):
|
||||||
|
manager.get_current_page().render()
|
||||||
|
|
||||||
|
assert manager.handle_touch_down(EMPTY_POINT) is None
|
||||||
|
|
||||||
|
def test_release_runs_the_link_action(self, manager, link_point):
|
||||||
|
manager.handle_touch_down(link_point)
|
||||||
|
|
||||||
|
frame, result = manager.handle_touch_up(link_point)
|
||||||
|
|
||||||
|
assert isinstance(frame, Image.Image)
|
||||||
|
assert result == "action:go"
|
||||||
|
|
||||||
|
def test_release_without_a_press_is_a_no_op(self, manager):
|
||||||
|
manager.get_current_page().render()
|
||||||
|
|
||||||
|
assert manager.handle_touch_up(EMPTY_POINT) == (None, None)
|
||||||
|
|
||||||
|
def test_a_full_press_release_cycle_leaves_no_state(self, manager, link_point):
|
||||||
|
manager.handle_touch_down(link_point)
|
||||||
|
manager.handle_touch_up(link_point)
|
||||||
|
|
||||||
|
assert manager.handle_touch_up(link_point) == (None, None)
|
||||||
|
|
||||||
|
|
||||||
|
class TestStateFollowsTheDisplayedPage:
|
||||||
|
def test_navigating_rebinds_the_state_machine(self, manager, link_point):
|
||||||
|
before = manager._interaction_state()
|
||||||
|
|
||||||
|
manager.next_page()
|
||||||
|
|
||||||
|
assert manager._interaction_state() is not before, \
|
||||||
|
"press state belongs to one rendered page"
|
||||||
|
|
||||||
|
def test_state_survives_repeated_access_on_one_page(self, manager, link_point):
|
||||||
|
assert manager._interaction_state() is manager._interaction_state()
|
||||||
|
|
||||||
|
def test_reset_is_safe_before_any_interaction(self, manager):
|
||||||
|
manager.reset_interaction_state() # must not raise
|
||||||
|
|
||||||
|
def test_reset_clears_a_pending_press(self, manager, link_point):
|
||||||
|
manager.handle_touch_down(link_point)
|
||||||
|
|
||||||
|
manager.reset_interaction_state()
|
||||||
|
|
||||||
|
assert manager.handle_touch_up(link_point) == (None, None)
|
||||||
|
|
||||||
|
|
||||||
|
class TestPressedRenderingRegression:
|
||||||
|
"""
|
||||||
|
LinkText.render passed [origin, origin + size] - two numpy arrays - to
|
||||||
|
PIL's draw.rectangle, which needs a flat four-scalar box. Rendering any
|
||||||
|
hovered or pressed link raised TypeError. Nothing caught it because the
|
||||||
|
only caller was an example.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_rendering_a_hovered_link_does_not_raise(self, manager, link_point):
|
||||||
|
manager.handle_hover(link_point)
|
||||||
|
|
||||||
|
assert isinstance(manager.get_current_page().render(), Image.Image)
|
||||||
|
|
||||||
|
def test_rendering_a_pressed_link_does_not_raise(self, manager, link_point):
|
||||||
|
manager.handle_touch_down(link_point)
|
||||||
|
|
||||||
|
assert isinstance(manager.get_current_page().render(), Image.Image)
|
||||||
@@ -570,30 +570,6 @@ class TestBidirectionalLayouter:
|
|||||||
# Should return same block
|
# Should return same block
|
||||||
assert scaled == paragraph
|
assert scaled == paragraph
|
||||||
|
|
||||||
def test_estimate_page_start(self):
|
|
||||||
"""Test estimation of page start position."""
|
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
|
||||||
|
|
||||||
end_pos = RenderingPosition(chapter_index=0, block_index=20, word_index=0)
|
|
||||||
|
|
||||||
estimated = layouter._estimate_page_start(end_pos, 1.0)
|
|
||||||
|
|
||||||
# Should estimate some blocks before the end position
|
|
||||||
assert estimated.block_index < end_pos.block_index
|
|
||||||
assert estimated.block_index >= 0
|
|
||||||
|
|
||||||
def test_estimate_page_start_with_font_scale(self):
|
|
||||||
"""Test that font scale affects page start estimation."""
|
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
|
||||||
|
|
||||||
end_pos = RenderingPosition(chapter_index=0, block_index=20, word_index=0)
|
|
||||||
|
|
||||||
est_normal = layouter._estimate_page_start(end_pos, 1.0)
|
|
||||||
est_large = layouter._estimate_page_start(end_pos, 2.0)
|
|
||||||
|
|
||||||
# Larger font should estimate fewer blocks
|
|
||||||
assert est_large.block_index >= est_normal.block_index
|
|
||||||
|
|
||||||
def test_scale_block_fonts_paragraph(self, sample_font):
|
def test_scale_block_fonts_paragraph(self, sample_font):
|
||||||
"""Test scaling fonts in a paragraph block."""
|
"""Test scaling fonts in a paragraph block."""
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
layouter = BidirectionalLayouter([], PageStyle())
|
||||||
@@ -784,50 +760,6 @@ class TestBidirectionalLayouter:
|
|||||||
# Start position should be before or at end position
|
# Start position should be before or at end position
|
||||||
assert start_pos.block_index <= end_position.block_index
|
assert start_pos.block_index <= end_position.block_index
|
||||||
|
|
||||||
def test_adjust_start_estimate_overshot(self):
|
|
||||||
"""Test adjustment when forward render overshoots target."""
|
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
|
||||||
|
|
||||||
current_start = RenderingPosition(block_index=5)
|
|
||||||
target_end = RenderingPosition(block_index=10)
|
|
||||||
actual_end = RenderingPosition(block_index=12) # Overshot (went too far)
|
|
||||||
|
|
||||||
adjusted = layouter._adjust_start_estimate(
|
|
||||||
current_start, target_end, actual_end)
|
|
||||||
|
|
||||||
# Overshot means we rendered too far forward
|
|
||||||
# So we need to start EARLIER (decrease block_index) to not go as far
|
|
||||||
assert adjusted.block_index < current_start.block_index
|
|
||||||
|
|
||||||
def test_adjust_start_estimate_undershot(self):
|
|
||||||
"""Test adjustment when forward render undershoots target."""
|
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
|
||||||
|
|
||||||
current_start = RenderingPosition(block_index=5)
|
|
||||||
target_end = RenderingPosition(block_index=10)
|
|
||||||
actual_end = RenderingPosition(block_index=8) # Undershot (didn't go far enough)
|
|
||||||
|
|
||||||
adjusted = layouter._adjust_start_estimate(
|
|
||||||
current_start, target_end, actual_end)
|
|
||||||
|
|
||||||
# Undershot means we didn't render far enough forward
|
|
||||||
# So we need to start LATER (increase block_index) to include more content
|
|
||||||
assert adjusted.block_index > current_start.block_index
|
|
||||||
|
|
||||||
def test_adjust_start_estimate_exact(self):
|
|
||||||
"""Test adjustment when forward render hits target exactly."""
|
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
|
||||||
|
|
||||||
current_start = RenderingPosition(block_index=5)
|
|
||||||
target_end = RenderingPosition(block_index=10)
|
|
||||||
actual_end = RenderingPosition(block_index=10) # Exact
|
|
||||||
|
|
||||||
adjusted = layouter._adjust_start_estimate(
|
|
||||||
current_start, target_end, actual_end)
|
|
||||||
|
|
||||||
# Should return same or similar position
|
|
||||||
assert adjusted.block_index >= 0
|
|
||||||
|
|
||||||
def test_layout_paragraph_on_page_with_pretext(
|
def test_layout_paragraph_on_page_with_pretext(
|
||||||
self, sample_font, sample_page_style):
|
self, sample_font, sample_page_style):
|
||||||
"""Test paragraph layout with pretext (hyphenated word continuation)."""
|
"""Test paragraph layout with pretext (hyphenated word continuation)."""
|
||||||
@@ -899,5 +831,43 @@ class TestBidirectionalLayouter:
|
|||||||
assert next_pos == position # No progress possible
|
assert next_pos == position # No progress possible
|
||||||
|
|
||||||
|
|
||||||
|
class TestNoPageMonkeyPatching:
|
||||||
|
"""
|
||||||
|
R5: importing this module used to run _add_page_methods(), which attached
|
||||||
|
can_fit_line/available_width to Page if they were absent. They are not
|
||||||
|
absent, so it never fired - but its can_fit_line took (line_height) and
|
||||||
|
ignored descenders, while Page's takes (baseline_spacing, ascent, descent).
|
||||||
|
Had Page's ever been renamed, the import would have silently reinstated the
|
||||||
|
pre-S2 clipping bug from another package.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def test_module_does_not_patch_page(self):
|
||||||
|
import pyWebLayout.layout.ereader_layout as ereader_layout
|
||||||
|
|
||||||
|
assert not hasattr(ereader_layout, '_add_page_methods')
|
||||||
|
|
||||||
|
def test_page_owns_its_geometry_methods(self):
|
||||||
|
from pyWebLayout.concrete.page import Page
|
||||||
|
|
||||||
|
assert 'can_fit_line' in vars(Page)
|
||||||
|
assert 'available_width' in vars(Page)
|
||||||
|
|
||||||
|
def test_can_fit_line_still_accounts_for_descenders(self, sample_page_style):
|
||||||
|
"""
|
||||||
|
The patched version took a single line_height and had no way to express
|
||||||
|
descent, so a descender hanging past the content box counted as fitting.
|
||||||
|
"""
|
||||||
|
from pyWebLayout.concrete.page import Page
|
||||||
|
|
||||||
|
page = Page(size=(200, 100), style=sample_page_style)
|
||||||
|
content_y, content_h = page.content_rect[1], page.content_rect[3]
|
||||||
|
available = content_y + content_h - page._current_y_offset
|
||||||
|
|
||||||
|
assert page.can_fit_line(0, ascent=available, descent=0)
|
||||||
|
assert not page.can_fit_line(0, ascent=available, descent=1), \
|
||||||
|
"a descender past the content box must not be reported as fitting"
|
||||||
|
assert page.can_fit_line(0, ascent=available - 1, descent=1)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
pytest.main([__file__, "-v"])
|
pytest.main([__file__, "-v"])
|
||||||
|
|||||||
@@ -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 = """
|
||||||
|
<p>Go to <a href="http://example.com" title="Tooltip">this link</a> now.</p>
|
||||||
|
<blockquote><p>Quoted <a href="http://q.example">qlink</a> text.</p></blockquote>
|
||||||
|
<ul><li>item <a href="http://l.example">llink</a> one</li></ul>
|
||||||
|
<table>
|
||||||
|
<thead><tr><th>head <a href="http://h.example">hlink</a></th></tr></thead>
|
||||||
|
<tbody><tr><td>cell <a href="http://c.example">clink</a></td></tr></tbody>
|
||||||
|
</table>
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
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("<h3>Title here</h3>")[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(
|
||||||
|
'<p>Go to <a href="http://example.com">this link</a> now.</p>')
|
||||||
|
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"}
|
||||||
Reference in New Issue
Block a user