Skip to content

Read content-stream operators through PdfOperatorVariant instead of PdfOperator - #589

Merged
Velli20 merged 1 commit into
mainfrom
remove-pdf-operator-trait
Oct 20, 2026
Merged

Velli20 merged 1 commit into
mainfrom
remove-pdf-operator-trait

Conversation

@Velli20

@Velli20 Velli20 commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Summary

Removes the PdfOperator trait and most of the per-operator structs. Each operator becomes one arm in PdfOperatorVariant::read and one in PdfOperatorVariant::call.

1. Remove PdfOperator (dc8bdf5)

The trait was closed and only used to fill the operation_map dispatch table:

  • NAME was never read. It repeated the table's literals, which is how F ended up unmapped.
  • SC/sc needed placeholder types whose call could never run.
  • BI needed a read that always failed, plus a parse-hook field used by no other operator.

PdfOperatorVariant::read(name, operands) now matches (name, operand count) in one place, and the stream parser handles BI itself. Recovery behaviour is unchanged. Without the per-operator function pointer, dispatch-heavy parsing is about 17% faster.

2. Inline operator operands (05b3fda)

  • Operators that only held decoded operands are now struct variants (e.g. LineTo { x, y }). Each is read in one expression and dispatched in one call arm. Their structs, new/read/call methods and eleven emptied modules are deleted.
  • LineCap, LineJoin and TextRenderingMode gain FromPdfObject decoders in pdf-object-reader, with a new ObjectError::InvalidEnumValue. The J, j and Tr operators read them with operands.at(0)?. ExtGState uses the same decoders for /LC and /LJ, replacing its own validation and two error variants.
  • The stroking and non-stroking colour operators share one ColorOperands { components, pattern } type, replacing two identical structs and four duplicated readers.
  • Only BeginMarkedContentWithProps keeps a struct.

About 1,810 lines removed and 755 added compared with main.

Performance

operator_parsing, median of 5 interleaved runs, shipped release profile:

Benchmark main dc8bdf5 Change
dispatch_heavy 462.9 µs 381.7 µs −17.5% (5/5 faster)
numeric_heavy 1252.5 µs 1243.4 µs −0.7% (4/5 faster)
mixed 2224.4 µs 2230.5 µs +0.3% (noise)

Commit 2 compared with dc8bdf5:

Benchmark dc8bdf5 05b3fda Change
numeric_heavy 1266.0 µs 1271.7 µs +0.5%
dispatch_heavy 387.0 µs 393.3 µs +1.6%
mixed 2272.5 µs 2272.7 µs 0.0%

Three builds of dc8bdf5 with a no-op change set the noise floor at ±0.7% for dispatch_heavy and ±1.3% for the other two.

The parse loop is inlined into one function, and at opt-level = "z" its layout shifts whenever code elsewhere in it changes, even in arms a benchmark never runs. Merging the colour readers left them with two callers, so LLVM stopped inlining them. That reshaped the loop and slowed every operator by 2.4%. #[inline] on the ColorOperands readers brings this back to the +1.6% above. dispatch_heavy only parses zero-operand operators, none of whose arms changed, and the remaining cost is close to the noise floor. Other attempts (out-of-line read, direct pushes, different decoder inlining) were all slower.

Testing

  • Passed locally: cargo test, cargo clippy --all --workspace -- -D warnings, cargo fmt --check, and a wasm check of web-canvas-example.
  • Not run locally: pdf-graphics-skia and pdf-conformance, because their Skia build needs a download the local sandbox blocks. CI covers them.
  • Not covered by a test: reading /LC or /LJ from an ExtGState dictionary.

Follow-ups

🤖 Generated with Claude Code

Most operator structs only held decoded operands, assigned them in new,
and forwarded them to one backend method. They become struct variants of
PdfOperatorVariant, read in one expression in PdfOperatorVariant::read
and dispatched in PdfOperatorVariant::call.

Line cap, line join and text rendering mode are read through new
FromPdfObject decoders for the pdf-graphics enums, which ExtGState now
uses for /LC and /LJ in place of its own validation. The stroking and
non-stroking color operators share one ColorOperands type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Conformance reads

Documents that Safe-PDF reads differently from the base commit, compared with PDFium. A read
regresses when Safe-PDF fails to open a document it opened before, crashes or times out, or
counts a different number of pages than PDFium.

pdf.js corpus

1009 documents compared with f1dcf15: 1 read worse, 0 read better.

Regressions

Document Base This pull request
issue17190_1 read, 1 page timeout: time limit exceeded (stage safe-read)

840 documents of the PDFium corpus read as on f1dcf15.

Details are in the job summaries of the workflow run. Rerun a case locally with
cargo conformance run --corpus <corpus> --read-only --case <id>.

@Velli20
Velli20 merged commit c8353d7 into main Oct 20, 2026
10 checks passed
@Velli20
Velli20 deleted the remove-pdf-operator-trait branch October 20, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant