mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 05:58:12 -06:00
M2: page_render and render_preview report the render warnings next to the PNG (#4340)
The renderer reports when a page's picture is incomplete or approximate
(content cut at the page's safety budget, a colour painted in a fallback
colour), but only `pdfcraft-cli render` showed it. `page_render` and
`render_preview` returned the PNG alone, so a script or MCP client could
not tell that the image was not the whole page.
Both tools now return a JSON part after the PNG when the page has
warnings: `{ "page": 1, "warnings": [...] }`, with the sentences the CLI
prints. Warnings are strings, like every other tool's `warnings`, so
`command_run` can add its own to the same list. A page without warnings
still returns the PNG alone, so existing callers see no change.
The viewer notice the issue also asks for is left for a separate change.
This commit is contained in:
@@ -181,6 +181,12 @@ working document unchanged.
|
||||
`content_list` or `object_list`. It removes only that item in a read-only preview and is
|
||||
mutually exclusive with `hide_text_block`.
|
||||
|
||||
When the renderer reports a warning for the page, `page_render` and `render_preview` return a
|
||||
JSON part after the PNG, `{ "page": 1, "warnings": [...] }`, with the sentences
|
||||
`pdfcraft-cli render` prints: "content past the page's safety budget was skipped" and "a colour
|
||||
that could not be computed was painted in a fallback colour". A page without warnings returns
|
||||
the PNG alone.
|
||||
|
||||
|
||||
## Existing text and document fonts
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Shared command/inspection/preview entry points, using the existing confined tool path.
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::{Args, Automation, Content, Result, ToolError, encode_png, failed, info, tools};
|
||||
use crate::{Args, Automation, Content, Result, ToolError, failed, info, rendered_content, tools};
|
||||
use pdfcraft_engine::commands;
|
||||
use pdfcraft_render::{RenderRequest, RequestKind};
|
||||
|
||||
@@ -108,7 +108,8 @@ impl Automation {
|
||||
Ok(json!({"documents":self.session.docs().iter().map(info).collect::<Vec<_>>()}))
|
||||
}
|
||||
|
||||
pub(crate) fn render_preview(&mut self, args: &Args) -> Result<Content> {
|
||||
/// A bounded PNG of the page, with its render warnings after it as `page_render` gives them.
|
||||
pub(crate) fn render_preview(&mut self, args: &Args) -> Result<Vec<Content>> {
|
||||
let id = if args.opt_int("doc")?.is_some() {
|
||||
self.doc(args)?.id
|
||||
} else {
|
||||
@@ -134,7 +135,6 @@ impl Automation {
|
||||
if let Some(error) = out.error {
|
||||
return Err(failed(error));
|
||||
}
|
||||
let data = encode_png(out.width, out.height, &out.rgba)?;
|
||||
Ok(Content::Png { data, width: out.width, height: out.height })
|
||||
rendered_content(&out, page)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -225,7 +225,7 @@ impl Automation {
|
||||
let edit = Edit::SetInfo { key: a.str("key")?.into(), value: a.str("value")?.into() };
|
||||
self.apply(&a, edit)?
|
||||
}
|
||||
"page_render" => return self.page_render(&a).map(|c| vec![c]),
|
||||
"page_render" => return self.page_render(&a),
|
||||
"comment_image_preview" => return self.comment_image_preview(&a),
|
||||
"text_extract" => self.text_extract(&a)?,
|
||||
"text_word_count" => self.text_word_count(&a)?,
|
||||
@@ -329,7 +329,7 @@ impl Automation {
|
||||
"command_run" => return self.command_run(&a),
|
||||
"command_batch" => self.command_batch(&a)?,
|
||||
"doc_inspect" => self.doc_inspect(&a)?,
|
||||
"render_preview" => return self.render_preview(&a).map(|c| vec![c]),
|
||||
"render_preview" => return self.render_preview(&a),
|
||||
"page_number" => {
|
||||
use pdfcraft_organize::LabelStyle as L;
|
||||
let n = self.doc(&a)?.info.pages.len();
|
||||
@@ -2023,7 +2023,9 @@ impl Automation {
|
||||
Ok(&mut entry.1)
|
||||
}
|
||||
|
||||
fn page_render(&mut self, a: &Args) -> Result<Content> {
|
||||
/// The page as a PNG, followed by a JSON part listing the render warnings when there are any,
|
||||
/// so a caller can tell that the picture is incomplete, as `pdfcraft-cli render` does (#4340).
|
||||
fn page_render(&mut self, a: &Args) -> Result<Vec<Content>> {
|
||||
let id = self.doc(a)?.id;
|
||||
let page = self.page(a)?;
|
||||
let dpi = a.opt_num("dpi")?.unwrap_or(DEFAULT_DPI);
|
||||
@@ -2056,8 +2058,7 @@ impl Automation {
|
||||
if let Some(e) = out.error {
|
||||
return Err(failed(format!("page {} could not be rendered: {e}", page + 1)));
|
||||
}
|
||||
let data = encode_png(out.width, out.height, &out.rgba)?;
|
||||
Ok(Content::Png { data, width: out.width, height: out.height })
|
||||
rendered_content(&out, page)
|
||||
}
|
||||
|
||||
/// A page as RGBA pixels, without encoding a PNG. `page` is 1-based and `scale` is pixels
|
||||
@@ -2733,6 +2734,29 @@ fn encode_png(width: u32, height: u32, premultiplied: &[u8]) -> Result<Vec<u8>>
|
||||
pdfcraft_engine::export::encode_png(width, height, premultiplied).map_err(failed)
|
||||
}
|
||||
|
||||
/// The output of a page-rendering tool: the PNG of the 0-based `page`, followed by a JSON part
|
||||
/// listing its render warnings when there are any. Without warnings it is the PNG alone, as it
|
||||
/// always was.
|
||||
fn rendered_content(out: &pdfcraft_render::RenderedPage, page: usize) -> Result<Vec<Content>> {
|
||||
let data = encode_png(out.width, out.height, &out.rgba)?;
|
||||
let mut content = vec![Content::Png { data, width: out.width, height: out.height }];
|
||||
if !out.warnings.is_empty() {
|
||||
// Sentences, like the other tools' `warnings`, so command_run can add its own to the list.
|
||||
let warnings: Vec<&str> = out.warnings.iter().map(|&w| render_warning_text(w)).collect();
|
||||
content.push(Content::Json(json!({ "page": page.saturating_add(1), "warnings": warnings })));
|
||||
}
|
||||
Ok(content)
|
||||
}
|
||||
|
||||
/// What a render warning means, in the words `pdfcraft-cli render` prints for it.
|
||||
fn render_warning_text(warning: pdfcraft_render::RenderWarning) -> &'static str {
|
||||
use pdfcraft_render::RenderWarning;
|
||||
match warning {
|
||||
RenderWarning::ContentTruncated => "content past the page's safety budget was skipped",
|
||||
RenderWarning::ColourFallback => "a colour that could not be computed was painted in a fallback colour",
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the text of `pages` (0-based) with one renderer per worker thread.
|
||||
fn extract_parallel(bytes: &Arc<Vec<u8>>, password: Option<Arc<str>>, pages: &[usize]) -> PageTexts {
|
||||
let extract = |r: &mut PageRenderer, p: usize| {
|
||||
|
||||
@@ -164,7 +164,7 @@ pub fn tools() -> Vec<ToolDef> {
|
||||
)),
|
||||
t("doc_set_info", "Set document metadata", "Set a document information entry such as Title, Author, Subject or Keywords, or a custom property under any other name (no spaces, delimiters, / or #; at most 127 bytes; doc_info lists them under custom); an empty value removes it. When the file has XMP metadata, the matching XMP property (dc:title, dc:creator, dc:description, pdf:Keywords, xmp:CreatorTool, pdf:Producer) is changed too, so viewers that read XMP first show the new value. Undoable.")
|
||||
.with(schema(json!({ "doc": doc(), "key": { "type": "string" }, "value": { "type": "string" } }), &["doc", "key", "value"])),
|
||||
t("page_render", "Render a page", "Render one page to a PNG image (default 96 dpi, at most 600).")
|
||||
t("page_render", "Render a page", "Render one page to a PNG image (default 96 dpi, at most 600). When the picture is incomplete or approximate (content cut at the page's safety budget, a colour painted in a fallback colour), a JSON part follows the image: { page, warnings: [sentence, ...] }.")
|
||||
.ro()
|
||||
.with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "dpi": { "type": "number", "minimum": 1, "maximum": 600 },
|
||||
"hide_text_block": { "type": "integer", "minimum": 1, "description": "Render an inline editor background without this paragraph (number from text_paragraphs, resolved as text_edit does: refused once an edit merged that paragraph into a neighbour). Does not modify the document." },
|
||||
@@ -1442,7 +1442,7 @@ pub fn tools() -> Vec<ToolDef> {
|
||||
.destructive().with(schema(json!({"steps":{"type":"array","items":schema(json!({"id":{"type":"string"},"params":{"type":"object"}}), &["id"])},"stop_on_error":{"type":"boolean"}}), &["steps"])),
|
||||
t("doc_inspect", "Inspect open documents", "With doc, return doc_info; otherwise return information for every open document (empty documents array when none are open).")
|
||||
.ro().with(schema(json!({"doc":doc()}), &[])),
|
||||
t("render_preview", "Render a preview", "Render a PNG bounded by max_side (default 1024), without changing or saving the document. Page defaults to 1; doc may be omitted when exactly one document is open.")
|
||||
t("render_preview", "Render a preview", "Render a PNG bounded by max_side (default 1024), without changing or saving the document. Page defaults to 1; doc may be omitted when exactly one document is open. Render warnings follow the image as page_render gives them.")
|
||||
.ro().with(schema(json!({"doc":doc(),"page":{"type":"integer","minimum":1},"max_side":{"type":"integer","minimum":1,"maximum":4096}}), &[])),
|
||||
]
|
||||
}
|
||||
|
||||
@@ -8,3 +8,4 @@ mod button_caption;
|
||||
mod form_bulk;
|
||||
mod hostile_args;
|
||||
mod optimize_content_boundaries;
|
||||
mod render_warnings;
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
//! `page_render` reports the renderer's warnings next to the PNG, so a headless caller can tell
|
||||
//! that a picture is incomplete or approximate, as `pdfcraft-cli render` can (#4340).
|
||||
|
||||
use std::path::PathBuf;
|
||||
|
||||
use pdfcraft_automation::{Automation, Content};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
/// The warning a colour that falls back gives, worded as `pdfcraft-cli render` prints it.
|
||||
const FALLBACK: &str = "a colour that could not be computed was painted in a fallback colour";
|
||||
|
||||
/// A one-page 200 × 200 pt PDF that draws `content`, with the colour space `CS1` defined as
|
||||
/// `[/DeviceN [/Spot1 /Spot2] /DeviceRGB 5 0 R]` and object 5 a calculator function `program`.
|
||||
fn devicen_pdf(content: &str, program: &str) -> Vec<u8> {
|
||||
format!(
|
||||
"%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\
|
||||
2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\
|
||||
3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << /ColorSpace << /CS1 [/DeviceN [/Spot1 /Spot2] /DeviceRGB 5 0 R] >> >> /Contents 4 0 R >> endobj\n\
|
||||
4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n\
|
||||
5 0 obj << /FunctionType 4 /Domain [0 1 0 1] /Range [0 1 0 1 0 1] /Length {} >> stream\n{program}\nendstream endobj\n\
|
||||
trailer << /Root 1 0 R >>\n%%EOF",
|
||||
content.len(),
|
||||
program.len()
|
||||
)
|
||||
.into_bytes()
|
||||
}
|
||||
|
||||
/// A page whose colour falls back: the tint transform pops more than it is given, so it can't be
|
||||
/// evaluated (the renderer's own test of the warning uses the same function).
|
||||
fn fallback_pdf() -> Vec<u8> {
|
||||
devicen_pdf("/CS1 cs 0.5 0.5 scn 0 0 100 100 re f", "{ pop pop pop }")
|
||||
}
|
||||
|
||||
/// The same page with a tint transform that works: nothing to warn about.
|
||||
fn clean_pdf() -> Vec<u8> {
|
||||
devicen_pdf("/CS1 cs 0.5 0.5 scn 0 0 100 100 re f", "{ pop pop 0 0 0 }")
|
||||
}
|
||||
|
||||
/// A fresh folder holding `fallback.pdf` and `clean.pdf`, and an automation rooted there.
|
||||
fn setup(test: &str) -> (PathBuf, Automation) {
|
||||
let dir = std::env::temp_dir().join(format!("pdfcraft-render-warnings-{test}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
std::fs::write(dir.join("fallback.pdf"), fallback_pdf()).unwrap();
|
||||
std::fs::write(dir.join("clean.pdf"), clean_pdf()).unwrap();
|
||||
let a = Automation::new().with_root(&dir).unwrap();
|
||||
(dir, a)
|
||||
}
|
||||
|
||||
fn open(a: &mut Automation, path: &str) -> u64 {
|
||||
match a.call("doc_open", &json!({ "path": path })).unwrap().remove(0) {
|
||||
Content::Json(v) => v["doc"].as_u64().unwrap(),
|
||||
other => panic!("expected JSON: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn page_render_lists_the_warnings_after_the_png() {
|
||||
let (dir, mut a) = setup("fallback");
|
||||
let doc = open(&mut a, "fallback.pdf");
|
||||
let out = a.call("page_render", &json!({ "doc": doc, "page": 1, "dpi": 72 })).unwrap();
|
||||
assert_eq!(out.len(), 2, "the PNG and the warnings: {out:?}");
|
||||
assert!(matches!(out[0], Content::Png { width: 200, height: 200, .. }), "the image comes first, as before");
|
||||
let Content::Json(report) = &out[1] else { panic!("expected the warnings as JSON: {:?}", out[1]) };
|
||||
assert_eq!(report["page"], 1, "the page is 1-based, like the argument");
|
||||
assert_eq!(report["warnings"], json!([FALLBACK]));
|
||||
let _ = std::fs::remove_dir_all(dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_preview_lists_the_warnings_after_the_png_too() {
|
||||
let (dir, mut a) = setup("preview");
|
||||
let doc = open(&mut a, "fallback.pdf");
|
||||
let out = a.call("render_preview", &json!({ "doc": doc, "max_side": 64 })).unwrap();
|
||||
assert!(matches!(out.first(), Some(Content::Png { width: 64, height: 64, .. })), "{out:?}");
|
||||
let Some(Content::Json(report)) = out.get(1) else { panic!("expected the warnings as JSON: {out:?}") };
|
||||
assert_eq!(report["page"], 1, "the page defaults to the first");
|
||||
assert_eq!(report["warnings"], json!([FALLBACK]));
|
||||
let clean = open(&mut a, "clean.pdf");
|
||||
assert_eq!(a.call("render_preview", &json!({ "doc": clean })).unwrap().len(), 1, "a clean page gives the PNG alone");
|
||||
let _ = std::fs::remove_dir_all(dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_clean_page_renders_to_the_png_alone() {
|
||||
let (dir, mut a) = setup("clean");
|
||||
let fallback = open(&mut a, "fallback.pdf");
|
||||
let clean = open(&mut a, "clean.pdf");
|
||||
// The warning of one render doesn't carry over to the next, nor to another document.
|
||||
a.call("page_render", &json!({ "doc": fallback, "page": 1, "dpi": 72 })).unwrap();
|
||||
let out = a.call("page_render", &json!({ "doc": clean, "page": 1, "dpi": 72 })).unwrap();
|
||||
assert_eq!(out.len(), 1, "no warnings, no extra part: {out:?}");
|
||||
assert!(matches!(out[0], Content::Png { .. }));
|
||||
let again = a.call("page_render", &json!({ "doc": fallback, "page": 1, "dpi": 72 })).unwrap();
|
||||
assert_eq!(again.len(), 2, "a cached renderer still reports the page's warning: {again:?}");
|
||||
let _ = std::fs::remove_dir_all(dir);
|
||||
}
|
||||
|
||||
/// Over MCP the warnings are a text block after the image and the call's `structuredContent`.
|
||||
#[cfg(feature = "mcp")]
|
||||
#[test]
|
||||
fn mcp_page_render_carries_the_warnings_as_structured_content() {
|
||||
use pdfcraft_automation::mcp::McpServer;
|
||||
let (dir, a) = setup("mcp");
|
||||
let mut s = McpServer::new(a);
|
||||
let mut rpc = |id: u64, method: &str, params: Value| -> Value {
|
||||
let line = json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params }).to_string();
|
||||
serde_json::from_str(&s.handle_line(&line).expect("a reply")).unwrap()
|
||||
};
|
||||
rpc(1, "initialize", json!({}));
|
||||
rpc(2, "tools/call", json!({ "name": "doc_open", "arguments": { "path": "fallback.pdf" } }));
|
||||
let reply = rpc(3, "tools/call", json!({ "name": "page_render", "arguments": { "doc": 1, "page": 1 } }));
|
||||
let result = &reply["result"];
|
||||
assert_eq!(result["isError"], false, "{reply}");
|
||||
assert_eq!(result["content"][0]["type"], "image");
|
||||
assert_eq!(result["content"][1]["type"], "text");
|
||||
assert_eq!(result["structuredContent"]["warnings"], json!([FALLBACK]));
|
||||
let _ = std::fs::remove_dir_all(dir);
|
||||
}
|
||||
Reference in New Issue
Block a user