From 4635eaa4679cbc89bb48edf93270c7d869890c5f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 19 Oct 2026 22:33:10 +0200 Subject: [PATCH 1/9] :bug: Show request access modal on viewer and dashboard urls (#12456) The exception page compared the screen query param against literals that never occur ("view", "dashboard"), so viewer and dashboard urls of inaccessible files and teams fell through to the plain not-found page instead of the request access modal. Classify the param with a tested helper that recognizes "viewer" and every "dashboard-*" route. Closes #11224 AI-assisted-by: muse-spark-1.3-contributor --- frontend/src/app/main/ui/static.cljs | 30 +++++++++-- frontend/test/frontend_tests/runner.cljs | 2 + .../test/frontend_tests/ui/static_test.cljs | 54 +++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 frontend/test/frontend_tests/ui/static_test.cljs diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index ec5e20a72a..fe75e1f7b2 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -34,6 +34,7 @@ [app.util.timers :as tm] [app.util.webapi :as wapi] [beicon.v2.core :as rx] + [cuerdas.core :as str] [rumext.v2 :as mf])) ;; FIXME: this is a workaround until we export this class on beicon library @@ -608,16 +609,37 @@ [:> internal-error* props]))) +(defn screen-kind + "Classify the `screen` query param for the exception page. + + The param carries the route name (`?screen=¶ms`), so the + viewer is \"viewer\" and every team dashboard is \"dashboard-*\". Returns + `:workspace`, `:dashboard`, `:viewer`, or nil when the screen is not a + request-access candidate. Reads the param with `rt/get-query-param` + because a repeated param arrives as a vector." + [query-params] + (let [screen (rt/get-query-param query-params :screen)] + (cond + (= screen "workspace") + :workspace + + (and (string? screen) + (str/starts-with? screen "dashboard")) + :dashboard + + (= screen "viewer") + :viewer))) + (mf/defc exception-page* [{:keys [data route] :as props}] (let [type (:type data) params (:query-params route) - screen (get params :screen) - workspace? (= screen "workspace") - dashboard? (= screen "dashboard") - view? (= screen "view") + kind (screen-kind params) + workspace? (= kind :workspace) + dashboard? (= kind :dashboard) + view? (= kind :viewer) ;; We store the request access info int this state info* (mf/use-state nil) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index ad9b96d56f..8cea58e9b6 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -125,6 +125,7 @@ [frontend-tests.ui.shape-filters-test] [frontend-tests.ui.shortcuts-labels-test] [frontend-tests.ui.sidebar-scroll-test] + [frontend-tests.ui.static-test] [frontend-tests.ui.stroke-menu-test] [frontend-tests.ui.workspace-libraries-test] [frontend-tests.util-clipboard-test] @@ -272,6 +273,7 @@ 'frontend-tests.ui.shape-filters-test 'frontend-tests.ui.shortcuts-labels-test 'frontend-tests.ui.sidebar-scroll-test + 'frontend-tests.ui.static-test 'frontend-tests.ui.stroke-menu-test 'frontend-tests.ui.workspace-libraries-test 'frontend-tests.util-clipboard-test diff --git a/frontend/test/frontend_tests/ui/static_test.cljs b/frontend/test/frontend_tests/ui/static_test.cljs new file mode 100644 index 0000000000..acdc1b0b2c --- /dev/null +++ b/frontend/test/frontend_tests/ui/static_test.cljs @@ -0,0 +1,54 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns frontend-tests.ui.static-test + (:require + [app.main.ui.static :as static] + [cljs.test :as t :include-macros true])) + +(t/deftest screen-kind-classifies-known-screens + ;; Regression test (penpot/penpot#11224): the exception page shows the + ;; REQUEST ACCESS modal only when it recognizes the screen the failing + ;; URL belongs to. The `screen` query param carries the route name, so + ;; the viewer is "viewer" and every team dashboard is "dashboard-*". + (t/testing "workspace" + (t/is (= :workspace (static/screen-kind {:screen "workspace"})))) + + (t/testing "bare dashboard prefix also classifies" + ;; No such route exists (routes are always "dashboard-*"), and the + ;; modal still needs loaded team/file info, so this is harmless and + ;; matches the prefix convention used in routes.cljs. + (t/is (= :dashboard (static/screen-kind {:screen "dashboard"})))) + + (t/testing "viewer" + (t/is (= :viewer (static/screen-kind {:screen "viewer"})))) + + (t/testing "dashboard variants" + (doseq [screen ["dashboard-recent" + "dashboard-members" + "dashboard-files" + "dashboard-settings"]] + (t/is (= :dashboard (static/screen-kind {:screen screen})) + (str "screen: " screen))))) + +(t/deftest screen-kind-ignores-unknown-screens + ;; Unknown, missing, or auth screens are not request-access candidates: + ;; the exception page must keep showing the plain not-found page. + (doseq [params [nil + {} + {:screen "auth-login"} + {:screen "settings-profile"} + ;; The "view" literal never was a real screen name; the + ;; exception page compared against it before the fix. + {:screen "view"}]] + (t/is (nil? (static/screen-kind params)) + (str "params: " (pr-str params))))) + +(t/deftest screen-kind-tolerates-duplicate-screen-params + ;; A repeated query param arrives as a vector; the classifier reads the + ;; scalar the same way the router does instead of crashing on it. + (t/is (= :dashboard + (static/screen-kind {:screen ["dashboard-recent" "dashboard-recent"]})))) From 60a94c65ccabe34e6f59ae5f1336153931c6a112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Tue, 20 Oct 2026 09:32:53 +0200 Subject: [PATCH 2/9] :bug: Fix missing letters when copying text from exported PDFs (#12441) Skia maps ligatures and other substituted glyphs to U+0000 in the PDF ToUnicode maps. Rebuild those maps from the laid-out paragraphs and append them as an incremental update. Closes #12393 --- render-wasm/Cargo.lock | 1 + render-wasm/Cargo.toml | 1 + render-wasm/src/render/pdf.rs | 8 +- render-wasm/src/render/pdf/glyph_text.rs | 278 ++++++++ render-wasm/src/render/pdf/to_unicode.rs | 793 +++++++++++++++++++++++ 5 files changed, 1080 insertions(+), 1 deletion(-) create mode 100644 render-wasm/src/render/pdf/glyph_text.rs create mode 100644 render-wasm/src/render/pdf/to_unicode.rs diff --git a/render-wasm/Cargo.lock b/render-wasm/Cargo.lock index ee162a0029..1142a20b67 100644 --- a/render-wasm/Cargo.lock +++ b/render-wasm/Cargo.lock @@ -463,6 +463,7 @@ dependencies = [ "indexmap", "insta", "macros", + "miniz_oxide", "skia-safe", "thiserror", "unicode-segmentation", diff --git a/render-wasm/Cargo.toml b/render-wasm/Cargo.toml index 22bb7e1590..e71f616191 100644 --- a/render-wasm/Cargo.toml +++ b/render-wasm/Cargo.toml @@ -26,6 +26,7 @@ gl = "0.14.0" glam = "0.24.2" indexmap = "2.7.1" macros = { path = "macros" } +miniz_oxide = "0.8.0" skia-safe = { version = "0.93.1", default-features = false, features = [ "gl", "svg", diff --git a/render-wasm/src/render/pdf.rs b/render-wasm/src/render/pdf.rs index b8e204dcb6..d70f353ef4 100644 --- a/render-wasm/src/render/pdf.rs +++ b/render-wasm/src/render/pdf.rs @@ -7,6 +7,9 @@ use crate::uuid::Uuid; use super::vector; use super::RenderResources; +mod glyph_text; +mod to_unicode; + /// Renders a shape tree to a PDF document and returns the raw PDF bytes. /// /// This is a dedicated vector-PDF render path that draws directly to a Skia @@ -49,5 +52,8 @@ pub fn render_to_pdf( let document = on_page.end_page(); document.close(); - Ok(pdf_bytes) + // Skia maps ligatures and other substituted glyphs to U+0000; give them + // back their text so it can be copied from the PDF. + let glyph_texts = glyph_text::collect(tree, id); + Ok(to_unicode::fix_to_unicode(pdf_bytes, &glyph_texts)) } diff --git a/render-wasm/src/render/pdf/glyph_text.rs b/render-wasm/src/render/pdf/glyph_text.rs new file mode 100644 index 0000000000..d3623bf87f --- /dev/null +++ b/render-wasm/src/render/pdf/glyph_text.rs @@ -0,0 +1,278 @@ +//! Collects the text behind each drawn glyph, per font. Skia's PDF backend +//! maps substituted glyphs (ligatures, contextual alternates) to U+0000. + +use std::collections::HashMap; + +use skia_safe as skia; + +use crate::shapes::Type; +use crate::state::ShapesPoolRef; +use crate::uuid::Uuid; + +#[derive(Debug, Clone)] +struct GlyphText { + text: String, + /// The glyph is the only one drawn for its cluster. + single_glyph: bool, + typeface: skia::typeface::TypefaceId, + /// Two typefaces sharing a font name disagree on this glyph. + conflict: bool, +} + +/// Text for each glyph, keyed by the font name Skia writes in `/BaseFont` +/// (without the subset tag). +#[derive(Debug, Default)] +pub struct GlyphTexts { + fonts: HashMap>, +} + +impl GlyphTexts { + /// Text that should replace the current mapping of `glyph` in `font`, if + /// any. `current` is the mapping Skia wrote (empty or `"\0"` when unknown). + pub fn replacement(&self, font: &str, glyph: skia::GlyphId, current: &str) -> Option<&str> { + let entry = self.fonts.get(font)?.get(&glyph)?; + if entry.conflict || entry.text.is_empty() || entry.text == current { + return None; + } + let unmapped = current.is_empty() || current == "\0"; + // A single glyph standing for several characters is a ligature: the + // font's `cmap` entry (e.g. U+FB02 for "fl") is not what was typed. + let ligature = entry.single_glyph && entry.text.chars().count() > 1; + (unmapped || ligature).then_some(entry.text.as_str()) + } + + pub fn is_empty(&self) -> bool { + self.fonts.is_empty() + } + + #[cfg(test)] + pub(super) fn with_glyph(mut self, font: &str, glyph: skia::GlyphId, text: &str) -> Self { + let value = GlyphText { + text: text.to_string(), + single_glyph: true, + typeface: 1, + conflict: false, + }; + self.insert(font, glyph, value); + self + } + + fn insert(&mut self, font: &str, glyph: skia::GlyphId, value: GlyphText) { + let glyphs = self.fonts.entry(font.to_string()).or_default(); + match glyphs.get_mut(&glyph) { + None => { + glyphs.insert(glyph, value); + } + // Same typeface, different text: keep the first one, as Skia does. + Some(existing) + if existing.typeface != value.typeface && existing.text != value.text => + { + existing.conflict = true; + } + Some(_) => {} + } + } + + /// Records the glyphs of a laid-out paragraph. `text` is the UTF-8 text the + /// paragraph was built from. + pub fn add_paragraph(&mut self, text: &str, paragraph: &mut skia::textlayout::Paragraph) { + let mut runs: Vec = Vec::new(); + paragraph.visit(|_, info| { + let Some(info) = info else { + return; + }; + let typeface = info.font().typeface(); + runs.push(Run { + typeface: typeface.unique_id(), + font_name: pdf_font_name(&typeface), + glyphs: info.glyphs().to_vec(), + starts: info.utf8_starts().iter().map(|s| *s as usize).collect(), + }); + }); + self.add_runs(text, &runs); + } + + fn add_runs(&mut self, text: &str, runs: &[Run]) { + // Cluster starts of the whole paragraph: a cluster ends where the next + // one starts, and that one may sit in another run or line. + let mut boundaries: Vec = runs + .iter() + .flat_map(|run| run.starts.iter().copied()) + .chain([text.len()]) + .collect(); + boundaries.sort_unstable(); + boundaries.dedup(); + + for run in runs { + let count = run.glyphs.len(); + for (i, &glyph) in run.glyphs.iter().enumerate() { + let start = run.starts[i]; + if i > 0 && run.starts[i - 1] == start { + continue; + } + let single_glyph = i + 1 >= count || run.starts[i + 1] != start; + if let Some(cluster) = cluster_text(text, &boundaries, start) { + self.insert( + &run.font_name, + glyph, + GlyphText { + text: cluster.to_string(), + single_glyph, + typeface: run.typeface, + conflict: false, + }, + ); + } + } + } + } +} + +struct Run { + typeface: skia::typeface::TypefaceId, + font_name: String, + glyphs: Vec, + starts: Vec, +} + +/// Text of the cluster starting at `start`. Whitespace ends it: spaces trimmed +/// at a line break are not drawn, so their start is missing from `boundaries`. +fn cluster_text<'a>(text: &'a str, boundaries: &[usize], start: usize) -> Option<&'a str> { + let end = boundaries + .iter() + .copied() + .find(|b| *b > start) + .unwrap_or(text.len()); + let cluster = text.get(start..end)?; + match cluster + .char_indices() + .skip(1) + .find(|(_, c)| c.is_whitespace()) + { + Some((at, _)) => Some(&cluster[..at]), + None => Some(cluster), + } +} + +/// The name Skia's PDF backend writes for a typeface: its PostScript name, or +/// its family name when it has none. +fn pdf_font_name(typeface: &skia::Typeface) -> String { + typeface + .post_script_name() + .filter(|name| !name.is_empty()) + .unwrap_or_else(|| typeface.family_name()) +} + +/// Collects the glyph texts of every visible text shape under `root`. +pub fn collect(tree: ShapesPoolRef, root: &Uuid) -> GlyphTexts { + let mut glyph_texts = GlyphTexts::default(); + let Some(root_shape) = tree.get(root) else { + return glyph_texts; + }; + for id in root_shape.all_children_iter(tree, false, true) { + let Some(shape) = tree.get(&id) else { + continue; + }; + let Type::Text(text_content) = &shape.shape_type else { + continue; + }; + if shape.hidden { + continue; + } + let text_content = text_content.new_bounds(shape.selrect()); + let width = text_content.get_width(shape.selrect().width()); + for group in text_content + .paragraph_builder_group_from_text(None) + .iter_mut() + { + for builder in group.iter_mut() { + // `build` takes the text out of the builder. + let text = builder.get_text().to_string(); + let mut paragraph = builder.build(); + paragraph.layout(width); + glyph_texts.add_paragraph(&text, &mut paragraph); + } + } + } + glyph_texts +} + +#[cfg(test)] +mod tests { + use super::*; + + fn run(glyphs: &[u16], starts: &[usize]) -> Run { + Run { + typeface: 1, + font_name: "Font".to_string(), + glyphs: glyphs.to_vec(), + starts: starts.to_vec(), + } + } + + #[test] + fn ligature_glyph_maps_to_its_whole_cluster() { + let mut texts = GlyphTexts::default(); + // "office": o, ffi (one glyph), c, e + texts.add_runs("office", &[run(&[10, 20, 30, 40], &[0, 1, 4, 5, 6])]); + + assert_eq!(texts.replacement("Font", 20, "\0"), Some("ffi")); + assert_eq!(texts.replacement("Font", 20, "\u{FB03}"), Some("ffi")); + assert_eq!(texts.replacement("Font", 10, "o"), None); + assert_eq!(texts.replacement("Font", 30, "\0"), Some("c")); + } + + #[test] + fn single_char_glyph_with_cmap_entry_keeps_it() { + let mut texts = GlyphTexts::default(); + texts.add_runs("a", &[run(&[5], &[0, 1])]); + + assert_eq!(texts.replacement("Font", 5, "\u{00E1}"), None); + assert_eq!(texts.replacement("Font", 5, ""), Some("a")); + } + + #[test] + fn multi_glyph_cluster_only_fills_the_first_glyph() { + let mut texts = GlyphTexts::default(); + // "é" drawn as base + mark glyphs sharing one cluster. + texts.add_runs("é", &[run(&[7, 8], &[0, 0, 2])]); + + assert_eq!(texts.replacement("Font", 7, "\0"), Some("é")); + assert_eq!(texts.replacement("Font", 8, "\0"), None); + } + + #[test] + fn clusters_end_at_starts_from_other_runs() { + let mut texts = GlyphTexts::default(); + // Right-to-left run: glyph order is the reverse of text order, and its + // last cluster ends where the next run starts. + texts.add_runs( + "abcd", + &[run(&[3, 2], &[1, 0, 0]), run(&[4, 5], &[2, 3, 4])], + ); + + assert_eq!(texts.replacement("Font", 3, "\0"), Some("b")); + assert_eq!(texts.replacement("Font", 2, "\0"), Some("a")); + } + + #[test] + fn cluster_stops_at_whitespace_dropped_at_line_breaks() { + let mut texts = GlyphTexts::default(); + // "lo world" wraps after "lo": the space at 2 is not drawn, so the + // "o" cluster would run until "w" without the whitespace stop. + texts.add_runs("lo world", &[run(&[8, 9], &[0, 1, 3])]); + + assert_eq!(texts.replacement("Font", 9, "\0"), Some("o")); + } + + #[test] + fn conflicting_typefaces_with_the_same_name_are_ignored() { + let mut texts = GlyphTexts::default(); + texts.add_runs("a", &[run(&[5], &[0, 1])]); + let mut other = run(&[5], &[0, 1]); + other.typeface = 2; + texts.add_runs("b", &[other]); + + assert_eq!(texts.replacement("Font", 5, "\0"), None); + } +} diff --git a/render-wasm/src/render/pdf/to_unicode.rs b/render-wasm/src/render/pdf/to_unicode.rs new file mode 100644 index 0000000000..a724490bc7 --- /dev/null +++ b/render-wasm/src/render/pdf/to_unicode.rs @@ -0,0 +1,793 @@ +//! Rewrites the `/ToUnicode` maps of a Skia PDF's Type0 fonts in an appended +//! incremental update, leaving the bytes Skia wrote untouched. + +use std::collections::{BTreeMap, HashMap}; + +use super::glyph_text::GlyphTexts; + +/// Glyph code to UTF-16 text. +type CMap = BTreeMap>; + +/// Returns `pdf` with fixed `/ToUnicode` maps, or unchanged when nothing needs +/// fixing or the file is not laid out as expected. +pub fn fix_to_unicode(pdf: Vec, glyph_texts: &GlyphTexts) -> Vec { + if glyph_texts.is_empty() { + return pdf; + } + match updated_cmaps(&pdf, glyph_texts) { + Some(Update { + cmaps, + xref_offset, + trailer, + }) if !cmaps.is_empty() => append_update(pdf, &cmaps, xref_offset, &trailer), + _ => pdf, + } +} + +struct Update { + /// ToUnicode object number to its new CMap. + cmaps: BTreeMap, + xref_offset: usize, + trailer: Vec, +} + +fn updated_cmaps(pdf: &[u8], glyph_texts: &GlyphTexts) -> Option { + let xref = parse_xref(pdf)?; + let mut cmaps = BTreeMap::new(); + + for &offset in xref.offsets.values() { + let Some(font) = read_object(pdf, offset) else { + continue; + }; + if dict_name(font.dict, b"/Type") != Some(b"Font".as_slice()) + || dict_name(font.dict, b"/Subtype") != Some(b"Type0".as_slice()) + { + continue; + } + let (Some(base_font), Some(to_unicode)) = ( + dict_name(font.dict, b"/BaseFont"), + dict_ref(font.dict, b"/ToUnicode"), + ) else { + continue; + }; + let font_name = strip_subset_tag(&decode_name(base_font)); + let Some(stream) = xref + .offsets + .get(&to_unicode) + .and_then(|offset| read_object(pdf, *offset)) + .and_then(|object| stream_data(&object)) + else { + continue; + }; + + let mut cmap = parse_cmap(&stream); + let mut changed = false; + for (code, value) in cmap.iter_mut() { + let current = String::from_utf16_lossy(value); + if let Some(text) = glyph_texts.replacement(&font_name, *code, ¤t) { + *value = text.encode_utf16().collect(); + changed = true; + } + } + if changed { + cmaps.insert(to_unicode, cmap); + } + } + + Some(Update { + cmaps, + xref_offset: xref.offset, + trailer: xref.trailer, + }) +} + +fn append_update( + mut pdf: Vec, + cmaps: &BTreeMap, + prev_xref: usize, + trailer: &[u8], +) -> Vec { + if pdf.last() != Some(&b'\n') { + pdf.push(b'\n'); + } + let mut offsets = Vec::with_capacity(cmaps.len()); + for (&number, cmap) in cmaps { + let data = write_cmap(cmap); + offsets.push((number, pdf.len())); + pdf.extend_from_slice( + format!("{number} 0 obj\n<> stream\n", data.len()).as_bytes(), + ); + pdf.extend_from_slice(&data); + pdf.extend_from_slice(b"\nendstream\nendobj\n"); + } + + let xref_offset = pdf.len(); + pdf.extend_from_slice(b"xref\n"); + for (number, offset) in offsets { + pdf.extend_from_slice(format!("{number} 1\n{offset:010} 00000 n \n").as_bytes()); + } + // Same entries as the previous trailer (`/Size`, `/Root`, `/Info`, `/ID`), + // plus `/Prev`. + let body = &trailer[..trailer.len() - 2]; + pdf.extend_from_slice(b"trailer\n"); + pdf.extend_from_slice(body); + pdf.extend_from_slice( + format!("\n/Prev {prev_xref}>>\nstartxref\n{xref_offset}\n%%EOF\n").as_bytes(), + ); + pdf +} + +// --------------------------------------------------------------------------- +// CMap + +fn parse_cmap(data: &[u8]) -> CMap { + let mut cmap = CMap::new(); + let mut tokens = CMapTokens { data, pos: 0 }; + while let Some(token) = tokens.next() { + match token { + Token::Keyword(b"beginbfchar") => { + while let Some(Token::Hex(code)) = tokens.next() { + let Some(Token::Hex(value)) = tokens.next() else { + break; + }; + if let Some(code) = to_code(&code) { + cmap.insert(code, to_utf16(&value)); + } + } + } + Token::Keyword(b"beginbfrange") => { + while let Some(Token::Hex(start)) = tokens.next() { + let (Some(Token::Hex(end)), Some(first)) = (tokens.next(), tokens.next()) + else { + break; + }; + let (Some(start), Some(end)) = (to_code(&start), to_code(&end)) else { + continue; + }; + match first { + Token::Hex(value) => { + let value = to_utf16(&value); + for (step, code) in (start..=end).enumerate() { + let mut value = value.clone(); + if let Some(last) = value.last_mut() { + *last = last.wrapping_add(step as u16); + } + cmap.insert(code, value); + } + } + Token::ArrayStart => { + let mut code = start; + while let Some(Token::Hex(value)) = tokens.next() { + cmap.insert(code, to_utf16(&value)); + code = code.wrapping_add(1); + } + } + _ => break, + } + } + } + _ => {} + } + } + cmap +} + +fn write_cmap(cmap: &CMap) -> Vec { + let mut out = String::from( + "/CIDInit /ProcSet findresource begin\n\ + 12 dict begin\n\ + begincmap\n\ + /CIDSystemInfo\n\ + << /Registry (Adobe)\n\ + /Ordering (UCS)\n\ + /Supplement 0\n\ + >> def\n\ + /CMapName /Adobe-Identity-UCS def\n\ + /CMapType 2 def\n\ + 1 begincodespacerange\n\ + <0000> \n\ + endcodespacerange\n", + ); + let entries: Vec<_> = cmap.iter().collect(); + // At most 100 entries per section (PDF 32000-1, 9.10.3). + for chunk in entries.chunks(100) { + out.push_str(&format!("{} beginbfchar\n", chunk.len())); + for (code, value) in chunk { + out.push_str(&format!("<{code:04X}> <")); + for unit in value.iter() { + out.push_str(&format!("{unit:04X}")); + } + out.push_str(">\n"); + } + out.push_str("endbfchar\n"); + } + out.push_str( + "endcmap\n\ + CMapName currentdict /CMap defineresource pop\n\ + end\n\ + end", + ); + out.into_bytes() +} + +enum Token<'a> { + Hex(Vec), + ArrayStart, + ArrayEnd, + Keyword(&'a [u8]), + Other, +} + +struct CMapTokens<'a> { + data: &'a [u8], + pos: usize, +} + +impl<'a> Iterator for CMapTokens<'a> { + type Item = Token<'a>; + + fn next(&mut self) -> Option> { + self.pos = skip_ws(self.data, self.pos); + let byte = *self.data.get(self.pos)?; + let start = self.pos; + self.pos += 1; + match byte { + b'<' if self.data.get(self.pos) == Some(&b'<') => { + self.pos += 1; + Some(Token::Other) + } + b'<' => { + let end = find(self.data, b">", self.pos)?; + let hex = parse_hex(&self.data[self.pos..end]); + self.pos = end + 1; + Some(Token::Hex(hex)) + } + b'[' => Some(Token::ArrayStart), + b']' => Some(Token::ArrayEnd), + _ => { + while self + .data + .get(self.pos) + .is_some_and(|b| !is_ws(*b) && !is_delimiter(*b)) + { + self.pos += 1; + } + Some(Token::Keyword(&self.data[start..self.pos])) + } + } + } +} + +fn parse_hex(hex: &[u8]) -> Vec { + let digits: Vec = hex + .iter() + .filter_map(|b| (*b as char).to_digit(16).map(|d| d as u8)) + .collect(); + digits + .chunks(2) + .map(|pair| (pair[0] << 4) | pair.get(1).copied().unwrap_or(0)) + .collect() +} + +fn to_code(bytes: &[u8]) -> Option { + match bytes { + [byte] => Some(*byte as u16), + [high, low] => Some(u16::from_be_bytes([*high, *low])), + _ => None, + } +} + +fn to_utf16(bytes: &[u8]) -> Vec { + bytes + .chunks(2) + .map(|pair| u16::from_be_bytes([pair[0], pair.get(1).copied().unwrap_or(0)])) + .collect() +} + +// --------------------------------------------------------------------------- +// PDF objects + +struct Xref { + /// Byte offset of the `xref` keyword. + offset: usize, + /// Object number to byte offset, for objects in use. + offsets: HashMap, + /// The trailer dictionary, `<<` to `>>`. + trailer: Vec, +} + +/// Reads the last xref table and its trailer. Skia writes a single classic +/// table, without xref streams. +fn parse_xref(pdf: &[u8]) -> Option { + let startxref = rfind(pdf, b"startxref")?; + let (offset, _) = parse_uint(pdf, skip_ws(pdf, startxref + b"startxref".len()))?; + parse_xref_at(pdf, offset as usize) +} + +fn parse_xref_at(pdf: &[u8], offset: usize) -> Option { + if !pdf.get(offset..)?.starts_with(b"xref") { + return None; + } + let mut offsets = HashMap::new(); + let mut pos = offset + b"xref".len(); + loop { + pos = skip_ws(pdf, pos); + if pdf.get(pos..)?.starts_with(b"trailer") { + break; + } + let (first, next) = parse_uint(pdf, pos)?; + let (count, next) = parse_uint(pdf, skip_ws(pdf, next))?; + pos = next; + for number in first..first + count { + let (entry_offset, next) = parse_uint(pdf, skip_ws(pdf, pos))?; + let (_generation, next) = parse_uint(pdf, skip_ws(pdf, next))?; + pos = skip_ws(pdf, next); + if *pdf.get(pos)? == b'n' { + offsets.insert(number as u32, entry_offset as usize); + } + pos += 1; + } + } + let start = skip_ws(pdf, pos + b"trailer".len()); + let end = skip_dict(pdf, start)?; + Some(Xref { + offset, + offsets, + trailer: pdf[start..end].to_vec(), + }) +} + +struct Object<'a> { + dict: &'a [u8], + stream: Option<&'a [u8]>, +} + +/// Reads the dictionary (and stream, if any) of the object at `offset`. +fn read_object(pdf: &[u8], offset: usize) -> Option> { + let (_, pos) = parse_uint(pdf, offset)?; + let (_, pos) = parse_uint(pdf, skip_ws(pdf, pos))?; + let pos = skip_ws(pdf, pos); + if !pdf.get(pos..)?.starts_with(b"obj") { + return None; + } + let start = skip_ws(pdf, pos + b"obj".len()); + let end = skip_dict(pdf, start)?; + let dict = &pdf[start..end]; + + let mut pos = skip_ws(pdf, end); + if !pdf.get(pos..)?.starts_with(b"stream") { + return Some(Object { dict, stream: None }); + } + pos += b"stream".len(); + if pdf.get(pos) == Some(&b'\r') { + pos += 1; + } + if pdf.get(pos) == Some(&b'\n') { + pos += 1; + } + let length = dict_uint(dict, b"/Length")? as usize; + Some(Object { + dict, + stream: Some(pdf.get(pos..pos + length)?), + }) +} + +fn stream_data(object: &Object) -> Option> { + let stream = object.stream?; + match dict_name(object.dict, b"/Filter") { + None => Some(stream.to_vec()), + Some(b"FlateDecode") => miniz_oxide::inflate::decompress_to_vec_zlib(stream).ok(), + Some(_) => None, + } +} + +/// Position right after the `>>` closing the dictionary at `pos`. +fn skip_dict(pdf: &[u8], pos: usize) -> Option { + if !pdf.get(pos..)?.starts_with(b"<<") { + return None; + } + let mut depth = 0; + let mut pos = pos; + while pos < pdf.len() { + match pdf[pos] { + b'<' if pdf.get(pos + 1) == Some(&b'<') => { + depth += 1; + pos += 2; + } + b'>' if pdf.get(pos + 1) == Some(&b'>') => { + depth -= 1; + pos += 2; + if depth == 0 { + return Some(pos); + } + } + b'<' => pos = find(pdf, b">", pos)? + 1, + b'(' => pos = skip_string(pdf, pos)?, + _ => pos += 1, + } + } + None +} + +/// Position right after the literal string starting at `pos`. +fn skip_string(pdf: &[u8], pos: usize) -> Option { + let mut depth = 0; + let mut pos = pos; + while pos < pdf.len() { + match pdf[pos] { + b'\\' => pos += 1, + b'(' => depth += 1, + b')' => { + depth -= 1; + if depth == 0 { + return Some(pos + 1); + } + } + _ => {} + } + pos += 1; + } + None +} + +/// Position of the value for `key`, matching whole keys only. +fn dict_value<'a>(dict: &'a [u8], key: &[u8]) -> Option<&'a [u8]> { + let mut from = 0; + while let Some(at) = find(dict, key, from) { + let end = at + key.len(); + if dict.get(end).is_some_and(|b| is_ws(*b) || is_delimiter(*b)) { + return Some(&dict[skip_ws(dict, end)..]); + } + from = end; + } + None +} + +fn dict_name<'a>(dict: &'a [u8], key: &[u8]) -> Option<&'a [u8]> { + let value = dict_value(dict, key)?.strip_prefix(b"/")?; + let end = value + .iter() + .position(|b| is_ws(*b) || is_delimiter(*b)) + .unwrap_or(value.len()); + Some(&value[..end]) +} + +fn dict_uint(dict: &[u8], key: &[u8]) -> Option { + parse_uint(dict_value(dict, key)?, 0).map(|(value, _)| value) +} + +/// Object number of an indirect reference (`N 0 R`). +fn dict_ref(dict: &[u8], key: &[u8]) -> Option { + let value = dict_value(dict, key)?; + let (number, pos) = parse_uint(value, 0)?; + let (_, pos) = parse_uint(value, skip_ws(value, pos))?; + (value.get(skip_ws(value, pos)) == Some(&b'R')).then_some(number as u32) +} + +/// Decodes `#xx` escapes in a PDF name. +fn decode_name(name: &[u8]) -> String { + let mut out = Vec::with_capacity(name.len()); + let mut i = 0; + while i < name.len() { + if name[i] == b'#' && i + 2 < name.len() { + if let [byte] = parse_hex(&name[i + 1..i + 3])[..] { + out.push(byte); + i += 3; + continue; + } + } + out.push(name[i]); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() +} + +/// Drops the `ABCDEF+` subset tag Skia puts before every font name. +fn strip_subset_tag(name: &str) -> String { + let bytes = name.as_bytes(); + if bytes.len() > 7 && bytes[6] == b'+' && bytes[..6].iter().all(u8::is_ascii_uppercase) { + name[7..].to_string() + } else { + name.to_string() + } +} + +fn parse_uint(data: &[u8], pos: usize) -> Option<(u64, usize)> { + let digits = data + .get(pos..)? + .iter() + .take_while(|b| b.is_ascii_digit()) + .count(); + if digits == 0 { + return None; + } + let value = std::str::from_utf8(&data[pos..pos + digits]) + .ok()? + .parse() + .ok()?; + Some((value, pos + digits)) +} + +fn skip_ws(data: &[u8], mut pos: usize) -> usize { + while data.get(pos).is_some_and(|b| is_ws(*b)) { + pos += 1; + } + pos +} + +fn is_ws(byte: u8) -> bool { + matches!(byte, b' ' | b'\n' | b'\r' | b'\t' | b'\x0c' | b'\0') +} + +fn is_delimiter(byte: u8) -> bool { + matches!( + byte, + b'/' | b'<' | b'>' | b'[' | b']' | b'(' | b')' | b'{' | b'}' | b'%' + ) +} + +fn find(hay: &[u8], needle: &[u8], from: usize) -> Option { + hay.get(from..)? + .windows(needle.len()) + .position(|window| window == needle) + .map(|at| at + from) +} + +fn rfind(hay: &[u8], needle: &[u8]) -> Option { + hay.windows(needle.len()) + .rposition(|window| window == needle) +} + +#[cfg(test)] +mod tests { + use super::*; + + use skia_safe as skia; + + use crate::render::RenderResources; + use crate::shapes::{ + Fill, FontFamily, FontStyle, GrowType, Paragraph, SolidColor, TextAlign, TextContent, + TextDirection, TextSpan, Type, + }; + use crate::state::ShapesPool; + use crate::utils::uuid_from_u32_quartet; + use crate::uuid::Uuid; + + fn cmap(entries: &[(u16, &str)]) -> CMap { + entries + .iter() + .map(|(code, text)| (*code, text.encode_utf16().collect())) + .collect() + } + + /// Objects visible in the final file, following `/Prev` back to the + /// first xref table. + fn latest_offsets(pdf: &[u8]) -> HashMap { + let mut xref = parse_xref(pdf).expect("xref"); + let mut offsets = xref.offsets.clone(); + while let Some(prev) = dict_uint(&xref.trailer, b"/Prev") { + xref = parse_xref_at(pdf, prev as usize).expect("previous xref"); + for (number, offset) in &xref.offsets { + offsets.entry(*number).or_insert(*offset); + } + } + offsets + } + + fn object<'a>(pdf: &'a [u8], offsets: &HashMap, number: u32) -> Object<'a> { + read_object(pdf, offsets[&number]).expect("object") + } + + /// The Type0 font's ToUnicode map, as seen by a reader. + fn to_unicode_of_only_font(pdf: &[u8]) -> CMap { + let offsets = latest_offsets(pdf); + let fonts: Vec<_> = offsets + .values() + .filter_map(|offset| read_object(pdf, *offset)) + .filter(|o| dict_name(o.dict, b"/Subtype") == Some(b"Type0".as_slice())) + .collect(); + assert_eq!(fonts.len(), 1, "expected a single Type0 font"); + let number = dict_ref(fonts[0].dict, b"/ToUnicode").expect("ToUnicode ref"); + parse_cmap(&stream_data(&object(pdf, &offsets, number)).expect("cmap data")) + } + + /// Builds a classic PDF from object bodies numbered from 1. + fn build_pdf(objects: &[Vec]) -> Vec { + let mut pdf = b"%PDF-1.4\n".to_vec(); + let mut offsets = Vec::new(); + for (i, body) in objects.iter().enumerate() { + offsets.push(pdf.len()); + pdf.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + pdf.extend_from_slice(body); + pdf.extend_from_slice(b"\nendobj\n"); + } + let xref = pdf.len(); + pdf.extend_from_slice( + format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes(), + ); + for offset in offsets { + pdf.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + pdf.extend_from_slice( + format!( + "trailer\n<>\nstartxref\n{xref}\n%%EOF", + objects.len() + 1 + ) + .as_bytes(), + ); + pdf + } + + fn stream_object(dict: &str, data: &[u8]) -> Vec { + let mut body = format!("<<{dict}/Length {}>> stream\n", data.len()).into_bytes(); + body.extend_from_slice(data); + body.extend_from_slice(b"\nendstream"); + body + } + + #[test] + fn parses_bfchar_and_both_bfrange_forms() { + let data = b"2 beginbfchar\n<0003> <0020>\n<0332> <00660066>\nendbfchar\n\ + 2 beginbfrange\n<0010> <0012> <0041>\n<0020> <0021> [<0061> ]\nendbfrange\n"; + + assert_eq!( + parse_cmap(data), + cmap(&[ + (0x03, " "), + (0x10, "A"), + (0x11, "B"), + (0x12, "C"), + (0x20, "a"), + (0x21, "\u{1F600}"), + (0x0332, "ff"), + ]) + ); + } + + #[test] + fn written_cmap_parses_back() { + let entries: Vec<(u16, &str)> = (0..250).map(|code| (code, "x")).collect(); + let mut original = cmap(&entries); + original.insert(0x0332, "ffi".encode_utf16().collect()); + + let written = write_cmap(&original); + + assert_eq!(parse_cmap(&written), original); + assert!(String::from_utf8(written) + .unwrap() + .contains("100 beginbfchar")); + } + + #[test] + fn appends_fixed_cmap_as_incremental_update() { + let skia_cmap = write_cmap(&cmap(&[(0x03, " "), (0x0332, "\0"), (0x0335, "\u{FB02}")])); + let compressed = miniz_oxide::deflate::compress_to_vec_zlib(&skia_cmap, 6); + let pdf = build_pdf(&[ + b"<>".to_vec(), + b"<>" + .to_vec(), + stream_object("/Filter /FlateDecode", &compressed), + ]); + let texts = GlyphTexts::default() + .with_glyph("My Font", 0x0332, "ffi") + .with_glyph("My Font", 0x0335, "fl") + .with_glyph("My Font", 0x03, " "); + + let fixed = fix_to_unicode(pdf.clone(), &texts); + + assert!( + fixed.starts_with(&pdf), + "original bytes must stay untouched" + ); + assert_eq!( + to_unicode_of_only_font(&fixed), + cmap(&[(0x03, " "), (0x0332, "ffi"), (0x0335, "fl")]) + ); + let trailer = parse_xref(&fixed).unwrap().trailer; + assert_eq!(dict_ref(&trailer, b"/Root"), Some(1)); + assert_eq!(dict_uint(&trailer, b"/Size"), Some(4)); + } + + #[test] + fn leaves_pdf_alone_when_nothing_changes() { + let pdf = build_pdf(&[ + b"<>".to_vec(), + b"<>".to_vec(), + stream_object("", &write_cmap(&cmap(&[(0x03, " ")]))), + ]); + let texts = GlyphTexts::default().with_glyph("Other", 0x0332, "ffi"); + + assert_eq!(fix_to_unicode(pdf.clone(), &texts), pdf); + } + + /// EB Garamond forms standard ligatures (fi, fl, ffi, ...). + const LIGATURE_FONT: &[u8] = + include_bytes!("../../../../frontend/playwright/data/render-wasm/assets/ebgaramond.ttf"); + + fn add_text(pool: &mut ShapesPool, id: Uuid, family: FontFamily, text: &str) { + let mut content = TextContent::new( + skia::Rect::from_xywh(0.0, 0.0, 600.0, 100.0), + GrowType::Fixed, + ); + let span = TextSpan::new( + text.to_string(), + family, + 40.0, + 1.2, + 0.0, + None, + None, + TextDirection::LTR, + 400, + Uuid::nil(), + vec![Fill::Solid(SolidColor(skia::Color::BLACK))], + ); + content.add_paragraph(Paragraph::new( + TextAlign::Left, + TextDirection::LTR, + None, + None, + 1.2, + 0.0, + vec![span], + )); + let shape = pool.add_shape(id); + shape.set_parent(Uuid::nil()); + shape.set_selrect(0.0, 0.0, 600.0, 100.0); + shape.set_shape_type(Type::Text(content)); + } + + /// The text a PDF reader copies from the page: every glyph code drawn, + /// read through the font's final ToUnicode map. + fn copied_text(pdf: &[u8]) -> String { + let offsets = latest_offsets(pdf); + let page = offsets + .values() + .filter_map(|offset| read_object(pdf, *offset)) + .find(|o| dict_name(o.dict, b"/Type") == Some(b"Page".as_slice())) + .expect("page"); + let contents = dict_ref(page.dict, b"/Contents").expect("contents"); + let content = stream_data(&object(pdf, &offsets, contents)).expect("content"); + let to_unicode = to_unicode_of_only_font(pdf); + CMapTokens { + data: &content, + pos: 0, + } + .filter_map(|token| match token { + Token::Hex(codes) => Some(codes), + _ => None, + }) + .flat_map(|codes| { + codes + .chunks(2) + .filter_map(to_code) + .map(|code| String::from_utf16_lossy(&to_unicode[&code])) + .collect::>() + }) + .collect() + } + + #[test] + fn exported_ligatures_copy_back_as_typed() { + let text = "office fluffy affinity"; + let family = FontFamily::new(uuid_from_u32_quartet(0, 0, 0, 99), 400, FontStyle::Normal); + let id = uuid_from_u32_quartet(0, 0, 0, 1); + let mut pool = ShapesPool::new(); + add_text(&mut pool, id, family, text); + + let mut resources = RenderResources::try_new_headless().expect("headless resources"); + resources + .fonts + .add(family, LIGATURE_FONT, false, false) + .expect("ligature font"); + let _guard = crate::globals::TestRenderResourcesGuard::install(&mut resources); + let pdf = super::super::render_to_pdf(&mut resources, &id, &pool, 1.0).expect("pdf"); + + let trailer = parse_xref(&pdf).expect("xref").trailer; + assert!( + dict_uint(&trailer, b"/Prev").is_some(), + "the ligatures should need a ToUnicode fix" + ); + assert_eq!(copied_text(&pdf), text); + } +} From c0450a77c37ad85e92f6f7019ee25298abfa627e Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 20 Oct 2026 09:49:05 +0200 Subject: [PATCH 3/9] :bug: Limit board guide hit testing to the board frame (#12460) WASM find_guide_at only checked distance on the guide axis, so board-bound guides stayed interactive outside their board. Respect frame_range in GuidePool hit testing so interaction matches the drawn solid segment and the SVG hit area. Closes #12322 --- render-wasm/src/state/ui.rs | 80 +++++++++++++++++++++++++++++++++++-- render-wasm/src/ui.rs | 18 +++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/render-wasm/src/state/ui.rs b/render-wasm/src/state/ui.rs index d29645cc02..6878875c5e 100644 --- a/render-wasm/src/state/ui.rs +++ b/render-wasm/src/state/ui.rs @@ -36,8 +36,11 @@ impl GuidePool { } let world_tolerance = tolerance / zoom; - let vertical = Self::find_closest_in_axis(&self.vertical, x, world_tolerance); - let horizontal = Self::find_closest_in_axis(&self.horizontal, y, world_tolerance); + // Vertical guides run along y; horizontal along x. Pass the along-line + // coordinate so board-bound guides are only hittable inside their + // frame_range (matching the solid drawn segment). + let vertical = Self::find_closest_in_axis(&self.vertical, x, y, world_tolerance); + let horizontal = Self::find_closest_in_axis(&self.horizontal, y, x, world_tolerance); match (vertical, horizontal) { (Some(v), Some(h)) => { @@ -53,7 +56,12 @@ impl GuidePool { } } - fn find_closest_in_axis(guides: &[Guide], coord: f32, world_tolerance: f32) -> Option<&Guide> { + fn find_closest_in_axis( + guides: &[Guide], + coord: f32, + along: f32, + world_tolerance: f32, + ) -> Option<&Guide> { if guides.is_empty() { return None; } @@ -66,6 +74,9 @@ impl GuidePool { for candidate_idx in [idx.wrapping_sub(1), idx] { if candidate_idx < guides.len() { let guide = &guides[candidate_idx]; + if !guide.contains_along(along) { + continue; + } let dist = (guide.position() - coord).abs(); if dist <= world_tolerance && dist <= closest_dist { closest_dist = dist; @@ -183,6 +194,24 @@ mod tests { ) } + fn vertical_board_guide(x: f32, y0: f32, y1: f32, index: usize) -> Guide { + Guide::new( + GuideKind::Vertical(x), + Color::BLACK, + Some(index), + Some((y0, y1)), + ) + } + + fn horizontal_board_guide(y: f32, x0: f32, x1: f32, index: usize) -> Guide { + Guide::new( + GuideKind::Horizontal(y), + Color::BLACK, + Some(index), + Some((x0, x1)), + ) + } + fn pool_with(guides: Vec) -> GuidePool { let mut pool = GuidePool::new(); pool.set(guides); @@ -264,4 +293,49 @@ mod tests { assert!(pool.find_at(104.0, 0.0, 2.0, 8.0).is_some()); assert!(pool.find_at(105.0, 0.0, 2.0, 8.0).is_none()); } + + #[test] + fn find_at_hits_vertical_board_guide_inside_frame_range() { + let pool = pool_with(vec![vertical_board_guide(100.0, 0.0, 200.0, 0)]); + let guide = pool.find_at(102.0, 50.0, 1.0, 8.0).unwrap(); + assert_eq!(guide.index, 0); + } + + #[test] + fn find_at_misses_vertical_board_guide_outside_frame_range() { + let pool = pool_with(vec![vertical_board_guide(100.0, 0.0, 200.0, 0)]); + assert!(pool.find_at(102.0, 250.0, 1.0, 8.0).is_none()); + } + + #[test] + fn find_at_hits_horizontal_board_guide_inside_frame_range() { + let pool = pool_with(vec![horizontal_board_guide(200.0, 0.0, 300.0, 1)]); + let guide = pool.find_at(50.0, 203.0, 1.0, 8.0).unwrap(); + assert_eq!(guide.index, 1); + } + + #[test] + fn find_at_misses_horizontal_board_guide_outside_frame_range() { + let pool = pool_with(vec![horizontal_board_guide(200.0, 0.0, 300.0, 1)]); + assert!(pool.find_at(350.0, 203.0, 1.0, 8.0).is_none()); + } + + #[test] + fn find_at_free_guide_still_hits_outside_any_board() { + let pool = pool_with(vec![vertical_guide(100.0, 0)]); + let guide = pool.find_at(102.0, 999.0, 1.0, 8.0).unwrap(); + assert_eq!(guide.index, 0); + } + + #[test] + fn find_at_misses_hidden_board_guide_empty_range() { + // Serializer uses [Infinity, Infinity] for rotated / non-root boards. + let pool = pool_with(vec![vertical_board_guide( + 100.0, + f32::INFINITY, + f32::INFINITY, + 0, + )]); + assert!(pool.find_at(100.0, 50.0, 1.0, 8.0).is_none()); + } } diff --git a/render-wasm/src/ui.rs b/render-wasm/src/ui.rs index d0e47c18e7..c7dfe0d8af 100644 --- a/render-wasm/src/ui.rs +++ b/render-wasm/src/ui.rs @@ -55,4 +55,22 @@ impl Guide { pub fn position(&self) -> f32 { self.kind.value() } + + /// Whether `along` (the coordinate along the guide line: y for vertical + /// guides, x for horizontal) falls inside this guide's interactive + /// segment. Free guides (`frame_range = None`) accept any value; board- + /// bound guides only accept points inside their clipped range. + pub fn contains_along(&self, along: f32) -> bool { + match self.frame_range { + None => true, + Some((start, end)) => { + let (lo, hi) = if start <= end { + (start, end) + } else { + (end, start) + }; + along >= lo && along <= hi + } + } + } } From 41d95abc5c1067690d1d5eb111b587055286d233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Tue, 20 Oct 2026 14:09:01 +0200 Subject: [PATCH 4/9] :bug: Fix math symbols rendered as boxes in the new render engine (#12471) Noto Sans Symbols lacks most arrows and math operators. Fall back to Noto Sans Math for them, and to Noto Sans for letterlike symbols. Closes #12469 AI-assisted-by: claude-opus-5-5 --- common/src/app/common/fonts.cljs | 11 +++++-- common/test/common_tests/fonts_test.cljs | 37 ++++++++++++++++++++++++ common/test/common_tests/runner.cljc | 2 ++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 common/test/common_tests/fonts_test.cljs diff --git a/common/src/app/common/fonts.cljs b/common/src/app/common/fonts.cljs index 237a6b362b..ee96b6c9df 100644 --- a/common/src/app/common/fonts.cljs +++ b/common/src/app/common/fonts.cljs @@ -213,7 +213,12 @@ ;; SMP: Mahjong/Domino/Playing Cards (U+1F000-1F0FF), Supplemental Arrows-C (U+1F800-1F8FF), ;; Legacy Computing Symbols (U+1FB00-1FBFF) :symbols-2 #"[\u2190-\u21FF\u2200-\u22FF\u2300-\u23FF\u25A0-\u25FF\u2600-\u26FF\u2700-\u27BF\u2B00-\u2BFF]|\uD83C[\uDC00-\uDCFF]|\uD83E[\uDC00-\uDCFF\uDF00-\uDFFF]" - :music #"[\u2669-\u267B]|\uD834[\uDD00-\uDD1F]"}) + :music #"[\u2669-\u267B]|\uD834[\uDD00-\uDD1F]" + ;; Arrows, Mathematical Operators, Misc Mathematical Symbols A/B, Supplemental + ;; Arrows A/B, Supplemental Math Operators: the Symbols fonts lack most of them + :math #"[\u2190-\u21FF\u2200-\u22FF\u27C0-\u27FF\u2900-\u2AFF]" + ;; Letterlike Symbols (ℝ, ℕ, ℃, ™, ...) + :letterlike #"[\u2100-\u214F]"}) (defn contains-emoji? [text] (let [result (re-find emoji-pattern text)] @@ -292,7 +297,9 @@ :meroitic {:font-id "gfont-noto-sans-meroitic" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true} :symbols {:font-id "gfont-noto-sans-symbols" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true} :symbols-2 {:font-id "gfont-noto-sans-symbols-2" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true} - :music {:font-id "gfont-noto-music" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true}}) + :music {:font-id "gfont-noto-music" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true} + :math {:font-id "gfont-noto-sans-math" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true} + :letterlike {:font-id "gfont-noto-sans" :font-variant-id "regular" :style 0 :weight 400 :is-fallback true}}) (defn add-noto-fonts [fonts languages] (reduce (fn [acc lang] diff --git a/common/test/common_tests/fonts_test.cljs b/common/test/common_tests/fonts_test.cljs new file mode 100644 index 0000000000..36786ccb7c --- /dev/null +++ b/common/test/common_tests/fonts_test.cljs @@ -0,0 +1,37 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns common-tests.fonts-test + (:require + [app.common.fonts :as cfnt] + [clojure.test :as t])) + +(defn- fallback-font-ids + [text] + (->> (cfnt/collect-used-languages #{} text) + (cfnt/add-noto-fonts #{}) + (into #{} (map :font-id)))) + +(t/deftest math-symbols-use-noto-sans-math + (t/testing "arrows" + (t/is (contains? (fallback-font-ids "↳") "gfont-noto-sans-math")) + (t/is (contains? (fallback-font-ids "⇒") "gfont-noto-sans-math"))) + (t/testing "mathematical operators" + (t/is (contains? (fallback-font-ids "∀x ∈ A") "gfont-noto-sans-math")) + (t/is (contains? (fallback-font-ids "a ≠ b") "gfont-noto-sans-math"))) + (t/testing "supplemental arrows and operators" + (t/is (contains? (fallback-font-ids "⟹") "gfont-noto-sans-math")) + (t/is (contains? (fallback-font-ids "⤴") "gfont-noto-sans-math")) + (t/is (contains? (fallback-font-ids "⨁") "gfont-noto-sans-math")))) + +(t/deftest letterlike-symbols-use-noto-sans + (t/is (contains? (fallback-font-ids "x ∈ ℝ") "gfont-noto-sans")) + (t/is (contains? (fallback-font-ids "20 ℃") "gfont-noto-sans")) + (t/is (contains? (fallback-font-ids "Penpot™") "gfont-noto-sans"))) + +(t/deftest plain-text-does-not-use-noto-sans-math + (t/is (not (contains? (fallback-font-ids "Hello world") "gfont-noto-sans-math"))) + (t/is (not (contains? (fallback-font-ids "★ ☀") "gfont-noto-sans-math")))) diff --git a/common/test/common_tests/runner.cljc b/common/test/common_tests/runner.cljc index 7a4c4243f4..e78fbf3da9 100644 --- a/common/test/common_tests/runner.cljc +++ b/common/test/common_tests/runner.cljc @@ -10,6 +10,7 @@ #?(:cljs [app.common.logging :as l]) #?(:cljs [clojure.string :as str]) #?(:cljs [clojure.tools.cli :refer [parse-opts]]) + #?(:cljs [common-tests.fonts-test]) #?(:cljs [goog.object :as gobj]) [clojure.test :as t] [common-tests.attrs-test] @@ -123,6 +124,7 @@ 'common-tests.data-test 'common-tests.exceptions-test 'common-tests.files-builder-test + #?(:cljs 'common-tests.fonts-test) 'common-tests.files-changes-test 'common-tests.files-migrations-0025-test 'common-tests.files-migrations-0026-test From ef68304cb88f72704fbc4ddc62b1f4eec58d39cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bel=C3=A9n=20Albeza?= Date: Tue, 20 Oct 2026 15:04:54 +0200 Subject: [PATCH 5/9] :bug: Draw masked group inner shadows in PDF and multi-element export (#12468) * :bug: Fix render_to_pdf call in the ToUnicode test The test passed a scale argument that render_to_pdf does not take, so the render-wasm test build failed to compile. * :bug: Draw masked group inner shadows in PDF and multi-element export The CPU renderer behind PDF and multi-element raster export drew a masked group's drop shadow but never its inner shadow, so the masked content came out flat. Wrap the masked content in a layer that draws the inner shadows over it, as the GPU single export does. * :bug: Export every visible inner shadow of a masked group The workspace skips shadows too small to see at the current zoom, to render faster. Export has no such need, so draw every visible inner shadow instead of filtering by scale. --- render-wasm/src/render/pdf/to_unicode.rs | 2 +- render-wasm/src/render/svg/tests.rs | 70 ++++++++++++++++++++++++ render-wasm/src/render/vector.rs | 15 +++++ render-wasm/src/shapes.rs | 41 ++++++++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/render-wasm/src/render/pdf/to_unicode.rs b/render-wasm/src/render/pdf/to_unicode.rs index a724490bc7..ffb36f233e 100644 --- a/render-wasm/src/render/pdf/to_unicode.rs +++ b/render-wasm/src/render/pdf/to_unicode.rs @@ -781,7 +781,7 @@ mod tests { .add(family, LIGATURE_FONT, false, false) .expect("ligature font"); let _guard = crate::globals::TestRenderResourcesGuard::install(&mut resources); - let pdf = super::super::render_to_pdf(&mut resources, &id, &pool, 1.0).expect("pdf"); + let pdf = super::super::render_to_pdf(&mut resources, &id, &pool).expect("pdf"); let trailer = parse_xref(&pdf).expect("xref").trailer; assert!( diff --git a/render-wasm/src/render/svg/tests.rs b/render-wasm/src/render/svg/tests.rs index 0a047e8d07..02eda33b79 100644 --- a/render-wasm/src/render/svg/tests.rs +++ b/render-wasm/src/render/svg/tests.rs @@ -3795,3 +3795,73 @@ fn raster_export_of_child_paints_inherited_group_fill() { assert_eq!(inherited_png, raster_png(&own, uid(2))); assert_ne!(inherited_png, raster_png(&bare, uid(2))); } + +/// Reads one pixel of an encoded raster export as unpremultiplied RGBA. +fn raster_pixel(png: &[u8], (x, y): (i32, i32)) -> skia::Color { + let image = skia::Image::from_encoded(skia::Data::new_copy(png)).expect("decode png"); + let info = skia::ImageInfo::new( + (1, 1), + skia::ColorType::RGBA8888, + skia::AlphaType::Unpremul, + None, + ); + let mut px = [0u8; 4]; + assert!(image.read_pixels( + &info, + &mut px, + 4, + (x, y), + skia::image::CachingHint::Disallow + )); + skia::Color::from_argb(px[3], px[0], px[1], px[2]) +} + +/// #12334: headless raster/PDF must draw a masked group's inner shadow +/// inside the mask outline, as the GPU single export does. +#[test] +fn raster_export_of_masked_group_draws_inner_shadow() { + let mut pool = ShapesPool::new(); + let group_id = uid(1); + let mask_id = uid(2); + let content_id = uid(3); + let blue = skia::Color::from_rgb(0, 63, 255); + + add_masked_group( + &mut pool, + group_id, + Uuid::nil(), + (0.0, 0.0, 100.0, 100.0), + &[mask_id, content_id], + ); + add_solid_circle( + &mut pool, + mask_id, + group_id, + (0.0, 0.0, 100.0, 100.0), + skia::Color::BLACK, + ); + add_solid_rect( + &mut pool, + content_id, + group_id, + (0.0, 0.0, 100.0, 100.0), + blue, + ); + pool.get_mut(&group_id).unwrap().add_shadow(Shadow::new( + skia::Color::BLACK, + 0.0, + 0.0, + (20.0, 0.0), + ShadowStyle::Inner, + false, + )); + + let (png, width, height) = raster_png_size(&pool, group_id); + assert_eq!((width, height), (100, 100)); + + // Left edge: the shifted silhouette leaves a band the shadow fills. + assert_eq!(raster_pixel(&png, (8, 50)), skia::Color::BLACK); + assert_eq!(raster_pixel(&png, (50, 50)), blue); + // Outside the circle the mask still clears everything. + assert_eq!(raster_pixel(&png, (3, 3)).a(), 0); +} diff --git a/render-wasm/src/render/vector.rs b/render-wasm/src/render/vector.rs index e9d873f353..bfbeedf0d0 100644 --- a/render-wasm/src/render/vector.rs +++ b/render-wasm/src/render/vector.rs @@ -821,7 +821,22 @@ fn render_group( let child_opts = opts.for_children_of(element); if masked { + // Inner shadows filter the masked pixels, so they follow the mask's + // outline. The canvas matrix already carries `scale`. + let inner_shadows = element.masked_group_inner_shadow_filter(); + if let Some(filter) = &inner_shadows { + let mut paint = Paint::default(); + paint.set_image_filter(filter.clone()); + canvas.save_layer( + &skia::canvas::SaveLayerRec::default() + .bounds(&element.extrect(tree, scale)) + .paint(&paint), + ); + } render_masked_children(shared, canvas, element, &children, tree, scale, &child_opts)?; + if inner_shadows.is_some() { + canvas.restore(); // inner shadow layer + } } else { for child_id in &children { render_tree_inner(shared, canvas, child_id, tree, scale, &child_opts)?; diff --git a/render-wasm/src/shapes.rs b/render-wasm/src/shapes.rs index aaf4e55a49..3baa6dd66c 100644 --- a/render-wasm/src/shapes.rs +++ b/render-wasm/src/shapes.rs @@ -1998,6 +1998,25 @@ impl Shape { }) } + /// Source graphic with every visible inner shadow of the masked group over + /// it, for the CPU export renderer; `None` without inner shadows. + pub fn masked_group_inner_shadow_filter(&self) -> Option { + if !self.is_masked_group() { + return None; + } + + let mut layers: Vec> = vec![None]; + for shadow in self.inner_shadows_visible() { + layers.push(shadow.get_inner_shadow_filter()); + } + + if layers.len() > 1 { + skia::image_filters::merge(layers, None) + } else { + None + } + } + /// Image filter for the `save_layer` that wraps a masked group, i.e. for /// the result *after* the mask has been composited into it. /// @@ -2390,6 +2409,28 @@ mod tests { math::Rect::from_ltrb(0.0, 0.0, 100.0, 100.0) } + #[test] + fn masked_group_inner_shadow_filter_needs_an_inner_shadow() { + let drop_only = masked_group_with(vec![drop_shadow(4.0, 0.0, (10.0, 10.0))], None); + assert!(drop_only.masked_group_inner_shadow_filter().is_none()); + + let inner = masked_group_with(vec![inner_shadow(4.0, (10.0, 10.0))], None); + assert!(inner.masked_group_inner_shadow_filter().is_some()); + + let mut unmasked = inner.clone(); + unmasked.set_masked(false); + assert!(unmasked.masked_group_inner_shadow_filter().is_none()); + } + + #[test] + fn masked_group_inner_shadow_filter_keeps_imperceptible_shadows() { + // Too small for the workspace filter at 1x, but export draws it. + let shape = masked_group_with(vec![inner_shadow(1.0, (0.0, 0.0))], None); + + assert!(shape.masked_group_layer_filter(1.0, false, false).is_none()); + assert!(shape.masked_group_inner_shadow_filter().is_some()); + } + #[test] fn masked_group_layer_filter_is_none_without_effects() { let shape = masked_group_with(vec![], None); From a1606c42ff52e95ef9f2c1c01fad96402b80186b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elena=20Torr=C3=B3?= Date: Tue, 20 Oct 2026 15:37:07 +0200 Subject: [PATCH 6/9] :bug: Export bitmaps over the GPU surface limit through the exporter (#12455) --- .../src/app/main/data/exports/assets.cljs | 31 ++++++++++++++----- frontend/src/app/render_wasm/api.cljs | 15 +++++++++ frontend/translations/en.po | 4 +++ frontend/translations/es.po | 4 +++ render-wasm/src/main.rs | 25 +++++++++++++++ render-wasm/src/render/raster.rs | 21 +++++++++++++ 6 files changed, 92 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/main/data/exports/assets.cljs b/frontend/src/app/main/data/exports/assets.cljs index fd6d98e010..5ca4f3e983 100644 --- a/frontend/src/app/main/data/exports/assets.cljs +++ b/frontend/src/app/main/data/exports/assets.cljs @@ -18,6 +18,7 @@ [app.main.features :as features] [app.main.repo :as rp] [app.main.store :as st] + [app.render-wasm.api :as wasm.api] [app.util.dom :as dom] [app.util.i18n :refer [tr]] [app.util.websocket :as ws] @@ -300,11 +301,18 @@ [state] (features/active-feature? state "render-wasm/v1")) -(defn- use-wasm-export? - "Whether to take the client-side WASM export path for `export`." - [state export] - (and (wasm-export-enabled? state) - (contains? wasm-export-types (:type export)))) +(defn- export-target + [state {:keys [type object-id scale]}] + (cond + (not (and (wasm-export-enabled? state) + (contains? wasm-export-types type))) + :exporter + + (contains? #{:pdf :svg} type) + :client + + :else + (or (wasm.api/export-target object-id (or scale 1) type) :exporter))) (defn- request-simple-export-wasm [export] @@ -318,18 +326,25 @@ (defn request-simple-export [{:keys [export]}] - (let [export (normalize-export export)] + (let [export (normalize-export export) + target (volatile! nil)] (ptk/reify ::request-simple-export ptk/UpdateEvent (update [_ state] + (vreset! target (export-target state export)) (cond-> state - (not (use-wasm-export? state export)) + (= :exporter @target) (update :export assoc :in-progress true :id uuid/zero))) ptk/WatchEvent (watch [_ state _] - (if (use-wasm-export? state export) + (case @target + :client (rx/of (request-simple-export-wasm export)) + + :too-large + (rx/of (ntf/error (tr "errors.export-too-large"))) + (let [profile-id (:profile-id state) params (normalize-export-shapes-params {:exports [export] :profile-id profile-id diff --git a/frontend/src/app/render_wasm/api.cljs b/frontend/src/app/render_wasm/api.cljs index 2227b4155e..7bed759057 100644 --- a/frontend/src/app/render_wasm/api.cljs +++ b/frontend/src/app/render_wasm/api.cljs @@ -3009,6 +3009,21 @@ (mem/free) {:x x :y y :width w :height h}))))) +(defn export-target + [shape-id scale format] + (when (initialized?) + (let [buffer (uuid/get-u32 shape-id)] + (case (h/call wasm/internal-module "_export_target" + (aget buffer 0) + (aget buffer 1) + (aget buffer 2) + (aget buffer 3) + scale + (sr/translate-raster-format format)) + 0 :client + 2 :too-large + :exporter)))) + (defn render-shape-pdf [shape-id] (when (initialized?) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index fe435da917..014f5b622b 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1586,6 +1586,10 @@ msgstr "The email «%s» has been reported as spam or permanently bounce." msgid "errors.export-failed" msgstr "The export failed. Try again in a moment." +#: src/app/main/data/exports/assets.cljs:346 +msgid "errors.export-too-large" +msgstr "Too large to export, try a smaller scale" + #: src/app/main/errors.cljs:974 msgid "errors.feature-mismatch" msgstr "" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index df0f23c90c..6eaf06da2a 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -1596,6 +1596,10 @@ msgstr "El email «%s» tiene reportes de spam o de rebote permanente." msgid "errors.export-failed" msgstr "La exportación ha fallado. Inténtalo de nuevo en un momento." +#: src/app/main/data/exports/assets.cljs:346 +msgid "errors.export-too-large" +msgstr "El archivo es demasiado grande, prueba con otra escala" + #: src/app/main/errors.cljs:974 msgid "errors.feature-mismatch" msgstr "" diff --git a/render-wasm/src/main.rs b/render-wasm/src/main.rs index 11a8256842..34f3aa8dad 100644 --- a/render-wasm/src/main.rs +++ b/render-wasm/src/main.rs @@ -806,6 +806,31 @@ pub extern "C" fn render_shape_pixels( }) } +#[no_mangle] +pub extern "C" fn export_target(a: u32, b: u32, c: u32, d: u32, scale: f32, format: u32) -> u32 { + let id = uuid_from_u32_quartet(a, b, c, d); + let Ok(format) = RasterFormat::from_u32(format) else { + return 1; + }; + + with_state!(state, { + let Some(shape) = state.shapes.get(&id) else { + return 1; + }; + let extrect = get_render_state().get_cached_extrect(shape, &state.shapes, scale); + let width = (scale * extrect.width()).ceil() as i32; + let height = (scale * extrect.height()).ceil() as i32; + let max_size = get_gpu_state().max_surface_size(); + if width <= max_size && height <= max_size { + 0 + } else if format.fits_raster(width, height) { + 1 + } else { + 2 + } + }) +} + #[no_mangle] pub extern "C" fn render_stats() { get_render_state().print_stats(); diff --git a/render-wasm/src/render/raster.rs b/render-wasm/src/render/raster.rs index 52dd798006..5bd001ac32 100644 --- a/render-wasm/src/render/raster.rs +++ b/render-wasm/src/render/raster.rs @@ -8,6 +8,16 @@ use macros::ToJs; use super::vector; use super::RenderResources; +/// Half the default 2 GiB emscripten heap (a 16384² RGBA surface), leaving +/// room for the encoded copies and the scene. Estimated, not measured. +const MAX_RASTER_BYTES: i64 = 1 << 30; +/// PNG stores each side in 31 bits; the memory budget limits it first. +const MAX_PNG_SIDE: i32 = i32::MAX; +/// libjpeg-turbo's `JPEG_MAX_DIMENSION`, below the 16-bit JPEG header limit. +const MAX_JPEG_SIDE: i32 = 65500; +/// libwebp's `WEBP_MAX_DIMENSION`: WebP stores each side in 14 bits. +const MAX_WEBP_SIDE: i32 = 16383; + /// Encoded output format for [`render_to_raster`] and `render_shape_pixels`. /// `ToJs` publishes the discriminants to `api/shared.js`, so the CLJS side /// reads them from here instead of hardcoding the codes. @@ -65,6 +75,17 @@ impl RasterFormat { pub fn clear_color(self) -> skia::Color { self.opaque_background().unwrap_or(skia::Color::TRANSPARENT) } + + pub fn fits_raster(self, width: i32, height: i32) -> bool { + let max_side = match self { + Self::Png => MAX_PNG_SIDE, + Self::Jpeg => MAX_JPEG_SIDE, + Self::Webp => MAX_WEBP_SIDE, + }; + width <= max_side + && height <= max_side + && width as i64 * height as i64 * 4 <= MAX_RASTER_BYTES + } } /// Renders a shape tree to encoded image bytes on a CPU raster surface (no From 114928ea5f5f9e30656bb13e848670f33ab801f6 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 20 Oct 2026 15:33:37 +0000 Subject: [PATCH 7/9] :bug: Fix the key reported when a redis DEL times out `app.redis/del` logged `:key key`, but `key` is not bound in that `let`: it resolved to `clojure.core/key`, so the line carried `#object[clojure.core$key ...]` instead of the keys that failed to be deleted. Log `:keys keys`, which is always the normalized vector, so a single key and a collection report the same shape. The tests pin the line: the keys, the hint, the error level, the logger and the timeout as cause, plus the `nil` the caller gets. The prod-infra memory now notes that client-side redis timeouts mean the JVM is paused, not that the server is slow. AI-assisted-by: deepseek-v4.1-flash --- .serena/memories/prod-infra/core.md | 2 +- backend/src/app/redis.clj | 2 +- backend/test/backend_tests/redis_test.clj | 76 +++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 backend/test/backend_tests/redis_test.clj diff --git a/.serena/memories/prod-infra/core.md b/.serena/memories/prod-infra/core.md index 5dfaeebed7..0ab9ecde8e 100644 --- a/.serena/memories/prod-infra/core.md +++ b/.serena/memories/prod-infra/core.md @@ -5,7 +5,7 @@ Backend (`app.config`, `PENPOT_*` env vars) is parameterized; deployments choose ## Services - **PostgreSQL**: durable store. Profiles, teams, files, sessions, audit, `storage_object` metadata, the `task` queue, `scheduled_task` cron registry, migrations. File-data also lives here when the file-data backend is `legacy-db`/`db`. One shared DB across all backends. -- **Redis (Valkey-compatible)**: per-backend message bus and cache. Concrete uses: msgbus Pub/Sub for collaborative-editing broadcasts and team/profile-org notifications fired by RPC handlers (`app.rpc.notifications`, `files_update`, `teams`, `websocket`); file-summary cache gated by `enable-redis-cache`; rate-limit counters; and the dispatcher→runner work hand-off list `penpot.worker.queue::`. `PENPOT_REDIS_URI`. +- **Redis (Valkey-compatible)**: per-backend message bus and cache. Concrete uses: msgbus Pub/Sub for collaborative-editing broadcasts and team/profile-org notifications fired by RPC handlers (`app.rpc.notifications`, `files_update`, `teams`, `websocket`); file-summary cache gated by `enable-redis-cache`; rate-limit counters; and the dispatcher→runner work hand-off list `penpot.worker.queue::`. `PENPOT_REDIS_URI`. A burst of client-side `RedisCommandTimeoutException` (2s on `DEL`/`SET`, 10s on `BLPOP`) with the exporter healthy is the JVM paused or starved, not Redis: the timeout fires because nothing reads the reply. - **Object storage**: backends `:s3` and `:fs`. S3 in prod; devenv uses RustFS. Holds uploaded media, file-data when the file-data backend is `storage`, exports. Backend-side details (resolve, dedup, bucket set, object lifecycle, and file-data backends): `mem:backend/storage`. - **SMTP mailer**: invitations, password resets, email verification (sent via the `:sendmail` worker task). - **LDAP** (optional auth provider): helpers in `app.auth.*`, gated by `enable-login-with-ldap`. diff --git a/backend/src/app/redis.clj b/backend/src/app/redis.clj index be9e331dda..fba9268193 100644 --- a/backend/src/app/redis.clj +++ b/backend/src/app/redis.clj @@ -375,7 +375,7 @@ (try (-del conn keys) (catch RedisCommandTimeoutException cause - (l/err :hint "timeout on del redis key" :key key :cause cause) + (l/err :hint "timeout on del redis key" :keys keys :cause cause) nil)))) (defn ping diff --git a/backend/test/backend_tests/redis_test.clj b/backend/test/backend_tests/redis_test.clj new file mode 100644 index 0000000000..df3c8e15a8 --- /dev/null +++ b/backend/test/backend_tests/redis_test.clj @@ -0,0 +1,76 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns backend-tests.redis-test + "The redis wrapper swallows a command timeout and returns nil, so the log + line is the only trace of the command that did not reach the server. These + tests pin what that line has to carry." + (:require + [app.common.logging :as l] + [app.redis :as rds] + [clojure.test :as t]) + (:import + io.lettuce.core.RedisCommandTimeoutException)) + +(def ^:private timeout-hint "timeout on del redis key") + +(defn- timeout-connection + "A connection whose DEL always exceeds the command timeout, the way Lettuce + answers when the response does not arrive in time." + [] + (reify rds/IDefaultConnection + (-del [_ _keys] + (throw (RedisCommandTimeoutException. "DEL. Command timed out after 2 second(s)"))))) + +(defn- capture-records + "Run `f` with every log record collected, newest last. The logging API emits + asynchronously, so a record may land after `f` returns; callers wait for the + one they need." + [f] + (let [records (atom [])] + (add-watch l/log-record ::capture + (fn [_ _ _ record] + (swap! records conj record))) + (try + (f records) + (finally + (remove-watch l/log-record ::capture))))) + +(defn- timeout-record + "Wait until the record that reports a timed out DEL is captured." + [records timeout] + (let [deadline (+ (System/currentTimeMillis) timeout)] + (loop [] + (or (some (fn [record] + (when (= timeout-hint (get (::l/props record) :hint)) + record)) + @records) + (when (<= (System/currentTimeMillis) deadline) + (Thread/sleep 10) + (recur)))))) + +(t/deftest del-logs-the-keys-that-timed-out + ;; The server never saw the command, so the operator has no other way to know + ;; which key stayed behind. + (capture-records + (fn [records] + (t/is (nil? (rds/del (timeout-connection) ["file-summary:1" "file-summary:2"]))) + (let [record (timeout-record records 2000)] + (t/is (some? record) "a timed out DEL must leave a log line") + (t/is (= ["file-summary:1" "file-summary:2"] + (get (::l/props record) :keys))) + (t/is (instance? RedisCommandTimeoutException (::l/cause record))) + (t/is (= :error (::l/level record))) + (t/is (= "app.redis" (::l/logger record))))))) + +(t/deftest del-logs-a-single-key-as-a-collection + ;; `del` accepts one key or a collection and always hands a collection to the + ;; driver; the log keeps the same shape so it does not depend on the call. + (capture-records + (fn [records] + (rds/del (timeout-connection) "file-summary:1") + (t/is (= ["file-summary:1"] + (get (::l/props (timeout-record records 2000)) :keys)))))) From 3bab9fa0aaad4edb485db6942ec4bc61a16015a9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 20 Oct 2026 20:28:06 +0000 Subject: [PATCH 8/9] :books: Note the host variable needs three updates The prod-infra memory now says that the `$host` multi-variable needs its `query`, its `options` list and its `allValue` regex updated together. A host present only in `query` is dropped when `All` is selected, so the panels that filter with `instance=~"$host..."` stop showing it. Panel 104 is the visible symptom. AI-assisted-by: minimax-m3.1-flash-free --- .serena/memories/prod-infra/grafana.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.serena/memories/prod-infra/grafana.md b/.serena/memories/prod-infra/grafana.md index 533b09516f..14324b1f7f 100644 --- a/.serena/memories/prod-infra/grafana.md +++ b/.serena/memories/prod-infra/grafana.md @@ -20,6 +20,7 @@ Keep this memory free of internal names: no host names, no deployed versions, no ## Main dashboard - `Production status`, uid `penpot-production-status`. Sections: general state and deploy, `RPC y CLIMIT`, PostgreSQL and pool, Redis/msgbus, tasks, JVM/process. Variables `$env` and `$host` (custom), no auto refresh. Every section opens collapsed except `RPC y CLIMIT`, which is the one read on a normal day. + - `$host` is a custom multi-variable: its `options` list and its `allValue` regex must both enumerate every host. A host added to `query` but missing from `options`/`allValue` is silently dropped when `All` is selected, so panels that filter with `instance=~"$host..."` stop showing it (panel 104 is the visible symptom). Update all three places together when adding a host. - Per-host investigation dashboards are separate, and the alert rules deeplink into them. - The PostgreSQL section ends with a block of vacuum panels: two stats (autovacuum workers active on the primary, and the launcher alive), a Loki timeseries of finished vacuums per interval and table, and then the `Vacuum TXID age` table, which is the last panel of the section. The worker stat is an approximation — it counts the `pg_stat_activity` bucket described above, because the exporter publishes no `backend_type` — and the Loki panel is retrospective by construction. A per-table, in-flight view (table, phase, progress) needs a `pg_stat_progress_vacuum` collector added to the exporter itself. From 021492a57772ff3178b8c21f215a1806c54b5ee3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 20 Oct 2026 20:36:24 +0000 Subject: [PATCH 9/9] :bug: Make migration 0156 compatible with PostgreSQL 15 `SET STORAGE DEFAULT` needs PostgreSQL 16, but the official docker images pin `postgres:15`, so migration 0156 failed on those deployments. The migration now uses `SET STORAGE EXTENDED`, the storage mode of the type of every column it touches (text, text[], jsonb and bytea), which leaves the catalog in the same state on any supported version. The documented PostgreSQL floor goes back to >=15, the reason for raising it was that syntax. Verified on the test database: the 62 columns reset and no table column keeps the legacy EXTERNAL storage. The backend memory now records that migrations must stay within PostgreSQL 15 syntax. AI-assisted-by: minimax-m3.1-flash-free --- .serena/memories/backend/core.md | 2 + .../sql/0156-reset-pg-storage-defaults.sql | 136 +++++++++--------- docs/technical-guide/configuration.md | 2 +- 3 files changed, 72 insertions(+), 68 deletions(-) diff --git a/.serena/memories/backend/core.md b/.serena/memories/backend/core.md index 42e7c7b756..7ed309bd18 100644 --- a/.serena/memories/backend/core.md +++ b/.serena/memories/backend/core.md @@ -41,6 +41,8 @@ Backend RPC command areas without focused memories include access tokens, binfil Database migrations live in `backend/src/app/migrations/`; pure SQL migrations are under `backend/src/app/migrations/sql/`. SQL filenames conventionally start with a sequence and verb/table description, e.g. `0026-mod-profile-table-add-is-active-field`. Applied migrations are tracked in the `migrations` table. +Migrations must stay within PostgreSQL 15 syntax: the official docker images (`docker/images/docker-compose.yaml`) pin `postgres:15`, so anything newer breaks those deployments (`SET STORAGE DEFAULT` is PostgreSQL 16, for instance). The documented floor is >=15. + For interactive PostgreSQL access with correct dev defaults, use `scripts/psql`; to dump the current DDL schema, use `scripts/db-schema` (see `mem:scripts/psql`). diff --git a/backend/src/app/migrations/sql/0156-reset-pg-storage-defaults.sql b/backend/src/app/migrations/sql/0156-reset-pg-storage-defaults.sql index 54bd98ace5..46a7cce54b 100644 --- a/backend/src/app/migrations/sql/0156-reset-pg-storage-defaults.sql +++ b/backend/src/app/migrations/sql/0156-reset-pg-storage-defaults.sql @@ -12,118 +12,120 @@ -- already stored TOAST values keep their strategy until the row is -- rewritten, new writes use the default. -- --- NOTE: DEFAULT restores the storage mode of the column's data type --- (EXTENDED for every column below: text, text[], jsonb and bytea). It --- requires PostgreSQL 16 or newer, which is the minimum version the --- project supports (see the database section of the technical guide). --- RESET STORAGE is not valid syntax for a column. +-- NOTE: `SET STORAGE DEFAULT` would express the same thing (it restores +-- the storage mode of the column's data type), but it only exists since +-- PostgreSQL 16 and the project also runs on PostgreSQL 15. Here every +-- column below has a type whose default storage mode is EXTENDED +-- (text, text[], jsonb and bytea), so `SET STORAGE EXTENDED` reaches the +-- same catalog state on any supported version. RESET STORAGE is not +-- valid syntax for a column. ALTER TABLE access_token - ALTER COLUMN name SET STORAGE DEFAULT, - ALTER COLUMN perms SET STORAGE DEFAULT, - ALTER COLUMN token SET STORAGE DEFAULT; + ALTER COLUMN name SET STORAGE EXTENDED, + ALTER COLUMN perms SET STORAGE EXTENDED, + ALTER COLUMN token SET STORAGE EXTENDED; ALTER TABLE audit_log - ALTER COLUMN context SET STORAGE DEFAULT, - ALTER COLUMN name SET STORAGE DEFAULT, - ALTER COLUMN props SET STORAGE DEFAULT, - ALTER COLUMN source SET STORAGE DEFAULT, - ALTER COLUMN type SET STORAGE DEFAULT; + ALTER COLUMN context SET STORAGE EXTENDED, + ALTER COLUMN name SET STORAGE EXTENDED, + ALTER COLUMN props SET STORAGE EXTENDED, + ALTER COLUMN source SET STORAGE EXTENDED, + ALTER COLUMN type SET STORAGE EXTENDED; ALTER TABLE comment - ALTER COLUMN content SET STORAGE DEFAULT; + ALTER COLUMN content SET STORAGE EXTENDED; ALTER TABLE comment_thread - ALTER COLUMN page_name SET STORAGE DEFAULT, - ALTER COLUMN participants SET STORAGE DEFAULT; + ALTER COLUMN page_name SET STORAGE EXTENDED, + ALTER COLUMN participants SET STORAGE EXTENDED; ALTER TABLE file - ALTER COLUMN data SET STORAGE DEFAULT, - ALTER COLUMN data_backend SET STORAGE DEFAULT, - ALTER COLUMN features SET STORAGE DEFAULT, - ALTER COLUMN name SET STORAGE DEFAULT; + ALTER COLUMN data SET STORAGE EXTENDED, + ALTER COLUMN data_backend SET STORAGE EXTENDED, + ALTER COLUMN features SET STORAGE EXTENDED, + ALTER COLUMN name SET STORAGE EXTENDED; ALTER TABLE file_change - ALTER COLUMN changes SET STORAGE DEFAULT, - ALTER COLUMN data SET STORAGE DEFAULT, - ALTER COLUMN features SET STORAGE DEFAULT, - ALTER COLUMN label SET STORAGE DEFAULT; + ALTER COLUMN changes SET STORAGE EXTENDED, + ALTER COLUMN data SET STORAGE EXTENDED, + ALTER COLUMN features SET STORAGE EXTENDED, + ALTER COLUMN label SET STORAGE EXTENDED; ALTER TABLE file_data_fragment - ALTER COLUMN metadata SET STORAGE DEFAULT; + ALTER COLUMN metadata SET STORAGE EXTENDED; ALTER TABLE file_media_object - ALTER COLUMN mtype SET STORAGE DEFAULT, - ALTER COLUMN name SET STORAGE DEFAULT; + ALTER COLUMN mtype SET STORAGE EXTENDED, + ALTER COLUMN name SET STORAGE EXTENDED; ALTER TABLE file_thumbnail - ALTER COLUMN data SET STORAGE DEFAULT, - ALTER COLUMN props SET STORAGE DEFAULT; + ALTER COLUMN data SET STORAGE EXTENDED, + ALTER COLUMN props SET STORAGE EXTENDED; ALTER TABLE global_complaint_report - ALTER COLUMN content SET STORAGE DEFAULT, - ALTER COLUMN type SET STORAGE DEFAULT; + ALTER COLUMN content SET STORAGE EXTENDED, + ALTER COLUMN type SET STORAGE EXTENDED; ALTER TABLE profile - ALTER COLUMN auth_backend SET STORAGE DEFAULT, - ALTER COLUMN email SET STORAGE DEFAULT, - ALTER COLUMN fullname SET STORAGE DEFAULT, - ALTER COLUMN lang SET STORAGE DEFAULT, - ALTER COLUMN password SET STORAGE DEFAULT, - ALTER COLUMN props SET STORAGE DEFAULT, - ALTER COLUMN theme SET STORAGE DEFAULT; + ALTER COLUMN auth_backend SET STORAGE EXTENDED, + ALTER COLUMN email SET STORAGE EXTENDED, + ALTER COLUMN fullname SET STORAGE EXTENDED, + ALTER COLUMN lang SET STORAGE EXTENDED, + ALTER COLUMN password SET STORAGE EXTENDED, + ALTER COLUMN props SET STORAGE EXTENDED, + ALTER COLUMN theme SET STORAGE EXTENDED; ALTER TABLE profile_complaint_report - ALTER COLUMN content SET STORAGE DEFAULT, - ALTER COLUMN type SET STORAGE DEFAULT; + ALTER COLUMN content SET STORAGE EXTENDED, + ALTER COLUMN type SET STORAGE EXTENDED; ALTER TABLE project - ALTER COLUMN name SET STORAGE DEFAULT; + ALTER COLUMN name SET STORAGE EXTENDED; ALTER TABLE scheduled_task - ALTER COLUMN cron_expr SET STORAGE DEFAULT, - ALTER COLUMN id SET STORAGE DEFAULT; + ALTER COLUMN cron_expr SET STORAGE EXTENDED, + ALTER COLUMN id SET STORAGE EXTENDED; ALTER TABLE server_error_report - ALTER COLUMN content SET STORAGE DEFAULT; + ALTER COLUMN content SET STORAGE EXTENDED; ALTER TABLE server_prop - ALTER COLUMN content SET STORAGE DEFAULT, - ALTER COLUMN id SET STORAGE DEFAULT; + ALTER COLUMN content SET STORAGE EXTENDED, + ALTER COLUMN id SET STORAGE EXTENDED; ALTER TABLE storage_object - ALTER COLUMN backend SET STORAGE DEFAULT, - ALTER COLUMN metadata SET STORAGE DEFAULT; + ALTER COLUMN backend SET STORAGE EXTENDED, + ALTER COLUMN metadata SET STORAGE EXTENDED; ALTER TABLE task - ALTER COLUMN error SET STORAGE DEFAULT, - ALTER COLUMN label SET STORAGE DEFAULT, - ALTER COLUMN name SET STORAGE DEFAULT, - ALTER COLUMN props SET STORAGE DEFAULT, - ALTER COLUMN queue SET STORAGE DEFAULT, - ALTER COLUMN status SET STORAGE DEFAULT; + ALTER COLUMN error SET STORAGE EXTENDED, + ALTER COLUMN label SET STORAGE EXTENDED, + ALTER COLUMN name SET STORAGE EXTENDED, + ALTER COLUMN props SET STORAGE EXTENDED, + ALTER COLUMN queue SET STORAGE EXTENDED, + ALTER COLUMN status SET STORAGE EXTENDED; ALTER TABLE team - ALTER COLUMN name SET STORAGE DEFAULT; + ALTER COLUMN name SET STORAGE EXTENDED; ALTER TABLE team_font_variant - ALTER COLUMN font_family SET STORAGE DEFAULT, - ALTER COLUMN font_style SET STORAGE DEFAULT; + ALTER COLUMN font_family SET STORAGE EXTENDED, + ALTER COLUMN font_style SET STORAGE EXTENDED; ALTER TABLE team_invitation - ALTER COLUMN email_to SET STORAGE DEFAULT, - ALTER COLUMN role SET STORAGE DEFAULT; + ALTER COLUMN email_to SET STORAGE EXTENDED, + ALTER COLUMN role SET STORAGE EXTENDED; ALTER TABLE usage_quote - ALTER COLUMN target SET STORAGE DEFAULT; + ALTER COLUMN target SET STORAGE EXTENDED; ALTER TABLE webhook - ALTER COLUMN error_code SET STORAGE DEFAULT, - ALTER COLUMN mtype SET STORAGE DEFAULT, - ALTER COLUMN secret_key SET STORAGE DEFAULT, - ALTER COLUMN uri SET STORAGE DEFAULT; + ALTER COLUMN error_code SET STORAGE EXTENDED, + ALTER COLUMN mtype SET STORAGE EXTENDED, + ALTER COLUMN secret_key SET STORAGE EXTENDED, + ALTER COLUMN uri SET STORAGE EXTENDED; ALTER TABLE webhook_delivery - ALTER COLUMN error_code SET STORAGE DEFAULT, - ALTER COLUMN req_data SET STORAGE DEFAULT, - ALTER COLUMN rsp_data SET STORAGE DEFAULT; + ALTER COLUMN error_code SET STORAGE EXTENDED, + ALTER COLUMN req_data SET STORAGE EXTENDED, + ALTER COLUMN rsp_data SET STORAGE EXTENDED; diff --git a/docs/technical-guide/configuration.md b/docs/technical-guide/configuration.md index 59939a522b..374c5e408c 100644 --- a/docs/technical-guide/configuration.md +++ b/docs/technical-guide/configuration.md @@ -484,7 +484,7 @@ PENPOT_SECRET_KEY: my-super-secure-key ### Database -Penpot only supports PostgreSQL and we highly recommend >=16 version. If you are using official +Penpot only supports PostgreSQL and we highly recommend >=15 version. If you are using official docker images this is already solved for you. Essential database configuration: