From e87fecf710cf2e8af1a58c317bd9911e01322a9b Mon Sep 17 00:00:00 2001 From: Rodrigo Tobar Date: Wed, 14 Dec 2022 23:00:40 +0800 Subject: [PATCH] LibPDF: Switch to best-effort PDF rendering The current rendering routine aborts as soon as an error is found during rendering, which potentially severely limits the contents we show on screen. Moreover, whenever an error happens the PDFViewer widget shows an error dialog, and doesn't display the bitmap that has been painted so far. This commit improves the situation in both fronts, implementing rendering now with a best-effort approach. Firstly, execution of operations isn't halted after an operand results in an error, but instead execution of all operations is always attempted, and all collected errors are returned in bulk. Secondly, PDFViewer now always displays the resulting bitmap, regardless of error being produced or not. To communicate errors, an on_render_errors callback has been added so clients can subscribe to these events and handle them as appropriate. --- Userland/Applications/PDFViewer/PDFViewer.cpp | 7 ++++- Userland/Applications/PDFViewer/PDFViewer.h | 1 + Userland/Libraries/LibPDF/Renderer.cpp | 26 ++++++++++++------- Userland/Libraries/LibPDF/Renderer.h | 4 +-- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Userland/Applications/PDFViewer/PDFViewer.cpp b/Userland/Applications/PDFViewer/PDFViewer.cpp index 6b461a87ac..746442f386 100644 --- a/Userland/Applications/PDFViewer/PDFViewer.cpp +++ b/Userland/Applications/PDFViewer/PDFViewer.cpp @@ -319,7 +319,12 @@ PDF::PDFErrorOr> PDFViewer::render_page(u32 page_inde auto& page_size = m_page_dimension_cache.render_info[page_index].size; auto bitmap = TRY(Gfx::Bitmap::try_create(Gfx::BitmapFormat::BGRA8888, page_size.to_type())); - TRY(PDF::Renderer::render(*m_document, page, bitmap, m_rendering_preferences)); + auto maybe_errors = PDF::Renderer::render(*m_document, page, bitmap, m_rendering_preferences); + if (maybe_errors.is_error()) { + auto errors = maybe_errors.release_error(); + on_render_errors(page_index, errors); + return bitmap; + } if (page.rotate + m_rotations != 0) { int rotation_count = ((page.rotate + m_rotations) / 90) % 4; diff --git a/Userland/Applications/PDFViewer/PDFViewer.h b/Userland/Applications/PDFViewer/PDFViewer.h index 59d0d25a90..91bba33334 100644 --- a/Userland/Applications/PDFViewer/PDFViewer.h +++ b/Userland/Applications/PDFViewer/PDFViewer.h @@ -53,6 +53,7 @@ public: PDF::PDFErrorOr set_document(RefPtr); Function on_page_change; + Function on_render_errors; void zoom_in(); void zoom_out(); diff --git a/Userland/Libraries/LibPDF/Renderer.cpp b/Userland/Libraries/LibPDF/Renderer.cpp index de597c6e21..1cc8186e9b 100644 --- a/Userland/Libraries/LibPDF/Renderer.cpp +++ b/Userland/Libraries/LibPDF/Renderer.cpp @@ -22,7 +22,7 @@ namespace PDF { -PDFErrorOr Renderer::render(Document& document, Page const& page, RefPtr bitmap, RenderingPreferences rendering_preferences) +PDFErrorsOr Renderer::render(Document& document, Page const& page, RefPtr bitmap, RenderingPreferences rendering_preferences) { return Renderer(document, page, bitmap, rendering_preferences).render(); } @@ -91,7 +91,7 @@ Renderer::Renderer(RefPtr document, Page const& page, RefPtrfill(Gfx::Color::NamedColor::White); } -PDFErrorOr Renderer::render() +PDFErrorsOr Renderer::render() { // Use our own vector, as the /Content can be an array with multiple // streams which gets concatenated @@ -113,26 +113,32 @@ PDFErrorOr Renderer::render() auto operators = TRY(Parser::parse_operators(m_document, byte_buffer)); - for (auto& op : operators) - TRY(handle_operator(op)); - + Errors errors; + for (auto& op : operators) { + auto maybe_error = handle_operator(op); + if (maybe_error.is_error()) { + errors.add_error(maybe_error.release_error()); + } + } + if (!errors.errors().is_empty()) + return errors; return {}; } PDFErrorOr Renderer::handle_operator(Operator const& op, Optional> extra_resources) { switch (op.type()) { -#define V(name, snake_name, symbol) \ - case OperatorType::name: \ - MUST(handle_##snake_name(op.arguments(), extra_resources)); \ +#define V(name, snake_name, symbol) \ + case OperatorType::name: \ + TRY(handle_##snake_name(op.arguments(), extra_resources)); \ break; ENUMERATE_OPERATORS(V) #undef V case OperatorType::TextNextLineShowString: - MUST(handle_text_next_line_show_string(op.arguments())); + TRY(handle_text_next_line_show_string(op.arguments())); break; case OperatorType::TextNextLineShowStringSetSpacing: - MUST(handle_text_next_line_show_string_set_spacing(op.arguments())); + TRY(handle_text_next_line_show_string_set_spacing(op.arguments())); break; } diff --git a/Userland/Libraries/LibPDF/Renderer.h b/Userland/Libraries/LibPDF/Renderer.h index c8d0814b64..ccf2f9be73 100644 --- a/Userland/Libraries/LibPDF/Renderer.h +++ b/Userland/Libraries/LibPDF/Renderer.h @@ -96,12 +96,12 @@ struct RenderingPreferences { class Renderer { public: - static PDFErrorOr render(Document&, Page const&, RefPtr, RenderingPreferences preferences); + static PDFErrorsOr render(Document&, Page const&, RefPtr, RenderingPreferences preferences); private: Renderer(RefPtr, Page const&, RefPtr, RenderingPreferences); - PDFErrorOr render(); + PDFErrorsOr render(); PDFErrorOr handle_operator(Operator const&, Optional> = {}); #define V(name, snake_name, symbol) \