0

Revert "[PDF Ink Signatures] Switch DrawRenderTransform test case to call Draw()"

This reverts commit 9d82486a93.

Reason for revert:
  Likely causing PdfInkModuleStrokeTest.DrawRenderTransform failure
  on win-asan. First failure:
  https://ci.chromium.org/ui/p/chromium/builders/ci/win-asan/32344/overview

Original change's description:
> [PDF Ink Signatures] Switch DrawRenderTransform test case to call Draw()
>
> Remove the code that checks the transformed Draw() would have used, and
> instead check the output from Draw(). Add a FuzzyMatchesPngFile() test
> utility to check against new expectation PNGs in //pdf/test/data/ink.
> The existing exact-pixel MatchesPngFile() function mostly works, but not
> for ASAN/TSAN.
>
> Bug: 351990827
> Change-Id: Ie11244bcfe91a413f102b8a3bbc34d4c199f789c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5871447
> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
> Reviewed-by: Alan Screen <awscreen@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1357215}

Bug: 351990827
Change-Id: I5efae8a8797dd2f61d5098749d972eeb2f630bf7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5874746
Auto-Submit: Jonathan Lee <jonathanjlee@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Jonathan Lee <jonathanjlee@google.com>
Cr-Commit-Position: refs/heads/main@{#1357422}
This commit is contained in:
Jonathan Lee
2024-09-19 00:34:10 +00:00
committed by Chromium LUCI CQ
parent b4c62008e2
commit bb55899b38
7 changed files with 54 additions and 61 deletions

@ -124,6 +124,9 @@ void PdfInkModule::Draw(SkCanvas& canvas) {
const gfx::Rect content_rect = client_->GetPageContentsRect(page_index);
const ink::AffineTransform transform =
GetInkRenderTransform(origin_offset, rotation, content_rect, zoom);
if (draw_render_transform_callback_for_testing_) {
draw_render_transform_callback_for_testing_.Run(transform);
}
SkAutoCanvasRestore save_restore(&canvas, /*doSave=*/true);
canvas.clipRect(GetDrawPageClipRect(content_rect, origin_offset));
@ -146,6 +149,9 @@ void PdfInkModule::Draw(SkCanvas& canvas) {
client_->GetPageContentsRect(state.page_index);
const ink::AffineTransform transform =
GetInkRenderTransform(origin_offset, rotation, content_rect, zoom);
if (draw_render_transform_callback_for_testing_) {
draw_render_transform_callback_for_testing_.Run(transform);
}
SkAutoCanvasRestore save_restore(&canvas, /*doSave=*/true);
canvas.clipRect(GetDrawPageClipRect(content_rect, origin_offset));
@ -275,6 +281,11 @@ PdfInkModule::GetVisibleStrokesInputPositionsForTesting() const {
return all_strokes_points;
}
void PdfInkModule::SetDrawRenderTransformCallbackForTesting(
RenderTransformCallback callback) {
draw_render_transform_callback_for_testing_ = std::move(callback);
}
bool PdfInkModule::OnMouseDown(const blink::WebMouseEvent& event) {
CHECK(enabled());

@ -14,6 +14,7 @@
#include <vector>
#include "base/containers/flat_set.h"
#include "base/functional/callback.h"
#include "base/memory/raw_ref.h"
#include "base/time/time.h"
#include "base/values.h"
@ -34,6 +35,10 @@ class WebInputEvent;
class WebMouseEvent;
} // namespace blink
namespace ink {
class AffineTransform;
}
namespace chrome_pdf {
class PdfInkBrush;
@ -51,6 +56,9 @@ class PdfInkModule {
// strokes for that page.
using DocumentStrokeInputPointsMap = std::map<int, PageStrokeInputPoints>;
using RenderTransformCallback =
base::RepeatingCallback<void(const ink::AffineTransform& transform)>;
explicit PdfInkModule(PdfInkModuleClient& client);
PdfInkModule(const PdfInkModule&) = delete;
PdfInkModule& operator=(const PdfInkModule&) = delete;
@ -90,6 +98,11 @@ class PdfInkModule {
DocumentStrokeInputPointsMap GetVisibleStrokesInputPositionsForTesting()
const;
// For testing only. Provide a callback to use whenever the rendering
// transform is determined for `Draw()`.
void SetDrawRenderTransformCallbackForTesting(
RenderTransformCallback callback);
private:
using StrokeInputSegment = std::vector<ink::StrokeInput>;
@ -258,6 +271,8 @@ class PdfInkModule {
DocumentStrokesMap strokes_;
PdfInkUndoRedoModel undo_redo_model_;
RenderTransformCallback draw_render_transform_callback_for_testing_;
};
} // namespace chrome_pdf

@ -12,7 +12,6 @@
#include "base/containers/contains.h"
#include "base/containers/span.h"
#include "base/containers/to_vector.h"
#include "base/files/file_path.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/values.h"
@ -22,15 +21,12 @@
#include "pdf/pdf_ink_transform.h"
#include "pdf/test/mouse_event_builder.h"
#include "pdf/test/pdf_ink_test_helpers.h"
#include "pdf/test/test_helpers.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/input/web_mouse_event.h"
#include "third_party/ink/src/ink/brush/brush.h"
#include "third_party/ink/src/ink/geometry/affine_transform.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkImage.h"
#include "ui/gfx/geometry/point_f.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/rect_conversions.h"
@ -85,10 +81,6 @@ constexpr gfx::PointF kTwoPageVerticalLayoutPageExitAndReentrySegment2[] = {
gfx::PointF(10.0f, 0.0f), gfx::PointF(10.0f, 5.0f),
gfx::PointF(15.0f, 10.0f)};
base::FilePath GetInkTestDataFilePath(std::string_view filename) {
return base::FilePath(FILE_PATH_LITERAL("ink")).AppendASCII(filename);
}
class FakeClient : public PdfInkModuleClient {
public:
FakeClient() = default;
@ -481,20 +473,6 @@ class PdfInkModuleStrokeTest : public PdfInkModuleTest {
return ink_module().GetVisibleStrokesInputPositionsForTesting();
}
void DrawWithSizeAndFuzzyCompare(const gfx::Size& bitmap_size,
std::string_view expected_png_filename) {
// Uses MakeN32Premul() and clear the canvas just like
// PdfViewWebPlugin::Paint().
SkBitmap bitmap;
bitmap.allocPixels(
SkImageInfo::MakeN32Premul(bitmap_size.width(), bitmap_size.height()));
SkCanvas canvas(bitmap);
canvas.clear(SK_ColorTRANSPARENT);
ink_module().Draw(canvas);
EXPECT_TRUE(FuzzyMatchesPngFile(
bitmap.asImage().get(), GetInkTestDataFilePath(expected_png_filename)));
}
private:
void ApplyStrokeWithMouseAtPointsMaybeHandled(
const gfx::PointF& mouse_down_point,
@ -576,13 +554,27 @@ TEST_F(PdfInkModuleStrokeTest, DrawRenderTransform) {
RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
constexpr gfx::Size kBitmapSize(100, 100);
DrawWithSizeAndFuzzyCompare(kBitmapSize,
"draw_render_transform_simple_stroke.png");
// Simulate drawing the strokes, and verify that the expected transform was
// used.
std::vector<ink::AffineTransform> draw_render_transforms;
ink_module().SetDrawRenderTransformCallbackForTesting(
base::BindLambdaForTesting([&](const ink::AffineTransform& transform) {
draw_render_transforms.push_back(transform);
}));
SkCanvas canvas;
ink_module().Draw(canvas);
// But if the one and only page is not visible, then Draw() does nothing.
// Just one transform provided, to match the captured stroke.
EXPECT_THAT(draw_render_transforms,
ElementsAre(InkAffineTransformEq(-1.0f, 0.0f, 54.0f, 0.0f, -1.0f,
44.0f)));
// But if the one and only page is not visible, then Draw() does no transform
// calculations.
draw_render_transforms.clear();
client().set_page_visibility(0, false);
DrawWithSizeAndFuzzyCompare(kBitmapSize, "draw_render_transform_blank.png");
ink_module().Draw(canvas);
EXPECT_TRUE(draw_render_transforms.empty());
}
TEST_F(PdfInkModuleStrokeTest, InvalidationsFromStroke) {

Binary file not shown.

Before

(image error) Size: 104 B

Binary file not shown.

Before

(image error) Size: 254 B

@ -20,27 +20,6 @@
namespace chrome_pdf {
namespace {
testing::AssertionResult MatchesPngFileImpl(
const SkImage* actual_image,
const base::FilePath& expected_png_file,
const cc::PixelComparator& comparitor) {
SkBitmap actual_bitmap;
if (!actual_image->asLegacyBitmap(&actual_bitmap)) {
return testing::AssertionFailure() << "Reference: " << expected_png_file;
}
if (!cc::MatchesPNGFile(actual_bitmap, GetTestDataFilePath(expected_png_file),
comparitor)) {
return testing::AssertionFailure() << "Reference: " << expected_png_file;
}
return testing::AssertionSuccess();
}
} // namespace
base::FilePath GetTestDataFilePath(const base::FilePath& path) {
return base::PathService::CheckedGet(base::DIR_SRC_TEST_DATA_ROOT)
.Append(FILE_PATH_LITERAL("pdf"))
@ -52,15 +31,16 @@ base::FilePath GetTestDataFilePath(const base::FilePath& path) {
testing::AssertionResult MatchesPngFile(
const SkImage* actual_image,
const base::FilePath& expected_png_file) {
return MatchesPngFileImpl(actual_image, expected_png_file,
cc::ExactPixelComparator());
}
SkBitmap actual_bitmap;
if (!actual_image->asLegacyBitmap(&actual_bitmap))
return testing::AssertionFailure() << "Reference: " << expected_png_file;
testing::AssertionResult FuzzyMatchesPngFile(
const SkImage* actual_image,
const base::FilePath& expected_png_file) {
return MatchesPngFileImpl(actual_image, expected_png_file,
cc::FuzzyPixelOffByOneComparator());
if (!cc::MatchesPNGFile(actual_bitmap, GetTestDataFilePath(expected_png_file),
cc::ExactPixelComparator())) {
return testing::AssertionFailure() << "Reference: " << expected_png_file;
}
return testing::AssertionSuccess();
}
sk_sp<SkSurface> CreateSkiaSurfaceForTesting(const gfx::Size& size,

@ -30,11 +30,6 @@ testing::AssertionResult MatchesPngFile(
const SkImage* actual_image,
const base::FilePath& expected_png_file);
// Same as MatchesPngFile() above, but with a fuzzy pixel comparator.
testing::AssertionResult FuzzyMatchesPngFile(
const SkImage* actual_image,
const base::FilePath& expected_png_file);
// Creates a Skia surface with dimensions `size` and filled with `color`.
sk_sp<SkSurface> CreateSkiaSurfaceForTesting(const gfx::Size& size,
SkColor color);