pdf-renderer: Refactor PdfRenderer to own document and decouple canva…#113
Merged
pdf-renderer: Refactor PdfRenderer to own document and decouple canva…#113
Conversation
…s backend This commit refactors the PdfRenderer struct to take ownership of the PdfDocument and removes the lifetime parameters and direct coupling to a canvas backend. The renderer now exposes methods to access the owned document and to render pages by passing a mutable backend reference per call. The render_page_to_recording function is now a method, and a helper for page lookup is added. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors pdf-renderer so PdfRenderer owns the PdfDocument and no longer holds a long-lived mutable borrow of a specific canvas backend, instead taking a mutable backend reference per render call. The examples and page-cache docs are updated to use the new API.
Changes:
- Refactor
PdfRendererto ownPdfDocument, remove lifetimes/generic backend coupling, and adddocument()/into_document()accessors. - Update rendering entrypoints to accept
&mut impl CanvasBackendper call, includingrender_page_cachedand the recording helper now as a method. - Update Skia/FemtoVG/Emscripten examples to use the new renderer ownership model.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
crates/pdf-renderer/src/lib.rs |
Refactors PdfRenderer ownership + rendering API; updates cache helper to take &PdfRenderer. |
crates/pdf-renderer/src/page_cache.rs |
Updates module docs/example to use the new PdfRenderer recording method. |
examples/skia.rs |
Removes Arc<PdfDocument> usage; stores a PdfRenderer in the application state and renders with a per-call backend. |
examples/femtovg.rs |
Updates app trait/state to pass around &PdfRenderer rather than &PdfDocument. |
examples/emscripten.rs |
Switches thread-local storage from PdfDocument to PdfRenderer; updates cached rendering callsites accordingly. |
Comments suppressed due to low confidence (1)
crates/pdf-renderer/src/lib.rs:49
PdfRenderer::rendernow takes acanvas_backend: &mut Bargument, but the doc comment’s “# Parameters” section only documentspage_index. Please update the docs to includecanvas_backend(and what coordinate space/size it represents) so callers know what to pass.
/// Renders a page onto the canvas backend.
///
/// # Parameters
///
/// - `page_index` – Zero-based index of the page to render.
///
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+121
to
125
| CURRENT_RENDERER.with(|renderer| { | ||
| let renderer_ref = renderer.borrow(); | ||
| let Some(renderer) = renderer_ref.as_ref() else { | ||
| return -2; | ||
| }; |
Comment on lines
132
to
136
| // Cache miss: render to recording canvas | ||
| let recording = render_page_to_recording(document, page_index, width, height)?; | ||
| let recording = renderer.render_page_to_recording(page_index, width, height)?; | ||
|
|
||
| // Replay to the actual backend | ||
| recording.replay(backend)?; |
Comment on lines
+69
to
+70
| /// This records all drawing commands for a page into a resolution-independent | ||
| /// `RecordingCanvas` that can be replayed to any backend at any size. |
Comment on lines
+21
to
23
| //! let recording = renderer.render_page_to_recording(page_index, width, height)?; | ||
| //! cache.insert(page_index, recording.clone()); | ||
| //! recording.replay(&mut backend)?; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…s backend
This commit refactors the PdfRenderer struct to take ownership of the PdfDocument and removes the lifetime parameters and direct coupling to a canvas backend. The renderer now exposes methods to access the owned document and to render pages by passing a mutable backend reference per call. The render_page_to_recording function is now a method, and a helper for page lookup is added.