Repository navigation
Fix PDF editing tools and store annotations in the PDF - #4
Merged
Merged
Conversation
The annotation tools and the save pipeline were broken in several independent ways. PDFium interop - FPDFPageObj_SetMatrix was declared as taking six doubles, but PDFium takes a const FS_MATRIX* (six floats). Calling it crashed the process (0xC0000005), so saving any page that had annotations killed the app. Added the FsMatrix struct and the correct signature. - FPDFPage_InsertObject returns void, not int; the bogus return check could throw at random. Saving - PDFium reads the source file lazily, so writing straight back over it truncated the data being saved. SaveDocumentCore now stages to a temp file and moves it into place, and the new InPlaceSaveService closes the document, swaps the file and reopens it. Reader Save and Organize Save both use it, and a SessionReplaced event repoints every shared session reference. - Flattened overlays are cleared after an in-place save so they are not drawn twice. - PageRenderCache is keyed on a per-session id instead of the native handle, which PDFium recycles across close/open, and is invalidated on close. Signature - SignatureCanvas had no background, so it never received pointer input and nothing could be drawn. - Capture no longer depends on RenderTargetBitmap running after the dialog has closed; strokes are rasterized directly and cropped to their bounds, and the signature is placed at its true aspect ratio. Edit surface - Selection bounds come from the overlay model instead of ActualWidth / ActualHeight, which were still zero right after a re-render, so the selection box and resize handle now appear immediately. - Live ink no longer re-creates every stroke on each pointer move, which repainted existing strokes in the current colour and dropped their tags. - Select, ink and eraser hit-test the model on the canvas so they work on top of text and signatures; only the text tool leaves text boxes live. - Ink strokes gained an id, and can be selected, moved and deleted. - Undo stack is cleared per page and only pushed on real edits. - Empty text boxes are pruned instead of saving "Enter text...". Rendering - Composited text used typographic points where the surface uses pixels, so saved text came out about a third too large. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Saving a page that had any annotation replaced the entire page with a rasterized bitmap. That destroyed the text layer, links, bookmarks and form fields on that page, and resampled everything to a fixed 144 DPI, so a 300 DPI scan lost most of its detail and the file ballooned. Annotations are now written as native page objects: - ink becomes stroked vector paths - text becomes real, selectable, searchable text objects - signatures become image stamps PdfOverlayWriter maps the overlay's display coordinates (top-left origin, Y down, rotation-aware) into PDF user space (bottom-left origin, Y up, crop-box relative, rotation-agnostic), including glyph orientation, so annotations land correctly on rotated pages and on pages whose crop box does not start at the origin. Because the overlays are appended to the live document rather than copied into a new one, the outline, links, form fields and metadata all survive. That mutation is not reversible, so every save now stages to a temp file, releases the handle, moves it into place and reopens from the original path. Save and Save As share that path via DocumentSaveService, which raises SessionReplaced so both the reader and the organize view repoint their shared sessions. Text encoding - The PDF base-14 fonts are single-byte encoded, so anything outside Latin-1 would have been silently dropped. Arial is now embedded as a CID font with Identity-H, which also makes the saved text match the edit surface exactly. Text containing CJK falls back to a font that covers it. Also fixed - The form-fill environment was torn down after FPDF_CloseDocument, which is a use-after-free. Every save now closes the document, so this went from latent to reachable. - A failed reopen used to hand back the closed session and report a successful write as a failure, so the overlays were embedded twice on the next save. Losing the handle is now reported separately from whether the bytes landed. - Embedding is no longer cancellable midway, which could otherwise leave some pages annotated and others not. Tests - ElliePdf.Tests now compiles the non-UI PDF sources directly and runs against the real pdfium.dll: text-layer preservation, ink colour and placement, text positioning via the char box, wrapping, Unicode round-trip, signature stamping, and all four page rotations. Verified by mutation that 10 of the 16 fail if the embedding regresses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…signatures Six gaps in the annotation tools, all extending the vector-annotation pipeline rather than the old flattening one. Undo and redo - History moves out of PdfEditSurface into OverlayHistory, keyed per tab and storing the page each snapshot belongs to. Undo previously died the moment you changed page, because loading a page cleared the stack; now it walks back across the whole document and navigates to wherever the edit was made. - Redo added, on Ctrl+Y and Ctrl+Shift+Z. - History is dropped when a tab closes and after a save flattens the overlays, since replaying an edit that is already baked into the page would draw it twice. Pen pressure - Ink points carry pressure, and strokes that vary are rendered and embedded as a tapered outline instead of a constant-width line. Digitizers that report no pressure send a flat 0.5 for every sample, so a stroke that never varies is normalised back to full width rather than being drawn at 67% of the chosen thickness. - Touch is ignored while a pen is in range, so a resting palm no longer scribbles on the page, and secondary contacts are ignored mid-stroke. Shapes - Rectangle, ellipse, line and arrow, with drag-to-draw preview, selection, move and resize. They are written to the PDF as real vector paths, ellipses as four beziers and arrowheads as filled polygons. Restyling - Ink and shapes were fixed at their creation colour and thickness. A style toolbar now edits the selection, including a fill toggle for closed shapes. The colour picker records one undo entry per interaction, not one per sampled colour. Eraser - Adjustable radius with a circle cursor showing the reach, and partial erase that cuts a stroke into fragments instead of deleting the whole thing. One undo entry covers a whole drag. Signatures - Signatures persist to settings, so they no longer have to be redrawn for every document, and can also be typed or imported from a photo or scan. Importing keys out a light background, working over a locked buffer and downscaling first so a phone photo does not freeze the window for several seconds. Also fixed - Resize scaled against the padded selection bounds while dividing by the unpadded geometry, so grabbing the handle instantly inflated the item by the padding: 1.9x for a thin stroke drawn with a thick pen. - A filled shape was opaque on screen but written at 27% alpha, so the saved file did not match what the shape was positioned against. - Hit tolerance is now defined in screen pixels and converted, so targets are equally easy to hit at any zoom. Tests - 53 tests. InkGeometry (taper, splitting, distance), ShapeGeometry (bounds, bezier ellipse, arrowhead, interiors) and OverlayHistory (cross-page undo, redo, branch invalidation, deep copies) are covered directly, and shapes and pressure ink are round-tripped through the real PDFium save path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Annotations were kept in a <file>.pdf.ellie.json sidecar while editing and only flattened into page content on save. Sharing the PDF on its own therefore risked losing work, and once flattened the annotations could no longer be selected or removed by any tool, including this one. Annotations are now written as standard PDF annotations: ink as /Ink, everything else as /Stamp, each with an appearance stream built from the same vector paths, text objects and images as before. Any viewer can display, move or delete them, they print, and text also lands in /Contents where viewers look when searching or listing comments. Reopening a document reads those annotations back and detaches them from the page, so editing continues exactly where it left off — including pen pressure, which an appearance stream alone could not preserve — and nothing renders twice. Each annotation carries its overlay item under a private key to make that lossless. Annotations from other tools are left untouched. The sidecar is gone: AnnotationStore is purely in-memory, and the auto-save-companion setting and its UI have been removed. A leftover .ellie.json is imported once when the document is opened and then deleted, so no existing work is lost and no new ones appear. Because annotations are held out of the page while a document is open, the overlay now stays visible outside edit mode; only pointer interaction is gated on editing. Data-loss paths this had to close - Organize export and merge copied pages from sessions whose annotations had been detached by a reader tab, silently dropping them — including annotations that were already in the file and had never been edited. Exports now put them back first and detach them again afterwards. - Save As reopened the original file, whose annotations were still on the page, while the overlays were also live in memory. They drew twice and each subsequent save wrote another physical copy. Overlays are now re-detached whenever the session is replaced, which also leaves them editable after an in-place save. - Deleting a page in Organize left overlay keys pointing at the wrong pages, since the store is keyed by absolute page index. Also fixed - The annotation rectangle was derived from an overlay's control points, but PDFium fixes the appearance stream's bounding box from it. A horizontal or vertical arrow has no extent across the shaft, so its barbs were sliced off in every other viewer while looking correct here. The rectangle now comes from the geometry actually emitted. Page objects cannot be measured instead: their bounds read as zero until they belong to a page or form. Tests - 71 tests. Round-trip (detach, edit, resave, no duplication), foreign annotations left alone, visibility to other viewers, straight-arrow clipping, page-index shifting, and that no save produces a companion file while a legacy one is imported and removed. The suite now runs serially, since PDFium and the harness share process-wide state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f995b9c8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+259
to
+262
| if (result.Saved && result.Session is not null) | ||
| { | ||
| saved++; | ||
| } |
There was a problem hiding this comment.
When a document is imported in Organize, opened in Reader so both views share its session, edited, and then saved from Organize, SaveAsync reopens a file containing the embedded annotations and the replacement event remaps both views to it. This success branch neither extracts those annotations from the replacement nor marks the shared tab/store clean, unlike EditSaveService, so Reader displays the embedded annotations together with the still-live overlay, continues to report unsaved changes, and each later save can append another copy.
Useful? React with 👍 / 👎.
Comment on lines
368
to
372
| finally | ||
| { | ||
| // Detach them again so the reader keeps editing them rather than seeing them twice. | ||
| await DetachOverlaysAfterExportAsync(cancellationToken); | ||
| IsBusy = false; |
There was a problem hiding this comment.
If cancellation occurs after RestoreOverlaysForExportAsync has appended annotations—for example, while MergeOrderedPagesAsync is importing pages—this finally block reuses the already-cancelled token. ExtractOverlaysAsync can therefore reject immediately while acquiring the PDFium gate, leaving the temporary annotations attached alongside Reader's overlay; subsequent exports or saves can then append duplicates. Cleanup should be non-cancellable and limited to sessions that were successfully restored.
Useful? React with 👍 / 👎.
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.
PDF annotations and editing had reliability gaps: drawing/signing tools were buggy, undo history could be lost, and edits depended on a
.ellie.jsoncompanion file. This change makes annotation workflows robust and self-contained so shared PDFs work correctly in other viewers and editors.What changed
/Inkand/Stamp) with appearance streams..ellie.jsondata if present, then delete it.Notable implementation details
/Contentsfor compatibility with viewer comment/search surfaces.Validation
Reviewer focus
Please pay extra attention to cross-flow persistence boundaries (save-as, organize/merge, and page deletion while a reader tab is open), since those were the key data-loss risks addressed here.