Merge branch 'dev' of https://gitea.maskantech.in/gitea_admin/pdf into azeem
This commit is contained in:
@@ -171,7 +171,8 @@ PYBIND11_MODULE(pdfengine, m) {
|
|||||||
.def_readonly("author", &pdfengine::PdfPage::AnnotationInfo::author)
|
.def_readonly("author", &pdfengine::PdfPage::AnnotationInfo::author)
|
||||||
.def_readonly("content", &pdfengine::PdfPage::AnnotationInfo::content)
|
.def_readonly("content", &pdfengine::PdfPage::AnnotationInfo::content)
|
||||||
.def_readonly("timestamp", &pdfengine::PdfPage::AnnotationInfo::timestamp)
|
.def_readonly("timestamp", &pdfengine::PdfPage::AnnotationInfo::timestamp)
|
||||||
.def_readonly("page_index", &pdfengine::PdfPage::AnnotationInfo::pageIndex);
|
.def_readonly("page_index", &pdfengine::PdfPage::AnnotationInfo::pageIndex)
|
||||||
|
.def_readonly("paths", &pdfengine::PdfPage::AnnotationInfo::paths);
|
||||||
|
|
||||||
py::class_<pdfengine::PdfPage, std::shared_ptr<pdfengine::PdfPage>>(m, "PdfPage")
|
py::class_<pdfengine::PdfPage, std::shared_ptr<pdfengine::PdfPage>>(m, "PdfPage")
|
||||||
.def_property_readonly("width", &pdfengine::PdfPage::width)
|
.def_property_readonly("width", &pdfengine::PdfPage::width)
|
||||||
|
|||||||
@@ -0,0 +1,117 @@
|
|||||||
|
# Engine Roadmap — Reality Audit & Gap Analysis
|
||||||
|
|
||||||
|
**Status:** Review-only. No code changed to produce this.
|
||||||
|
**Question answered:** Of the roadmap items marked *Done*, which are **real** and which are **mocks/stubs**? And do the stubs belong to a later phase?
|
||||||
|
|
||||||
|
## How this was verified (not taken on faith)
|
||||||
|
1. Read the C++ engine dispatch in [pdfium_document.cpp](engine/src/parser/pdfium_document.cpp) — specifically `applyEdits` (line 1432) and every edit-type handler.
|
||||||
|
2. **Runtime-tested each edit op through the real engine bindings** (gateway venv): apply → `save` → reload → assert the change actually persisted (annotation count, text length, page count).
|
||||||
|
3. Read the gateway edit pipeline [edits.py](gateway/app/routers/edits.py) (incl. the image preprocessing).
|
||||||
|
4. Confirmed deps + artifacts: Pillow 10.4.0 present (needed for images), `pdfengine.wasm` (60 KB) built, engine `pdfium=on, skia=off`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. Headline findings (the important part)
|
||||||
|
|
||||||
|
| # | Finding | Impact |
|
||||||
|
|---|---|---|
|
||||||
|
| **F1** | **`freehand` (ink) is an engine STUB.** Handler at [line 1867](engine/src/parser/pdfium_document.cpp#L1867) is literally `spdlog::info("Parsed freehand edit operation (stub)")` — it parses and does nothing. | Roadmap says **"Freehand / Ink Annotations — Done 100%"** — that is **false end-to-end**. Ink never persists to the saved PDF. |
|
||||||
|
| **F2** | **`free_text` is an engine STUB.** Handler at [line 1804](engine/src/parser/pdfium_document.cpp#L1804) is `spdlog::info("Parsed free_text edit operation (stub)")`. | The **Text-box tool** (which sends `free_text`) won't persist. (Note: `text_overlay` *is* real — see F3.) |
|
||||||
|
| **F3** | Several items marked **"Done — 0%"** (contradictory) are in fact **REAL and working**: `highlight`, `comment`, Edit-Layer schema. The 0% column is just wrong. | Good news — these are done. |
|
||||||
|
| **F4** | **`redaction`** (roadmap: *In Progress*) is **functionally complete** — it removes overlapping page objects, covers, and regenerates content ([line 1566](engine/src/parser/pdfium_document.cpp#L1566)). Verified: page text dropped 312→101 chars. | Under-reported; it's effectively done. |
|
||||||
|
| **F5** | **Frontend "text selection + copy — Done"** was a **mock** (`SelectionLayer` returned fake text) until it was wired to real glyph data **this session**. | Now real; flagging the roadmap was wrong. |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2. Edit operations — verified truth table
|
||||||
|
|
||||||
|
Tested by applying each op to a real corpus PDF and reloading the result.
|
||||||
|
|
||||||
|
| Edit op | Roadmap | Engine handler | Runtime result | Verdict |
|
||||||
|
|---|---|---|---|---|
|
||||||
|
| `highlight` | Done (0%) | Real (`FPDF_ANNOT_HIGHLIGHT`, quadpoints, color, author) | annots 0→1 | ✅ **REAL** |
|
||||||
|
| `comment` | Done (0%) | Real (`FPDF_ANNOT_TEXT` sticky) | annots 0→1 | ✅ **REAL** |
|
||||||
|
| `text_overlay` | Done (100%) | Real (draws text objects, font mapping) | text 312→321 | ✅ **REAL** |
|
||||||
|
| `image_overlay` | Done (100%) | Real engine **+ gateway** converts data-URI→BGRA→temp file | needs gateway path (Pillow ✓) | ✅ **REAL (via gateway)** |
|
||||||
|
| `redaction` | In Progress | Real content removal + cover + regen | text 312→101 | ✅ **REAL** |
|
||||||
|
| `page_rotation` | Done (100%) | Real (`FPDFPage_SetRotation`) | saved, bytes changed | ✅ **REAL** |
|
||||||
|
| `page_deletion` | Done (100%) | Real (`FPDFPage_Delete`, guards last page) | pages 100→99 | ✅ **REAL** |
|
||||||
|
| `page_reorder` | Done (100%) | Real (`FPDF_MovePages`) | saved ok | ✅ **REAL** |
|
||||||
|
| **`free_text`** | (in schema) | **STUB — no-op** | no change | ❌ **STUB** |
|
||||||
|
| **`freehand` (ink)** | **Done (100%)** | **STUB — no-op** | no change | ❌ **STUB** |
|
||||||
|
|
||||||
|
**8 of 10 edit ops are genuinely real and verified. 2 (`freehand`, `free_text`) are stubs.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3. Why the stubs matter right now (frontend consequence)
|
||||||
|
|
||||||
|
Both stubs are wired into the live UI, so they *look* like they work but silently fail on save:
|
||||||
|
|
||||||
|
- **Draw / Ink tool** → emits `freehand` → engine ignores it. In live (non-mock) mode the stroke renders locally, then **disappears after the save round-trip** (the reloaded annotation list from `extract_annotations` won't contain it). It will **not** be in the exported PDF.
|
||||||
|
- **Text-box tool** → emits `free_text` → same: text vanishes after reload; not in export.
|
||||||
|
|
||||||
|
> These didn't show up earlier because we've been in **mock mode** (engine flag off), where edits don't actually round-trip. They'll surface the moment the engine is enabled.
|
||||||
|
|
||||||
|
**Cheap mitigations (for later, your call):**
|
||||||
|
- Text-box: re-point it from `free_text` → **`text_overlay`** (already real). ~1-line change in the frontend handler.
|
||||||
|
- Ink: needs a real engine `freehand` handler (write a PDF **Ink annotation**, `FPDF_ANNOT_INK`, from the `paths`) — a small, well-scoped C++ addition, *not* a future-phase feature.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 4. Roadmap status, corrected
|
||||||
|
|
||||||
|
### Phase 0 — Infra (8 items, all *Done*)
|
||||||
|
✅ All real and consistent. CMake/vcpkg/CI, PDFium/Skia/FreeType/HarfBuzz builds, FastAPI + React scaffold, WASM hello-world, frozen contracts. No issues.
|
||||||
|
|
||||||
|
### Phase 1 — Core engine + viewer (13 items, all *Done*)
|
||||||
|
Mostly real. Corrections:
|
||||||
|
- **"React: text selection + copy — Done"** → was a **mock** in the live viewer; wired to real glyphs this session. Engine-side glyph extraction was real all along.
|
||||||
|
- `render`, `text extraction w/ bounds`, DPI/coordinate transforms, font load/substitution, thumbnails, incremental save — **verified real**.
|
||||||
|
- **"C++ Engine → WASM Facade — Done"** → `pdfengine.wasm` is built (60 KB) and `wasmLoader` loads it, **but falls back to a JS mock on failure**, and the live app renders **server-side** (`/render`), not via WASM. So the facade exists; it isn't the primary render path.
|
||||||
|
|
||||||
|
### Phase 2 — Editing (status mixed)
|
||||||
|
- **Real & done:** `text_overlay`, `highlight`, `image_overlay`, `comment`, `page_rotation`, `page_deletion`, `page_reorder`, Edit-Layer schema. (Several mislabeled 0%.)
|
||||||
|
- **Marked Done but STUB:** **`freehand`/ink (F1)**. ← the one genuinely wrong "Done".
|
||||||
|
- **Schema-only / STUB:** `free_text` (F2).
|
||||||
|
- **In Progress but effectively real:** `redaction`.
|
||||||
|
- **Genuinely In Progress / Not Started (correctly labeled):**
|
||||||
|
- Annotation reader (read exists via `extract_annotations`; "write/edit existing" not there).
|
||||||
|
- React annotation toolbar/editing UI (built this session, but **edit/delete of existing annots has no engine op**).
|
||||||
|
- Form field viewing — **Not Started** (no engine API).
|
||||||
|
- Export full-save — engine `save_full` works; gateway endpoint exists; treat as **functionally real**.
|
||||||
|
- WASM render path + Web Worker — **In Progress / Not Started** (secondary).
|
||||||
|
|
||||||
|
### Phase 3 — Advanced (16 items, all *Not Started*) ✅ accurate
|
||||||
|
Content-stream analyzer, **edit existing text / replace / reflow**, underline/decoration, font embedding for edits, custom path/Skia rasterizer, **glyph-accurate hit-testing**, **form filling**, encryption/password, regression suite, fuzzing, native SDK packages. **None started — correctly labeled.** This is where "true PDF editing" (retype existing text), forms, and password PDFs live.
|
||||||
|
|
||||||
|
### Phase 4 — Perf/enterprise (gated, *Not Started*) ✅ accurate
|
||||||
|
Custom glyph pipeline, tiling, GPU, reflow, accessibility, **digital (cryptographic) signatures**, advanced forms. Out of v1 scope, correctly labeled.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 5. Are the mocks/stubs "part of upcoming phases"?
|
||||||
|
|
||||||
|
Two different cases — important distinction:
|
||||||
|
|
||||||
|
| Stub | Belongs to a future phase? | Reality |
|
||||||
|
|---|---|---|
|
||||||
|
| `freehand` (ink) | **No.** It's a **Phase 2** item *marked done*. The real impl (write an Ink annotation) is small and overdue, not future work. | Incomplete Phase-2 task mislabeled "Done". |
|
||||||
|
| `free_text` | **Partly.** A *simple* FreeText annotation is small Phase-2-grade work. *True editable/reflowing text* is **Phase 3** (`Text replacement engine`, Not Started). | The basic annotation is a quick win; full text editing is correctly future. |
|
||||||
|
| Edit existing annotations (delete/update) | **Not on any phase.** No edit op exists. | Net-new small backend ticket. |
|
||||||
|
| Outline/bookmarks, forms, password PDFs, edit-existing-text | **Yes — Phase 2/3, Not Started.** | Correctly future; the UI already stubs these as "coming soon". |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 6. Bottom line
|
||||||
|
|
||||||
|
- **Engine completion is genuinely high** for v1-style editing: **8/10 edit ops real and verified**, render/text/search/pages/redaction/export all real.
|
||||||
|
- **The roadmap over-claims exactly two things:** `freehand`/ink is "Done" but a **stub**, and the **"Done 0%"** rows are mislabeled (they're actually real). It also under-claims `redaction`.
|
||||||
|
- **The only end-to-end-broken user-facing features** are **Ink** and **Text-box** (both ride the two stubs). Both have cheap fixes; neither requires Phase 3.
|
||||||
|
- **Everything labeled Phase 3/4 "Not Started" is accurate** — that's the real remaining work (edit existing text, forms, encryption, perf, certified signatures).
|
||||||
|
|
||||||
|
## 7. Suggested next steps (for review — nothing changed yet)
|
||||||
|
1. **Correct the roadmap**: `freehand` = In Progress (stub); `highlight`/`comment`/schema = Done (fix the 0%); `redaction` = Done.
|
||||||
|
2. **Decide the two cheap fixes**: (a) re-point Text-box tool to `text_overlay` now; (b) implement the engine `freehand` → Ink-annotation handler.
|
||||||
|
3. **Add two small backend tickets** already flagged by the UI: `delete/update_annotation` op, and an outline endpoint.
|
||||||
|
4. Leave Phase 3/4 as-is — correctly scoped future work.
|
||||||
@@ -1219,20 +1219,24 @@ std::expected<std::vector<PdfPage::AnnotationInfo>, EngineError> PdfiumPage::ext
|
|||||||
info.color = hex;
|
info.color = hex;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ink paths
|
// Ink geometry: read the /InkList strokes so the frontend can redraw
|
||||||
|
// them as an interactive overlay (Adobe-style, non-destructive).
|
||||||
|
// Points are converted from PDF (bottom-up) to top-left page-point space
|
||||||
|
// to match the frontend's coordinate convention (same as the rect above).
|
||||||
if (subtype == FPDF_ANNOT_INK) {
|
if (subtype == FPDF_ANNOT_INK) {
|
||||||
int objCount = FPDFAnnot_GetObjectCount(annot);
|
const double pageH = height();
|
||||||
for (int j = 0; j < objCount; ++j) {
|
unsigned long strokeCount = FPDFAnnot_GetInkListCount(annot);
|
||||||
FPDF_PAGEOBJECT obj = FPDFAnnot_GetObject(annot, j);
|
for (unsigned long s = 0; s < strokeCount; ++s) {
|
||||||
if (obj && FPDFPageObj_GetType(obj) == FPDF_PAGEOBJ_PATH) {
|
unsigned long ptCount = FPDFAnnot_GetInkListPath(annot, s, nullptr, 0);
|
||||||
[[maybe_unused]] int pathCount = FPDFPath_CountSegments(obj); // PDFium has FPDFPath_CountSegments ? Wait, let me check pdfium headers. Actually FPDFPath_GetPathSegmentCount doesn't exist, it is FPDFPath_CountSegments probably, or FPDFPath_CountSegments / FPDFPath_GetPathSegment.
|
if (ptCount == 0) continue;
|
||||||
// Wait, looking at PDFium fpdf_edit.h: `FPDFPath_CountSegments` doesn't exist, it's `FPDFPath_CountSegments`?
|
std::vector<FS_POINTF> pts(ptCount);
|
||||||
// Let me check if I can use FPDFPath_CountSegments
|
FPDFAnnot_GetInkListPath(annot, s, pts.data(), ptCount);
|
||||||
// It is usually int FPDFPath_CountSegments(FPDF_PAGEOBJECT path);
|
std::vector<Point2D> stroke;
|
||||||
// FPDF_PATHSEGMENT FPDFPath_GetPathSegment(FPDF_PAGEOBJECT path, int index);
|
stroke.reserve(ptCount);
|
||||||
// FPDFPathSegment_GetPoint(FPDF_PATHSEGMENT segment, float* x, float* y);
|
for (const auto& p : pts) {
|
||||||
// int FPDFPathSegment_GetType(FPDF_PATHSEGMENT segment);
|
stroke.push_back(Point2D{static_cast<double>(p.x), pageH - static_cast<double>(p.y)});
|
||||||
}
|
}
|
||||||
|
if (!stroke.empty()) info.paths.push_back(std::move(stroke));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1864,7 +1868,71 @@ std::expected<void, EngineError> PdfiumDocument::applyEdits(const std::string& e
|
|||||||
FPDFPage_CloseAnnot(annot);
|
FPDFPage_CloseAnnot(annot);
|
||||||
FPDF_ClosePage(page);
|
FPDF_ClosePage(page);
|
||||||
} else if (type == "freehand") {
|
} else if (type == "freehand") {
|
||||||
spdlog::info("Parsed freehand edit operation (stub)");
|
if (!op.contains("data") || !op["data"].is_object()) {
|
||||||
|
spdlog::error("freehand operation missing 'data' object");
|
||||||
|
return std::unexpected(EngineError::InvalidFormat);
|
||||||
|
}
|
||||||
|
auto data = op["data"];
|
||||||
|
|
||||||
|
FPDF_PAGE page = FPDF_LoadPage(doc_, pageIndex);
|
||||||
|
if (!page) {
|
||||||
|
spdlog::error("Failed to load page index {} for freehand", pageIndex);
|
||||||
|
return std::unexpected(EngineError::Unknown);
|
||||||
|
}
|
||||||
|
|
||||||
|
FPDF_ANNOTATION annot = FPDFPage_CreateAnnot(page, FPDF_ANNOT_INK);
|
||||||
|
if (!annot) {
|
||||||
|
spdlog::error("Failed to create ink annotation");
|
||||||
|
FPDF_ClosePage(page);
|
||||||
|
return std::unexpected(EngineError::Unknown);
|
||||||
|
}
|
||||||
|
|
||||||
|
double pageHeight = FPDF_GetPageHeightF(page);
|
||||||
|
|
||||||
|
std::string colorStr = data.value("color", "#000000");
|
||||||
|
unsigned int r = 0, g = 0, b = 0;
|
||||||
|
parseHexColor(colorStr, r, g, b);
|
||||||
|
FPDFAnnot_SetColor(annot, FPDFANNOT_COLORTYPE_Color, r, g, b, 255);
|
||||||
|
|
||||||
|
float thickness = static_cast<float>(data.value("thickness", 2.0));
|
||||||
|
FPDFAnnot_SetBorder(annot, 0.0f, 0.0f, thickness);
|
||||||
|
|
||||||
|
// Accumulate bounding box in PDF (bottom-up) space.
|
||||||
|
float minX = 1e9f, minY = 1e9f, maxX = -1e9f, maxY = -1e9f;
|
||||||
|
bool anyPoints = false;
|
||||||
|
|
||||||
|
if (data.contains("paths") && data["paths"].is_array()) {
|
||||||
|
for (const auto& path : data["paths"]) {
|
||||||
|
if (!path.is_array() || path.size() < 2) continue;
|
||||||
|
std::vector<FS_POINTF> pts;
|
||||||
|
pts.reserve(path.size());
|
||||||
|
for (const auto& pt : path) {
|
||||||
|
float px = static_cast<float>(pt.value("x", 0.0));
|
||||||
|
// Frontend stores ink points top-down in page points; flip to PDF bottom-up.
|
||||||
|
float py = static_cast<float>(pageHeight - pt.value("y", 0.0));
|
||||||
|
pts.push_back(FS_POINTF{px, py});
|
||||||
|
anyPoints = true;
|
||||||
|
minX = (std::min)(minX, px); maxX = (std::max)(maxX, px);
|
||||||
|
minY = (std::min)(minY, py); maxY = (std::max)(maxY, py);
|
||||||
|
}
|
||||||
|
if (pts.size() >= 2) {
|
||||||
|
FPDFAnnot_AddInkStroke(annot, pts.data(), pts.size());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (anyPoints) {
|
||||||
|
float pad = thickness + 1.0f;
|
||||||
|
FS_RECTF rect;
|
||||||
|
rect.left = minX - pad;
|
||||||
|
rect.bottom = minY - pad;
|
||||||
|
rect.right = maxX + pad;
|
||||||
|
rect.top = maxY + pad;
|
||||||
|
FPDFAnnot_SetRect(annot, &rect);
|
||||||
|
}
|
||||||
|
|
||||||
|
FPDFPage_CloseAnnot(annot);
|
||||||
|
FPDF_ClosePage(page);
|
||||||
} else if (type == "page_rotation") {
|
} else if (type == "page_rotation") {
|
||||||
if (!op.contains("data") || !op["data"].is_object()) {
|
if (!op.contains("data") || !op["data"].is_object()) {
|
||||||
spdlog::error("page_rotation operation missing 'data' object");
|
spdlog::error("page_rotation operation missing 'data' object");
|
||||||
|
|||||||
@@ -136,6 +136,8 @@ function App() {
|
|||||||
content: a.content,
|
content: a.content,
|
||||||
timestamp: a.timestamp,
|
timestamp: a.timestamp,
|
||||||
pageIndex: a.pageIndex,
|
pageIndex: a.pageIndex,
|
||||||
|
// Ink stroke geometry (top-left page points) so the overlay can redraw it interactively.
|
||||||
|
paths: Array.isArray(a.paths) && a.paths.length > 0 ? a.paths : undefined,
|
||||||
})));
|
})));
|
||||||
if (preservePageRef.current) preservePageRef.current = false;
|
if (preservePageRef.current) preservePageRef.current = false;
|
||||||
else setCurrentPage(0);
|
else setCurrentPage(0);
|
||||||
@@ -253,8 +255,8 @@ function App() {
|
|||||||
const handlePlaceText = (pageIndex: number, rectPts: Rect, text: string) => {
|
const handlePlaceText = (pageIndex: number, rectPts: Rect, text: string) => {
|
||||||
const pdf = viewportRectToPdf(rectPts, 1, pageHeightPts(pageIndex));
|
const pdf = viewportRectToPdf(rectPts, 1, pageHeightPts(pageIndex));
|
||||||
applyOps([{
|
applyOps([{
|
||||||
id: rid('txt'), type: 'free_text', pageIndex,
|
id: rid('txt'), type: 'text_overlay', pageIndex,
|
||||||
data: { x: pdf.x, y: pdf.y, width: pdf.width, height: pdf.height, text, fontSize: toolSettings.fontSize, color: toolSettings.textColor },
|
data: { x: pdf.x, y: pdf.y, width: pdf.width, height: pdf.height, text, fontSize: toolSettings.fontSize, fontFamily: 'Helvetica', color: toolSettings.textColor },
|
||||||
}], 'Text box added');
|
}], 'Text box added');
|
||||||
setActiveTool('select');
|
setActiveTool('select');
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -470,6 +470,9 @@ class AnnotationResponse(BaseModel):
|
|||||||
content: str
|
content: str
|
||||||
timestamp: str | None = None
|
timestamp: str | None = None
|
||||||
pageIndex: int
|
pageIndex: int
|
||||||
|
# Stroke geometry for ink annotations (top-left page-point space), so the
|
||||||
|
# frontend can redraw them as an interactive overlay rather than a flat image.
|
||||||
|
paths: list[list[dict[str, float]]] = []
|
||||||
|
|
||||||
@router.get("/{document_id}/annotations", response_model=list[AnnotationResponse])
|
@router.get("/{document_id}/annotations", response_model=list[AnnotationResponse])
|
||||||
def get_document_annotations(document_id: str) -> list[AnnotationResponse]:
|
def get_document_annotations(document_id: str) -> list[AnnotationResponse]:
|
||||||
@@ -502,7 +505,8 @@ def get_document_annotations(document_id: str) -> list[AnnotationResponse]:
|
|||||||
author=a.author,
|
author=a.author,
|
||||||
content=a.content,
|
content=a.content,
|
||||||
timestamp=getattr(a, "timestamp", None),
|
timestamp=getattr(a, "timestamp", None),
|
||||||
pageIndex=a.page_index
|
pageIndex=a.page_index,
|
||||||
|
paths=[[{"x": p.x, "y": p.y} for p in stroke] for stroke in getattr(a, "paths", [])],
|
||||||
))
|
))
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|||||||
Reference in New Issue
Block a user