Repository navigation
feat(ffi): add rofd_document_get_page_size_mm - #11
Merged
Merged
Conversation
Expose rofd-core's metadata-only page-size query through the C ABI, so consumers that need every page's physical box — a document viewer filling its page-size cache, for example — do not have to parse and cache each page's content. The call returns the same value as rofd_page_get_size_mm for the same index. It performs no content validation, collects no content warnings, and also succeeds for a page whose content cannot be loaded. - include/rofd.h: declare the entry point and its output transaction contract - src/document.rs: implement it over Document::page_size via the shared boundary - src/lib.rs: re-export the symbol - tests/document.rs: value parity with the page handle, metadata-only behaviour on a page with an unresolvable font resource, and argument/index/alias rejection - tests/c/expected-symbols.txt: extend the pinned export list - README.md and the v1 design doc: record the new entry point Verification: - cargo fmt --all -- --check - cargo clippy -p rofd-core -p rofd-render -p rofd-ffi --all-targets -- -D warnings - cargo test -p rofd-ffi - crates/rofd-ffi/tests/run_c_tests.sh
|
Hi @YangtseSu. Thanks for your PR. 😃 |
Reviewer's GuideAdds an additive C ABI entry point that retrieves a page’s physical size from document metadata without loading page content, with shared transactional FFI validation, updated API documentation and symbol coverage, and tests for parity, malformed content, and boundary conditions. Sequence diagram for metadata-only page size querysequenceDiagram
participant C as C Consumer
participant FFI as rofd_document_get_page_size_mm
participant Boundary as boundary
participant Document as Document::page_size
C->>FFI: rofd_document_get_page_size_mm(document, page_index, page_size_mm, error)
FFI->>Boundary: validate outputs and transaction rules
Boundary->>Document: page_size(page_index)
Document-->>Boundary: physical page box metadata
Boundary-->>FFI: rofd_rect_t
FFI-->>C: ROFD_STATUS_OK and page_size_mm
Flow diagram for metadata-only page size behaviorflowchart TD
A["Query page size from document"] --> B{"Valid document and outputs?"}
B -->|No| C["Return error without partial writes"]
B -->|Yes| D{"page_index in range?"}
D -->|No| E["Return ROFD_STATUS_PAGE_OUT_OF_RANGE"]
D -->|Yes| F["Read page area metadata"]
F --> G["Copy physical box to rofd_rect_t"]
G --> H["Return ROFD_STATUS_OK"]
F -.-> I["No page content loading or validation"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @YangtseSu. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/rofd-ffi/src/document.rs" line_range="160" />
<code_context>
+ boundary(error, ScalarOutput::required(page_size_mm), || {
+ let size = document_ref(document)?
+ .inner
+ .page_size(page_index)
+ .map_err(FfiError::from)?;
+ Ok(rofd_rect_t {
</code_context>
<issue_to_address>
**Page sizes fail on unreadable content**
When the page has a readable physical box but its XML is malformed, malformed within its content subtree, or exceeds the configured XML depth limit, `Document::page_size` parses or deserializes the page XML before extracting its area, so malformed content or a depth-limit violation returns an invalid-document error instead of the documented content-independent size result.
Read the page's physical box without parsing or deserializing its XML.
Also at `crates/rofd-ffi/README.md:94-99`, `crates/rofd-ffi/include/rofd.h:343-349`, `crates/rofd-ffi/src/document.rs:137-140`.
</issue_to_address>
### Comment 2
<location path="crates/rofd-ffi/src/document.rs" line_range="160" />
<code_context>
+ boundary(error, ScalarOutput::required(page_size_mm), || {
+ let size = document_ref(document)?
+ .inner
+ .page_size(page_index)
+ .map_err(FfiError::from)?;
+ Ok(rofd_rect_t {
</code_context>
<issue_to_address>
**Malformed page sizes disagree**
When a page's physical box has a nonpositive width or height, `Document::page_size` rejects nonpositive dimensions, while `Document::page` stores the parsed rectangle and `Page::size` returns it unchanged; this call therefore returns `ROFD_STATUS_INVALID_DOCUMENT` where `rofd_page_get_size_mm` returns a rectangle, breaking the documented parity.
Make both page-size APIs apply the same validity rules, or document and test the malformed-page exception to the parity guarantee.
</issue_to_address>| boundary(error, ScalarOutput::required(page_size_mm), || { | ||
| let size = document_ref(document)? | ||
| .inner | ||
| .page_size(page_index) |
There was a problem hiding this comment.
🟡 Medium · Page sizes fail on unreadable content
When the page has a readable physical box but its XML is malformed, malformed within its content subtree, or exceeds the configured XML depth limit, Document::page_size parses or deserializes the page XML before extracting its area, so malformed content or a depth-limit violation returns an invalid-document error instead of the documented content-independent size result.
Read the page's physical box without parsing or deserializing its XML.
Also at crates/rofd-ffi/README.md:94-99, crates/rofd-ffi/include/rofd.h:343-349, crates/rofd-ffi/src/document.rs:137-140.
Prompt for AI agents
In `crates/rofd-ffi/src/document.rs` at line 160:
**Page sizes fail on unreadable content**
When the page has a readable physical box but its XML is malformed, malformed within its content subtree, or exceeds the configured XML depth limit, `Document::page_size` parses or deserializes the page XML before extracting its area, so malformed content or a depth-limit violation returns an invalid-document error instead of the documented content-independent size result.
Read the page's physical box without parsing or deserializing its XML.
Also at `crates/rofd-ffi/README.md:94-99`, `crates/rofd-ffi/include/rofd.h:343-349`, `crates/rofd-ffi/src/document.rs:137-140`.| boundary(error, ScalarOutput::required(page_size_mm), || { | ||
| let size = document_ref(document)? | ||
| .inner | ||
| .page_size(page_index) |
There was a problem hiding this comment.
🟡 Medium · Malformed page sizes disagree
When a page's physical box has a nonpositive width or height, Document::page_size rejects nonpositive dimensions, while Document::page stores the parsed rectangle and Page::size returns it unchanged; this call therefore returns ROFD_STATUS_INVALID_DOCUMENT where rofd_page_get_size_mm returns a rectangle, breaking the documented parity.
Make both page-size APIs apply the same validity rules, or document and test the malformed-page exception to the parity guarantee.
Prompt for AI agents
In `crates/rofd-ffi/src/document.rs` at line 160:
**Malformed page sizes disagree**
When a page's physical box has a nonpositive width or height, `Document::page_size` rejects nonpositive dimensions, while `Document::page` stores the parsed rectangle and `Page::size` returns it unchanged; this call therefore returns `ROFD_STATUS_INVALID_DOCUMENT` where `rofd_page_get_size_mm` returns a rectangle, breaking the documented parity.
Make both page-size APIs apply the same validity rules, or document and test the malformed-page exception to the parity guarantee.
archlinux-github
pushed a commit
to archlinux/aur
that referenced
this pull request
Oct 14, 2026
Carry the metadata-only page-size query (rofd_document_get_page_size_mm) from upstream pull request linuxdeepin/rofd#11, so consumers that need every page's physical box do not have to parse all page content. - source: add rofd-ffi-page-size.patch (sha256 pinned), applied in prepare() - drop the patch source and that line once a rofd release contains the function
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA. |
lzwind
approved these changes
Oct 19, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, YangtseSu The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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
3 participants
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.
Motivation
rofd-corealready has a metadata-only page-size query (Document::page_size— "Reads the physical page box without loading or caching graphical objects"),
but the C ABI only exposes
rofd_page_get_size_mm, which needs an opened pageand therefore a full page parse.
Consumers that need every page's physical box pay a full document parse for it.
Papers' backend interface asks a document for the size of every page right
after load (its
setup_cachestep), so throughrofd-ffithat is thedifference between a fast open and parsing all page content up front — for a
several-hundred-page document, most of the open latency.
Change
Adds one entry point:
rofd_page_get_size_mmfor the same index;succeeds for a page whose content cannot be loaded;
[0, page_count)returnsROFD_STATUS_PAGE_OUT_OF_RANGE;Verification
cargo test -p rofd-ffi— three new tests intests/document.rs:rofd_page_get_size_mmon the repository fixture;references a font resource that does not exist: the size query succeeds while
rofd_document_get_pagefails withROFD_STATUS_INVALID_DOCUMENT;the error slot are all rejected transactionally, without partial writes.
crates/rofd-ffi/tests/run_c_tests.sh— export list (tests/c/expected-symbols.txtextended by one line), C/C++ header compile and smoke tests.
cargo fmt --all -- --checkandcargo clippy -p rofd-core -p rofd-render -p rofd-ffi --all-targets -- -D warnings.Summary by Sourcery
Expose metadata-only page size queries through the FFI so callers can inspect all page dimensions without parsing page content.
New Features:
Enhancements:
Documentation:
Tests: