Skip to content

[pdfx] Align default image format with native implementation (JPEG → PNG) - #581

Open
ktanaka101 wants to merge 3 commits into
ScerIO:mainfrom
ktanaka101:fix-set-default-image-format
Open

[pdfx] Align default image format with native implementation (JPEG → PNG)#581
ktanaka101 wants to merge 3 commits into
ScerIO:mainfrom
ktanaka101:fix-set-default-image-format

Conversation

@ktanaka101

Copy link
Copy Markdown

Description

What & Why

Both the public docs and the native Android / iOS layers say
PdfPage.render() defaults to PNG when format is omitted.
The Dart signature, however, still defaulted to PdfPageImageFormat.jpeg.

// produces PNG bytes even though the API claims JPEG
final img = await page.render(format: PdfPageImageFormat.jpeg);

That mismatch triggers a false-positive from the
avoid_redundant_argument_values
linter: it tells you to drop the argument, but doing so would silently
change the result from JPEG → PNG.

This PR aligns the Dart default with reality:

- PdfPageImageFormat format = PdfPageImageFormat.jpeg,
+ PdfPageImageFormat format = PdfPageImageFormat.png,

Tests

Added test/pdfx_test.dart.

State Result Meaning
Pre-patch ✅ Pass Reveals that PNG was already returned at runtime.
Post-patch ✅ Pass API, docs and native behaviour are now consistent.

Impact

  • No breaking change – callers already received PNG bytes by default.
  • Eliminates the misleading linter warning, improving DX.

Thanks for reviewing!

ktanaka101 and others added 2 commits May 5, 2025 18:43
The public docs—and the native Android / iOS layers—treat PNG as the
default output format when `format` is omitted, but the Dart signature
still defaulted to `PdfPageImageFormat.jpeg`.
@vicajilau

Copy link
Copy Markdown
Member

Thanks for the PR and the detailed writeup. Digging into the native implementations before merging, I want to flag something: this isn't actually a no-op default change.

Both Android (CompressFormats.kt) and iOS (Document.swift) genuinely branch on the format value and produce real JPEG vs PNG compressed bytes, so a caller that omits format today gets real lossy JPEG output, not PNG. The mocked-channel test added here only proves the Dart-side default argument changed, it can't observe the native encoding, so it doesn't actually demonstrate that pre-patch output was already PNG.

So switching the default to PNG is a genuine behavior change for anyone relying on the implicit default: larger output, lossless, alpha-preserving instead of JPEG's lossy compression. That's likely fine, arguably PNG is the better default given format aligns with the docs, but we'd want to merge it as an explicit behavior change with its own CHANGELOG entry, not as a docs/lint alignment fix.

Can you confirm you still want PNG as the new default with that understanding? If so we'll merge and call it out clearly in the release notes.

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.

2 participants