Skip to content

Commit 8664e50

Browse files
phfaistphfaist
committed
data urls
1 parent 3afa30b commit 8664e50

4 files changed

Lines changed: 437 additions & 17 deletions

File tree

‎.claude/archive/PLAN-flm-data-url.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Plan: handle `data:` (and remote) URLs in graphics collection
2+
3+
In broad strokes: Cache downloaded URL bytes in temp files, not in memory.
4+
Inspect image dimensions for all URLs, including `data:` as well as `https:` and
5+
others. Each URL is downloaded once, written to a temp file, and that path is
6+
reused everywhere downstream — so URL graphics flow through the existing
7+
`source_type == 'file'` code paths (inspect, hash, convert, copy) with almost no
8+
special-casing.
9+
10+
## Caching scope: the **Feature** instance (not the DocumentManager)
11+
12+
The download cache (bytes-on-disk + inspected info) lives on the
13+
`FeatureGraphicsCollection` **instance**, not on the per-document
14+
`DocumentManager`. The `DocumentManager` already references its owning feature
15+
(`self.feature`, set in `FeatureDocumentManagerBase.__init__`), and one Feature
16+
instance spawns every DocumentManager, so the cache is reachable from the scan
17+
code via `self.feature` with no plumbing.
18+
19+
### Lifetime analysis (read before relying on cross-render reuse)
20+
21+
- A single `main.run()` builds one environment, one set of Feature instances,
22+
and renders exactly **one** document (`run.py:1122/1164`; document-part
23+
fragments share that same document). For an ordinary CLI invocation, Feature
24+
scope and DocumentManager scope are therefore *identical in effect* — there is
25+
only ever one DocumentManager per Feature.
26+
- **Watch mode** rebuilds the environment and all Feature instances on every
27+
recompile (`watch.py:158` → `main.main()` → `run.py:372`; the `watch.py:145`
28+
comment deliberately avoids object reuse). A Feature-level cache does **not**
29+
survive across recompiles — each save re-downloads.
30+
- Feature scope therefore only beats DocumentManager scope when one environment
31+
is deliberately reused across several `make_document`/render passes (custom
32+
batch driver, multi-format render off one env). It is a strict superset of
33+
per-document dedup and is otherwise harmless.
34+
- **Follow-up (out of scope here):** to cache downloads across watch-mode
35+
recompiles, persistence must move to an even broader scope (a module-level
36+
dict keyed by URL, or persisting the environment across recompiles). Not done
37+
in this plan.
38+
39+
Because the Feature can outlive any single DocumentManager, the temp directory
40+
and the temp-file paths it hands out must remain valid for the whole Feature
41+
lifetime — which they do (see "temp-file lifetime" below).
42+
43+
## Plan
44+
45+
**0. Feature-level cache state (in `FeatureGraphicsCollection.__init__`).**
46+
Add, alongside the existing config attributes:
47+
- `self._url_cache = {}` — keyed by the URL string (`source_url`); value is a
48+
dict `{ 'temp_file_path', 'mimetype', 'detected_ext', 'info', 'input_hash' }`
49+
(or a failure marker, see step 3).
50+
- `self._url_tempdir = None` — created lazily on first URL download via
51+
`tempfile.TemporaryDirectory()` and stored on the Feature, so the temp files'
52+
lifetime is tied to the Feature lifetime.
53+
- `self._url_download_counter = 0` — monotonic, Feature-scoped, used to name temp
54+
files uniquely within the shared temp dir (per-document counters could collide
55+
in a shared dir).
56+
57+
These are plain attributes set in `__init__`; Transcrypt-safe. Mixing runtime
58+
cache state onto the (otherwise config-only) Feature object is intentional and
59+
required by the chosen scope.
60+
61+
**1. New Feature method `fetch_inspect_url(source_url)` — the memoizing core.**
62+
Lives on the Feature (it owns the cache + temp dir + already owns
63+
`inspect_graphics_file`). Behavior:
64+
- If `source_url in self._url_cache`, return the cached entry immediately (this is
65+
the dedup / single-download guarantee, now Feature-wide).
66+
- Otherwise download + inspect once:
67+
- `with urllib.request.urlopen(source_url) as r: content = r.read(); mimetype = r.headers.get_content_type()` — the default opener already has `DataHandler`, so `data:` is handled offline (the same call `collect_graphics` already uses).
68+
- `detected_ext` from a small override dict (`image/png`→`.png`, `image/jpeg`→`.jpg`, `image/svg+xml`→`.svg`, `application/pdf`→`.pdf`, `image/gif`→`.gif`), falling back to `mimetypes.guess_extension(mimetype)` (default `''`).
69+
- Lazily create `self._url_tempdir` if `None`; bump `self._url_download_counter`; write `content` to a temp file named `inline{counter}{detected_ext}` inside the temp dir, so the synthetic name carries the right extension for the inspector and converters.
70+
- Inspect by opening that temp file: `with open(tmp_path, 'rb') as fp: info = self.inspect_graphics_file(tmp_path, fp)`.
71+
- Compute `input_hash` once here from `content` (`hashlib.sha256(content).hexdigest()`), so it's cached with the entry and reused for `${hash}`/cache-skip.
72+
- Store and return `{ 'temp_file_path': tmp_path, 'mimetype', 'detected_ext', 'info': info or {}, 'input_hash' }`.
73+
- Wrap fetch+inspect in try/except: on failure, **warn and cache a failure marker**
74+
`{ 'temp_file_path': None, 'mimetype': None, 'detected_ext': '', 'info': {}, 'input_hash': None }` (preserves today's behavior for unreachable/odd URLs — no hard regression, and we don't retry a known-bad URL on every document).
75+
76+
**2. `get_source_info` — document the multi-call contract, keep it cheap.**
77+
Add a comment that it runs once per resource at scan *and* once per
78+
`\includegraphics` at render (`:1207`), only to classify the source and compute a
79+
stable `source_key`. No network / decode / disk I/O here. Return tuple and
80+
`source_resolved` unchanged; `data:` URLs keep classifying as `'url'`.
81+
82+
**3. Scan branch uses the Feature cache (the `# TODO`, `:839`).**
83+
Rewrite the `elif source_type == 'url':` branch in
84+
`inspect_add_graphics_resource` to delegate:
85+
- `entry = self.feature.fetch_inspect_url(source_url)` (downloads at most once
86+
per URL per Feature lifetime; later documents/renders reuse it).
87+
- `graphics_resource = GraphicsResource(src_url=source_url, **entry['info'])`.
88+
- `self.add_graphics(source_key, source_info, graphics_resource, url_entry=entry)`.
89+
90+
**4. Cache resolved paths in the per-document collection entry.**
91+
Have `add_graphics` (`:858`) accept an optional `url_entry` and store, in
92+
`graphics_collection[source_key]`, the extra fields `temp_file_path` (str or
93+
`None`), `mimetype`, `detected_ext`, and `input_hash`, copied from `url_entry`.
94+
For `file` sources (`url_entry=None`), set
95+
`detected_ext = splitext(full_file_path)[1]`, `temp_file_path=None`,
96+
`mimetype=None`, `input_hash=None`, so the fields are uniformly present. This
97+
per-document dict is internal, never serialized — `GraphicsResource` stays clean.
98+
The `temp_file_path` it stores points into the Feature's temp dir, which outlives
99+
the DocumentManager, so it is valid throughout scan→render→collect.
100+
101+
**5. `prepare_collect_graphics` — use `detected_ext` and the cached hash (the `# TODO (2)`).**
102+
For `source_type == 'url'`, take `ext` from the entry's `detected_ext` (not
103+
`splitext` of the data URL, which yields garbage), and take `input_hash` from the
104+
entry (already computed by the Feature in step 1 — no second read). For `file`
105+
sources keep the existing `hashlib.file_digest(f, 'sha256')` path. So URL graphics
106+
get cache-skip and `${hash}`/`${hash6}` template keys exactly like files.
107+
108+
**6. `collect_graphics` — copy/convert from the temp file.**
109+
In the url branch (`:1170`), if a `temp_file_path` is available (looked up via
110+
`source_key`, or threaded into `collect_info` in step 5), treat it like a local
111+
file: `shutil.copyfile(temp_file_path, target_path)` for the no-converter path,
112+
and pass the temp path (with `source_type='file'`) to `converter.convert(...)` so
113+
converters read it off disk with no second download. Fall back to the existing
114+
`urllib.request.urlopen` copy only when there is no temp file (earlier fetch
115+
failed). Converters stay completely untouched.
116+
117+
**7. Guard the non-collecting `relpath` (`:1228`).**
118+
`os.path.relpath` only when `source_type == 'file'`; otherwise return `src_url`
119+
unchanged so inline `data:` / remote URLs survive verbatim in non-collecting
120+
output.
121+
122+
**8. Imports:** add `tempfile` and `mimetypes` at module top (alongside existing
123+
`urllib.request`). (`io` not needed — inspection opens the temp file directly.)
124+
125+
**9. Tests.** Small base64 PNG `data:` URL through both a collecting render
126+
(assert the collected file ends `.png`, exists on disk, and the
127+
`GraphicsResource` has `physical_dimensions`) and a non-collecting render (assert
128+
the data URL passes through intact). Add one test that reuses a single
129+
`FeatureGraphicsCollection` instance across two documents and asserts the URL is
130+
fetched only once (e.g. by checking `len(feature._url_cache) == 1`, or by
131+
monkeypatching/counting `urlopen`) — this is what the Feature-scoped cache buys.
132+
Use a temp output dir; allowed assertion methods only
133+
(`assertTrue`/`assertEqual`/etc.).
134+
135+
## Notes on temp-file lifetime
136+
137+
- The `TemporaryDirectory` is created lazily (only if at least one URL is
138+
downloaded) and held on the **Feature**, so it persists across every document
139+
and render/collect pass that Feature serves, and is removed automatically when
140+
the Feature is garbage-collected. No unbounded in-memory bytes are retained
141+
(only the bytes-on-disk cache, bounded by the number of distinct URLs).
142+
- Deduplication is per URL across the whole Feature lifetime: a repeated identical
143+
URL — within one document *or across documents that share the Feature* — is
144+
downloaded to a temp file once.
145+
- Caveat (restated): in standard single-document runs and in watch mode the
146+
Feature is not reused across renders, so the cache does not persist across them.
147+
See the lifetime analysis above.
148+
149+
Shall I implement this?

‎flm/main/_inspectimagefile.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def get_image_file_info_pil(filename, fp):
6060

6161
if 'dpi' not in img.info:
6262
raise ValueError(
63-
"Your PNG image does not provide any DPI information. Please fix "
63+
"Your image does not provide any DPI information. Please fix "
6464
"your image so that it has a fixed DPI setting. We need this information "
6565
"to know how large your image is."
6666
)

0 commit comments

Comments
 (0)