Skip to content

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

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. Operators are now read through a single PdfOperatorVariant::read(name, operands) match.

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.

Changes

  • PdfOperatorVariant::read matches (name, operand count) in one place. A wrong operand count is dropped exactly as before.
  • Operators with operands keep their structs, now with an inherent read that returns Self and a call method.
  • The 20 zero-operand operators (q, Q, BT, S, f, W, …) become fieldless variants dispatched directly in PdfOperatorVariant::call. Their four files are removed.
  • SC/sc read through read_without_pattern on the two color structs.
  • The stream parser handles BI itself. Recovery behaviour is unchanged.
  • Deleted operator_trait.rs and operation_map.rs.

About 1,320 lines removed and 670 added.

Performance

operator_parsing, median of 5 interleaved runs against a git archive export of main, shipped release profile:

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

Testing

  • Passed locally: cargo test, cargo clippy --all --workspace, 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.

🤖 Generated with Claude Code

…dfOperator

The PdfOperator trait was a closed trait used only to fill the
operation-map dispatch table. Its NAME constant was never read and
repeated the table's literals, the SC/sc operators needed placeholder
types with unreachable call bodies, and inline images needed an
always-failing read plus a parse-hook field.

PdfOperatorVariant::read now matches the operator name and operand
count in one place and calls each operator's inherent read, which
returns the finished operator. Zero-operand operators become fieldless
variants dispatched directly in PdfOperatorVariant::call. The stream
parser handles BI itself. Recovery behaviour is unchanged.

Dispatch no longer goes through a function pointer, which speeds up
dispatch-heavy operator parsing by about 17%.

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 1d8cccb: 0 read worse, 2 read better.

Improvements

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

840 documents of the PDFium corpus read as on 1d8cccb.

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 f1dcf15 into main Oct 20, 2026
10 checks passed
@Velli20
Velli20 deleted the remove-pdf-operator-trait branch October 20, 2026 18:45
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