[PDF Ink Signatures] Deactivate Ink strokes before rendering thumbnails
For PDF thumbnails, PDFiumEngine::RequestThumbnail() should be drawing the original thumbnail, without any Ink strokes. PdfInkModule::DrawThumbnail() separately draws the Ink strokes as another layer on top of the original thumbnail. With the work in https://crbug.com/335517469, now the PDF loaded in PDFiumEngine contains Ink strokes, so refreshing PDF thumbnails will end up rendering the Ink strokes twice. Fix this by temporarily deactivating the page objects associated with Ink strokes for a given page when generating the thumbnail for that page. Bug: 395766076 Change-Id: I565df2f21b3fae8cf6ebe4df44149f3709fe149a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6271523 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Alan Screen <awscreen@chromium.org> Cr-Commit-Position: refs/heads/main@{#1422737}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
3ea9a63ba1
commit
003b56ea64
@ -514,6 +514,30 @@ void ParamsTransformPageToScreen(unsigned long view_fit_type,
|
||||
}
|
||||
}
|
||||
|
||||
#if BUILDFLAG(ENABLE_PDF_INK2)
|
||||
class ScopedPageObjectDeactivator {
|
||||
public:
|
||||
explicit ScopedPageObjectDeactivator(
|
||||
std::vector<FPDF_PAGEOBJECT> page_objects)
|
||||
: page_objects_(std::move(page_objects)) {
|
||||
for (FPDF_PAGEOBJECT page_object : page_objects_) {
|
||||
bool result = FPDFPageObj_SetIsActive(page_object, /*active=*/false);
|
||||
CHECK(result);
|
||||
}
|
||||
}
|
||||
~ScopedPageObjectDeactivator() {
|
||||
for (FPDF_PAGEOBJECT page_object : page_objects_) {
|
||||
bool result = FPDFPageObj_SetIsActive(page_object, /*active=*/true);
|
||||
CHECK(result);
|
||||
}
|
||||
}
|
||||
|
||||
private:
|
||||
std::vector<FPDF_PAGEOBJECT> page_objects_;
|
||||
};
|
||||
|
||||
#endif
|
||||
|
||||
} // namespace
|
||||
|
||||
void InitializeSDK(bool enable_v8,
|
||||
@ -4481,6 +4505,11 @@ void PDFiumEngine::RequestThumbnail(int page_index,
|
||||
// being progressively painted. Otherwise, wait for progressive painting to
|
||||
// finish.
|
||||
if (!GetProgressiveIndex(page_index).has_value()) {
|
||||
#if BUILDFLAG(ENABLE_PDF_INK2)
|
||||
ScopedPageObjectDeactivator deactivator(
|
||||
GetActiveInkPageObjectsForPage(page_index));
|
||||
#endif
|
||||
|
||||
pages_[page_index]->RequestThumbnail(device_pixel_ratio,
|
||||
std::move(send_callback));
|
||||
return;
|
||||
@ -4497,13 +4526,15 @@ void PDFiumEngine::MaybeRequestPendingThumbnail(int page_index) {
|
||||
CHECK(!GetProgressiveIndex(page_index).has_value());
|
||||
|
||||
auto it = pending_thumbnails_.find(page_index);
|
||||
if (it == pending_thumbnails_.end())
|
||||
if (it == pending_thumbnails_.end()) {
|
||||
return;
|
||||
}
|
||||
|
||||
PendingThumbnail& pending_thumbnail = it->second;
|
||||
pages_[page_index]->RequestThumbnail(
|
||||
pending_thumbnail.device_pixel_ratio,
|
||||
std::move(pending_thumbnail.send_callback));
|
||||
// CHECK() above prevents RequestThumbnail() from putting the request back
|
||||
// into `pending_thumbnail`.
|
||||
RequestThumbnail(page_index, pending_thumbnail.device_pixel_ratio,
|
||||
std::move(pending_thumbnail.send_callback));
|
||||
pending_thumbnails_.erase(it);
|
||||
}
|
||||
|
||||
@ -4645,6 +4676,27 @@ void PDFiumEngine::RegenerateContents() {
|
||||
ink_stroked_pages_needing_regeneration_.clear();
|
||||
}
|
||||
|
||||
std::vector<FPDF_PAGEOBJECT> PDFiumEngine::GetActiveInkPageObjectsForPage(
|
||||
int page_index) const {
|
||||
std::vector<FPDF_PAGEOBJECT> active_page_objects;
|
||||
for (const auto& it : ink_stroke_data_) {
|
||||
const InkStrokeData& datum = it.second;
|
||||
if (datum.page_index != page_index) {
|
||||
continue;
|
||||
}
|
||||
|
||||
for (FPDF_PAGEOBJECT page_object : datum.page_objects) {
|
||||
FPDF_BOOL active = false;
|
||||
bool result = FPDFPageObj_GetIsActive(page_object, &active);
|
||||
CHECK(result);
|
||||
if (active) {
|
||||
active_page_objects.push_back(page_object);
|
||||
}
|
||||
}
|
||||
}
|
||||
return active_page_objects;
|
||||
}
|
||||
|
||||
PDFiumEngine::InkStrokeData::InkStrokeData(
|
||||
int page_index,
|
||||
std::vector<FPDF_PAGEOBJECT> page_objects)
|
||||
|
@ -370,8 +370,12 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE {
|
||||
|
||||
virtual bool ReadLoadedBytes(uint32_t length, void* buffer);
|
||||
|
||||
// Requests for a thumbnail to be sent using a callback when the page is ready
|
||||
// to be rendered. `send_callback` is run with the thumbnail data when ready.
|
||||
// Requests rendering the page at `page_index` as a thumbnail at a given
|
||||
// `device_pixel_ratio`. Runs `send_callback` with the rendered thumbnail.
|
||||
//
|
||||
// - The callback is asynchronous because the request may be delayed if the
|
||||
// page is not ready to be rendered.
|
||||
// - Modifications made with ApplyStroke() are not included in the thumbnail.
|
||||
void RequestThumbnail(int page_index,
|
||||
float device_pixel_ratio,
|
||||
SendThumbnailCallback send_callback);
|
||||
@ -995,6 +999,11 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE {
|
||||
void OnOcrDisconnected();
|
||||
#endif
|
||||
|
||||
#if BUILDFLAG(ENABLE_PDF_INK2)
|
||||
std::vector<FPDF_PAGEOBJECT> GetActiveInkPageObjectsForPage(
|
||||
int page_index) const;
|
||||
#endif
|
||||
|
||||
const raw_ptr<PDFiumEngineClient> client_;
|
||||
|
||||
// The current document layout.
|
||||
|
@ -22,6 +22,7 @@
|
||||
#include "base/test/gtest_util.h"
|
||||
#include "base/test/mock_callback.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "base/test/test_future.h"
|
||||
#include "base/time/time.h"
|
||||
#include "build/build_config.h"
|
||||
#include "pdf/accessibility_structs.h"
|
||||
@ -71,6 +72,7 @@ namespace chrome_pdf {
|
||||
namespace {
|
||||
|
||||
using ::testing::_;
|
||||
using ::testing::Contains;
|
||||
using ::testing::InSequence;
|
||||
using ::testing::Invoke;
|
||||
using ::testing::IsEmpty;
|
||||
@ -2414,6 +2416,71 @@ TEST_P(PDFiumEngineInkDrawTest, LoadedV2InkPathsAndUpdateShapeActive) {
|
||||
1);
|
||||
}
|
||||
|
||||
TEST_P(PDFiumEngineInkDrawTest, ThumbnailsDoNotContainStrokes) {
|
||||
NiceMock<MockTestClient> client;
|
||||
std::unique_ptr<PDFiumEngine> engine =
|
||||
InitializeEngine(&client, FILE_PATH_LITERAL("blank.pdf"));
|
||||
ASSERT_TRUE(engine);
|
||||
|
||||
static constexpr int kPageIndex = 0;
|
||||
static constexpr float kDevicePixelRatio = 1;
|
||||
// Note that this is not the same size as pdf/test/data/blank.png.
|
||||
static constexpr gfx::Size kExpectedImageSize(140, 140);
|
||||
// Since blank.pdf renders at all white pixels, check by just counting the
|
||||
// pixels. The raw image data has 4 components per pixel.
|
||||
const size_t kExpectedWhiteComponentCount = kExpectedImageSize.GetArea() * 4;
|
||||
{
|
||||
base::test::TestFuture<Thumbnail> future;
|
||||
engine->RequestThumbnail(kPageIndex, kDevicePixelRatio,
|
||||
future.GetCallback());
|
||||
ASSERT_TRUE(future.Wait());
|
||||
|
||||
Thumbnail thumbnail = future.Take();
|
||||
ASSERT_EQ(kExpectedImageSize, thumbnail.image_size());
|
||||
EXPECT_THAT(thumbnail.GetImageData(),
|
||||
Contains(0xFF).Times(kExpectedWhiteComponentCount));
|
||||
}
|
||||
|
||||
// Draw 2 strokes.
|
||||
auto pen_brush = std::make_unique<PdfInkBrush>(PdfInkBrush::Type::kPen,
|
||||
SK_ColorRED, /*size=*/4.0f);
|
||||
static constexpr auto kPenInputs = std::to_array<PdfInkInputData>({
|
||||
{{5.0f, 5.0f}, base::Seconds(0.0f)},
|
||||
{{50.0f, 5.0f}, base::Seconds(0.1f)},
|
||||
});
|
||||
auto highlighter_brush = std::make_unique<PdfInkBrush>(
|
||||
PdfInkBrush::Type::kHighlighter, SK_ColorCYAN, /*size=*/6.0f);
|
||||
static constexpr auto kHighlighterInputs = std::to_array<PdfInkInputData>({
|
||||
{{75.0f, 5.0f}, base::Seconds(0.0f)},
|
||||
{{75.0f, 60.0f}, base::Seconds(0.1f)},
|
||||
});
|
||||
std::optional<ink::StrokeInputBatch> pen_inputs =
|
||||
CreateInkInputBatch(kPenInputs);
|
||||
ASSERT_TRUE(pen_inputs.has_value());
|
||||
std::optional<ink::StrokeInputBatch> highlighter_inputs =
|
||||
CreateInkInputBatch(kHighlighterInputs);
|
||||
ASSERT_TRUE(highlighter_inputs.has_value());
|
||||
ink::Stroke pen_stroke(pen_brush->ink_brush(), pen_inputs.value());
|
||||
ink::Stroke highligter_stroke(highlighter_brush->ink_brush(),
|
||||
highlighter_inputs.value());
|
||||
static constexpr InkStrokeId kPenStrokeId(1);
|
||||
static constexpr InkStrokeId kHighlighterStrokeId(2);
|
||||
engine->ApplyStroke(kPageIndex, kPenStrokeId, pen_stroke);
|
||||
engine->ApplyStroke(kPageIndex, kHighlighterStrokeId, highligter_stroke);
|
||||
|
||||
{
|
||||
base::test::TestFuture<Thumbnail> future;
|
||||
engine->RequestThumbnail(kPageIndex, kDevicePixelRatio,
|
||||
future.GetCallback());
|
||||
ASSERT_TRUE(future.Wait());
|
||||
|
||||
Thumbnail thumbnail = future.Take();
|
||||
ASSERT_EQ(kExpectedImageSize, thumbnail.image_size());
|
||||
EXPECT_THAT(thumbnail.GetImageData(),
|
||||
Contains(0xFF).Times(kExpectedWhiteComponentCount));
|
||||
}
|
||||
}
|
||||
|
||||
// Don't be concerned about any slight rendering differences in AGG vs. Skia,
|
||||
// covering one of these is sufficient for checking how data is written out.
|
||||
INSTANTIATE_TEST_SUITE_P(All, PDFiumEngineInkDrawTest, testing::Values(false));
|
||||
|
Reference in New Issue
Block a user