[rust png] Support using Rust-based PNG codec in XFA implementation.
After this CL, if `pdf_use_skia && pdf_enable_rust_png` then the
Rust-based PNG codec will be used in XFA implementation.
Bug: 444045690
Change-Id: Ia6ec12d0335b1fa74c278acc94973e97aaa30b60
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/135730
Commit-Queue: Łukasz Anforowicz <lukasza@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/build_overrides/pdfium.gni b/build_overrides/pdfium.gni
index 3870d39..f0cf35a 100644
--- a/build_overrides/pdfium.gni
+++ b/build_overrides/pdfium.gni
@@ -42,6 +42,4 @@
# Build PDFium with Rust-based PNG decoder and encoder.
# If enabled, it ensures that PDFium doesn't depend on `libpng`.
# Note that `pdf_enable_rust_png` requires Skia and Rust support.
-#
-# TODO(https://crbug.com/444045690): Cover decoding path in production code.
pdf_enable_rust_png_override = false
diff --git a/core/fxcodec/BUILD.gn b/core/fxcodec/BUILD.gn
index c5ecba0..f6930a1 100644
--- a/core/fxcodec/BUILD.gn
+++ b/core/fxcodec/BUILD.gn
@@ -81,6 +81,7 @@
"../fxge",
"//third_party:jpeg",
]
+ defines = []
if (pdf_enable_xfa) {
sources += [
"cfx_codec_memory.cpp",
@@ -115,12 +116,25 @@
]
}
if (pdf_enable_xfa_png) {
- sources += [
- "png/libpng_png_decoder.cpp",
- "png/libpng_png_decoder.h",
- "png/png_decoder_delegate.h",
- ]
- deps += [ "../../third_party:png" ]
+ sources += [ "png/png_decoder_delegate.h" ]
+
+ # TODO(https://crbug.com/444045690): Remove `pdf_enable_rust_png` from the
+ # condition below once this build mode has been tested and stabilized.
+ # (Chromium already sets `pdf_use_skia_override = true` so having an extra
+ # condition avoids affecting the Chromium behavior.)
+ if (pdf_use_skia && pdf_enable_rust_png) {
+ sources += [
+ "png/skia_png_decoder.cpp",
+ "png/skia_png_decoder.h",
+ ]
+ deps += [ "//skia" ]
+ } else {
+ sources += [
+ "png/libpng_png_decoder.cpp",
+ "png/libpng_png_decoder.h",
+ ]
+ deps += [ "../../third_party:png" ]
+ }
}
if (pdf_enable_xfa_tiff) {
sources += [
diff --git a/core/fxcodec/png/DEPS b/core/fxcodec/png/DEPS
index 9caa4d8..a3d387b 100644
--- a/core/fxcodec/png/DEPS
+++ b/core/fxcodec/png/DEPS
@@ -1,3 +1,8 @@
-include_rules = [
- '+third_party/libpng/png.h',
-]
+specific_include_rules = {
+ "libpng_png_decoder\.cpp": [
+ '+third_party/libpng/png.h',
+ ],
+ "skia_png_decoder\.cpp": [
+ '+third_party/skia/include',
+ ],
+}
diff --git a/core/fxcodec/png/libpng_png_decoder.cpp b/core/fxcodec/png/libpng_png_decoder.cpp
index dc14179..383456f 100644
--- a/core/fxcodec/png/libpng_png_decoder.cpp
+++ b/core/fxcodec/png/libpng_png_decoder.cpp
@@ -24,6 +24,10 @@
#include "third_party/libpng/png.h"
#endif
+#ifdef PDF_ENABLE_RUST_PNG
+#error "If Rust PNG is enabled, then `libpng` should not be used."
+#endif
+
#define PNG_ERROR_SIZE 256
using PngDecoderDelegate = fxcodec::PngDecoderDelegate;
diff --git a/core/fxcodec/png/png_decoder_delegate.h b/core/fxcodec/png/png_decoder_delegate.h
index d41395c..aa01a9c 100644
--- a/core/fxcodec/png/png_decoder_delegate.h
+++ b/core/fxcodec/png/png_decoder_delegate.h
@@ -11,12 +11,7 @@
namespace fxcodec {
-// Abstract interface used by the `libpng`-based decoder from `png_decoder.h`
-// (in the future by Skia-based decoder from `skia_png_decoder.h`).
-//
-// TODO(https://crbug.com/444045690): Update the comment above once the
-// Skia-based decoder is implemented and/or the `libpng`-based decoder moved
-// to renamed .h/.cc files.
+// Abstract interface used by `libpng_png_decoder.h` and `skia_png_decoder.h`.
class PngDecoderDelegate {
public:
// Called by `PngDecoder` after decoding the image header with
@@ -32,13 +27,18 @@
double* gamma) = 0;
// Called by `PngDecoder` to ask where to write decoded BGRA/8 pixels.
- // Implementation should return a span to the buffer where the
- // decoded pixels should be written to.
+ // Implementation should return a span to the buffer where decoder can write
+ // decoded pixels from the given `line`.
//
// `PngDecoder` guarantees that `0 <= line && line < height`
// (`height` that was earlier passed to `PngReadHeader`).
virtual pdfium::span<uint8_t> PngAskScanlineBuf(int line) = 0;
+ // Called by `PngDecoder` to ask where to write decoded BGRA/8 pixels.
+ // Implementation should return a span to the buffer where decoder can write
+ // all the decoded pixels (i.e. covering all rows/lines of the image).
+ virtual pdfium::span<uint8_t> PngAskImageBuf() = 0;
+
// Called by `PngDecoder` to communicate that all pixels have been decoded.
virtual void PngFinishedDecoding() = 0;
};
diff --git a/core/fxcodec/png/skia_png_decoder.cpp b/core/fxcodec/png/skia_png_decoder.cpp
new file mode 100644
index 0000000..6a13fd6
--- /dev/null
+++ b/core/fxcodec/png/skia_png_decoder.cpp
@@ -0,0 +1,254 @@
+// Copyright 2025 The PDFium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxcodec/png/skia_png_decoder.h"
+
+#include <utility>
+
+#include "core/fxcodec/cfx_codec_memory.h"
+#include "core/fxcodec/png/png_decoder_delegate.h"
+#include "core/fxcodec/progressive_decoder_context.h"
+#include "core/fxcrt/check.h"
+#include "core/fxcrt/check_op.h"
+#include "core/fxcrt/compiler_specific.h"
+#include "core/fxcrt/fx_safe_types.h"
+#include "core/fxcrt/notreached.h"
+#include "core/fxcrt/span.h"
+#include "core/fxcrt/unowned_ptr.h"
+#include "third_party/skia/include/codec/SkCodec.h"
+#include "third_party/skia/include/core/SkStream.h"
+
+#ifdef PDF_ENABLE_RUST_PNG
+#include "third_party/skia/include/codec/SkPngRustDecoder.h"
+#else
+#include "third_party/skia/include/codec/SkPngDecoder.h"
+#endif
+
+namespace fxcodec {
+
+namespace {
+
+sk_sp<SkColorSpace> GetTargetColorSpace(double gamma) {
+ skcms_ICCProfile profile;
+ skcms_Init(&profile);
+
+ skcms_TransferFunction fn;
+ fn.a = 1.0f;
+ fn.b = fn.c = fn.d = fn.e = fn.f = 0.0f;
+ fn.g = 1.0f / gamma;
+ skcms_SetTransferFunction(&profile, &fn);
+
+ skcms_Matrix3x3 to_xyzd50 = skcms_sRGB_profile()->toXYZD50;
+ skcms_SetXYZD50(&profile, &to_xyzd50);
+
+ return SkColorSpace::Make(profile);
+}
+
+// Implements/exposes `SkStream` API on top of `CFX_CodecMemory`.
+//
+// Note that the exposed API does *not* support seeking/rewinding, because this
+// layer only has access to the `CFX_CodecMemory` pushed into `ContinueDecode`
+// and can't reach for `IFX_SeekableReadStream` from which the bytes have been
+// pulled into `CFX_CodecMemory`. This lack of support for seeking/rewinding
+// requires avoiding calling any Skia APIs that may necessitate this
+// functionality later (e.g. calling `SkCodec::getFrameCount` may attempt to
+// read metadata of subsequent frames). This is not an issue today, because
+// PDFium only needs to decode the first (or only) image / animation frame.
+class CodecMemoryStream final : public SkStream {
+ public:
+ explicit CodecMemoryStream(RetainPtr<CFX_CodecMemory> codec_memory)
+ : codec_memory_(std::move(codec_memory)) {}
+
+ size_t read(void* buffer, size_t size) override {
+ // SAFETY: Relying on the caller to pass correct `buffer` and `size`.
+ uint8_t* bytes = static_cast<uint8_t*>(buffer);
+ auto byte_span = UNSAFE_BUFFERS(pdfium::span(bytes, size));
+
+ return codec_memory_->ReadBlock(byte_span);
+ }
+
+ bool isAtEnd() const override { return codec_memory_->IsEOF(); }
+
+ private:
+ RetainPtr<CFX_CodecMemory> const codec_memory_;
+};
+
+class SkiaPngContext final : public ProgressiveDecoderContext {
+ public:
+ // Caller needs to guarantee that `delegate` lives longer than
+ // `SkiaPngContext`.
+ explicit SkiaPngContext(PngDecoderDelegate* delegate) : delegate_(delegate) {}
+ ~SkiaPngContext() override = default;
+
+ // Starts or resumes decoding `codec_memory`.
+ //
+ // Returns `false` upon failure. Returns `true` when either 1) the whole
+ // image has been successfully decoded or 2) the image has been partially
+ // decoded but decoding should be continued/retried when more input data
+ // is available.
+ //
+ // Communicates image metadata (once read/available) via
+ // `PngDecoderDelegate::PngReadHeader`. Writes decoded BGRA pixels to the
+ // buffer provided via `PngDecoderDelegate::PngAskImageBuf`.
+ bool ContinueDecode(RetainPtr<CFX_CodecMemory> codec_memory);
+
+ private:
+ enum class State {
+ // `decoder_` is null.
+ //
+ // This is the initial state.
+ kNoDecoder,
+
+ // `decoder_` is not null and `delegate_->PngReadHeader` succeeded
+ // and provided `target_gamma_`.
+ //
+ // `startIncrementalDecode` didn't run, or returned `kIncompleteInput`.
+ kGotDecoder,
+
+ // `decoder_` is not null, got `target_gamma_`, and `startIncrementalDecode`
+ // has already suceeded.
+ //
+ // `incrementalDecode` didn't run yet, or returned `kIncompleteInput`.
+ kStartedDecode,
+
+ // `decoder_` is null. All pixels have been decoded to
+ // `delegate_->PngAskImageBuf`.
+ //
+ // This is a terminal state.
+ kFinishedDecoding,
+
+ // `decoder_` is null. A non-recoverable (i.e. non-`kIncompleteInput`-kind)
+ // error has been encountered.
+ //
+ // This is a terminal state.
+ kError,
+ };
+ State state_ = State::kNoDecoder;
+
+ UnownedPtr<PngDecoderDelegate> const delegate_;
+ std::unique_ptr<SkCodec> decoder_;
+ double target_gamma_ = 0.0;
+
+ // `CFX_CodecMemory` received in `ContinueDecode` may get wrapped in
+ // `CodecMemoryStream` and become transitively owned by `decoder_`. This
+ // class retains `codec_memory_` to `CHECK` that all calls to `ContinueDecode`
+ // use the same `CFX_CodecMemory` - this helps to ensure that `decoder_` and
+ // `CodecMemoryStream` won't accidentally use stale input data if a future,
+ // hypothetical refactoring of `ProgressiveDecoder` changes how it manages
+ // `CFX_CodecMemory`.
+ RetainPtr<CFX_CodecMemory> codec_memory_;
+};
+
+bool SkiaPngContext::ContinueDecode(RetainPtr<CFX_CodecMemory> codec_memory) {
+ // `ProgressiveDecoder` guarantees that all calls to `ContinueDecode` use the
+ // same `codec_memory`. Therefore `SkiaPngContext` expects that
+ // `codec_memory` passed to `CodecMemoryStream` below remains the right one to
+ // use going forward.
+ if (!codec_memory_) {
+ codec_memory_ = codec_memory;
+ }
+ CHECK_EQ(&*codec_memory_, &*codec_memory);
+
+ switch (state_) {
+ case State::kNoDecoder: {
+ CHECK(!decoder_);
+ auto stream =
+ std::make_unique<CodecMemoryStream>(std::move(codec_memory));
+ SkCodec::Result result = SkCodec::kSuccess;
+#ifdef PDF_ENABLE_RUST_PNG
+ decoder_ = SkPngRustDecoder::Decode(std::move(stream), &result);
+#else
+ decoder_ = SkPngDecoder::Decode(std::move(stream), &result);
+#endif
+ switch (result) {
+ case SkCodec::kSuccess: {
+ SkImageInfo info = decoder_->getInfo();
+ if (!delegate_->PngReadHeader(info.width(), info.height(),
+ &target_gamma_)) {
+ decoder_.reset();
+ state_ = State::kError;
+ return false;
+ }
+ state_ = State::kGotDecoder;
+ break; // continue decoding
+ }
+ case SkCodec::kIncompleteInput:
+ // Rewind to start from the beginning of input when retrying later.
+ // This will also prompt `ProgressiveDecoder::ReadMoreData` to grow
+ // the `codec_memory_` as needed.
+ codec_memory_->Seek(0);
+ return true; // retry when called later with more data
+ default:
+ decoder_.reset();
+ state_ = State::kError;
+ return false; // fatal error
+ }
+ break;
+ }
+ case State::kGotDecoder:
+ case State::kStartedDecode:
+ break;
+ case State::kFinishedDecoding:
+ case State::kError:
+ NOTREACHED();
+ }
+
+ if (state_ == State::kGotDecoder) {
+ SkImageInfo dst_info =
+ decoder_->getInfo()
+ .makeColorSpace(GetTargetColorSpace(target_gamma_))
+ .makeColorType(kBGRA_8888_SkColorType);
+
+ pdfium::span<uint8_t> dst_buffer = delegate_->PngAskImageBuf();
+ FX_SAFE_SIZE_T row_bytes = dst_buffer.size();
+ row_bytes /= dst_info.height();
+
+ SkCodec::Result result = decoder_->startIncrementalDecode(
+ dst_info, dst_buffer.data(), row_bytes.ValueOrDie());
+ switch (result) {
+ case SkCodec::kSuccess:
+ state_ = State::kStartedDecode;
+ break; // continue decoding
+ case SkCodec::kIncompleteInput:
+ return true; // retry when called later with more data
+ default:
+ decoder_.reset();
+ state_ = State::kError;
+ return false; // fatal error
+ }
+ }
+
+ CHECK_EQ(state_, State::kStartedDecode);
+ SkCodec::Result result = decoder_->incrementalDecode(nullptr);
+ switch (result) {
+ case SkCodec::kSuccess:
+ decoder_.reset();
+ delegate_->PngFinishedDecoding();
+ state_ = State::kFinishedDecoding;
+ return true; // finished decoding
+ case SkCodec::kIncompleteInput:
+ return true; // retry when called later with more data
+ default:
+ decoder_.reset();
+ state_ = State::kError;
+ return false; // fatal error
+ }
+}
+
+} // namespace
+
+// static
+std::unique_ptr<ProgressiveDecoderContext> SkiaPngDecoder::StartDecode(
+ PngDecoderDelegate* delegate) {
+ return std::make_unique<SkiaPngContext>(delegate);
+}
+
+// static
+bool SkiaPngDecoder::ContinueDecode(ProgressiveDecoderContext* context,
+ RetainPtr<CFX_CodecMemory> codec_memory) {
+ auto* ctx = static_cast<SkiaPngContext*>(context);
+ return ctx->ContinueDecode(std::move(codec_memory));
+}
+
+} // namespace fxcodec
diff --git a/core/fxcodec/png/skia_png_decoder.h b/core/fxcodec/png/skia_png_decoder.h
new file mode 100644
index 0000000..eccac70
--- /dev/null
+++ b/core/fxcodec/png/skia_png_decoder.h
@@ -0,0 +1,46 @@
+// Copyright 2025 The PDFium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CORE_FXCODEC_PNG_SKIA_PNG_DECODER_H_
+#define CORE_FXCODEC_PNG_SKIA_PNG_DECODER_H_
+
+#include <memory>
+
+#include "core/fxcodec/cfx_codec_memory.h"
+#include "core/fxcrt/retain_ptr.h"
+
+#ifndef PDF_ENABLE_XFA_PNG
+#error "PNG must be enabled"
+#endif
+
+#ifndef PDF_USE_SKIA
+#error "Skia must be enabled"
+#endif
+
+namespace fxcodec {
+
+class PngDecoderDelegate;
+class ProgressiveDecoderContext;
+
+// PNG decoder that uses the Skia library to decode pixels. (Whether
+// `SkPngDecoder` vs `SkPngRustDecoder` is used depends on whether the build
+// configuration sets certain macro definitions like `PDF_ENABLE_RUST_PNG`).
+class SkiaPngDecoder {
+ public:
+ static std::unique_ptr<ProgressiveDecoderContext> StartDecode(
+ PngDecoderDelegate* delegate);
+
+ static bool ContinueDecode(ProgressiveDecoderContext* context,
+ RetainPtr<CFX_CodecMemory> codec_memory);
+
+ SkiaPngDecoder() = delete;
+ SkiaPngDecoder(const SkiaPngDecoder&) = delete;
+ SkiaPngDecoder& operator=(const SkiaPngDecoder&) = delete;
+};
+
+} // namespace fxcodec
+
+using PngDecoder = fxcodec::SkiaPngDecoder;
+
+#endif // CORE_FXCODEC_PNG_SKIA_PNG_DECODER_H_
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp
index cb00ef3..68159f6 100644
--- a/core/fxcodec/progressive_decoder.cpp
+++ b/core/fxcodec/progressive_decoder.cpp
@@ -39,8 +39,16 @@
#endif // PDF_ENABLE_XFA_GIF
#ifdef PDF_ENABLE_XFA_PNG
-#include "core/fxcodec/png/libpng_png_decoder.h"
#include "core/fxcodec/png/png_decoder_delegate.h"
+// TODO(https://crbug.com/444045690): Remove `pdf_enable_rust_png` from the
+// condition below once this build mode has been tested and stabilized.
+// (Chromium already sets `pdf_use_skia_override = true` so having an extra
+// condition avoids affecting the Chromium behavior.)
+#if defined(PDF_USE_SKIA) && defined(PDF_ENABLE_RUST_PNG)
+#include "core/fxcodec/png/skia_png_decoder.h"
+#else
+#include "core/fxcodec/png/libpng_png_decoder.h"
+#endif
#endif // PDF_ENABLE_XFA_PNG
#ifdef PDF_ENABLE_XFA_TIFF
@@ -114,6 +122,12 @@
return device_bitmap_->GetWritableScanline(line);
}
+pdfium::span<uint8_t> ProgressiveDecoder::PngAskImageBuf() {
+ CHECK_EQ(device_bitmap_->GetFormat(), FXDIB_Format::kBgra);
+ CHECK_EQ(src_format_, FXCodec_Argb);
+ return device_bitmap_->GetWritableBuffer();
+}
+
void ProgressiveDecoder::PngFinishedDecoding() {
status_ = FXCODEC_STATUS::kDecodeFinished;
}
diff --git a/core/fxcodec/progressive_decoder.h b/core/fxcodec/progressive_decoder.h
index 260bf18..1c3d783 100644
--- a/core/fxcodec/progressive_decoder.h
+++ b/core/fxcodec/progressive_decoder.h
@@ -90,6 +90,7 @@
int height,
double* gamma) override;
pdfium::span<uint8_t> PngAskScanlineBuf(int line) override;
+ pdfium::span<uint8_t> PngAskImageBuf() override;
void PngFinishedDecoding() override;
#endif // PDF_ENABLE_XFA_PNG
diff --git a/pdfium.gni b/pdfium.gni
index e2d3ef4..73ea9f4 100644
--- a/pdfium.gni
+++ b/pdfium.gni
@@ -65,8 +65,6 @@
# Build PDFium with Rust-based PNG decoder and encoder.
# If enabled, it ensures that PDFium doesn't depend on `libpng`.
# Note that `pdf_enable_rust_png` requires Skia and Rust support.
- #
- # TODO(https://crbug.com/444045690): Cover decoding path in production code.
pdf_enable_rust_png = pdf_enable_rust_png_override
# Build PDFium standalone. Now only controls whether the test binaries
