0

[PDF Ink Signatures] Clean up PdfInkBrush code

- Change GetInkBrush() to an accessor, since that is what it is.
- Mark `ink_brush_` as const.
- Clarify comments.
- Remove an unused #include.

Change-Id: Ib08ddb893dfba56737851d0b7428a882e4b6355c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5918058
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1366264}
This commit is contained in:
Lei Zhang
2024-10-09 17:03:34 +00:00
committed by Chromium LUCI CQ
parent f3bb96419b
commit f52632d73d
5 changed files with 14 additions and 21 deletions

@@ -103,10 +103,6 @@ PdfInkBrush::PdfInkBrush(Type brush_type, Params brush_params)
PdfInkBrush::~PdfInkBrush() = default; PdfInkBrush::~PdfInkBrush() = default;
const ink::Brush& PdfInkBrush::GetInkBrush() const {
return ink_brush_;
}
gfx::Rect PdfInkBrush::GetInvalidateArea(const gfx::PointF& center1, gfx::Rect PdfInkBrush::GetInvalidateArea(const gfx::PointF& center1,
const gfx::PointF& center2) const { const gfx::PointF& center2) const {
// For a line connecting `center1` to `center2`, the invalidate // For a line connecting `center1` to `center2`, the invalidate

@@ -5,7 +5,6 @@
#ifndef PDF_PDF_INK_BRUSH_H_ #ifndef PDF_PDF_INK_BRUSH_H_
#define PDF_PDF_INK_BRUSH_H_ #define PDF_PDF_INK_BRUSH_H_
#include <memory>
#include <optional> #include <optional>
#include <string> #include <string>
@@ -19,7 +18,7 @@ class PointF;
namespace chrome_pdf { namespace chrome_pdf {
// A class used to create ink brushes for PDF annotation mode and support // A class used to create Ink brushes for PDF annotation mode and support
// invalidation for rendering. // invalidation for rendering.
class PdfInkBrush { class PdfInkBrush {
public: public:
@@ -36,7 +35,6 @@ class PdfInkBrush {
}; };
PdfInkBrush(Type brush_type, Params brush_params); PdfInkBrush(Type brush_type, Params brush_params);
PdfInkBrush(const PdfInkBrush&) = delete; PdfInkBrush(const PdfInkBrush&) = delete;
PdfInkBrush& operator=(const PdfInkBrush&) = delete; PdfInkBrush& operator=(const PdfInkBrush&) = delete;
~PdfInkBrush(); ~PdfInkBrush();
@@ -55,12 +53,11 @@ class PdfInkBrush {
// Returns whether `size` is in range or not. // Returns whether `size` is in range or not.
static bool IsToolSizeInRange(float size); static bool IsToolSizeInRange(float size);
// Returns the `ink::Brush` that `this` represents. const ink::Brush& ink_brush() const { return ink_brush_; }
const ink::Brush& GetInkBrush() const;
private: private:
// The ink brush of type `type_` with params` params_`. // The Ink brush initialized based on the PdfInkBrush ctor parameters.
ink::Brush ink_brush_; const ink::Brush ink_brush_;
}; };
} // namespace chrome_pdf } // namespace chrome_pdf

@@ -662,7 +662,7 @@ PdfInkModule::CreateInProgressStrokeSegmentsFromInputs() const {
} }
ink::InProgressStroke stroke; ink::InProgressStroke stroke;
stroke.Start(state.brush->GetInkBrush()); stroke.Start(state.brush->ink_brush());
auto enqueue_results = auto enqueue_results =
stroke.EnqueueInputs(segment, /*predicted_inputs=*/{}); stroke.EnqueueInputs(segment, /*predicted_inputs=*/{});
CHECK(enqueue_results.ok()); CHECK(enqueue_results.ok());
@@ -824,7 +824,7 @@ void PdfInkModule::MaybeSetCursor() {
SkColor color; SkColor color;
float brush_size; float brush_size;
if (is_drawing_stroke()) { if (is_drawing_stroke()) {
const auto& ink_brush = drawing_stroke_state().brush->GetInkBrush(); const auto& ink_brush = drawing_stroke_state().brush->ink_brush();
color = GetSkColorFromInkBrush(ink_brush); color = GetSkColorFromInkBrush(ink_brush);
brush_size = ink_brush.GetSize(); brush_size = ink_brush.GetSize();
} else { } else {

@@ -303,7 +303,7 @@ TEST_F(PdfInkModuleTest, HandleSetAnnotationBrushMessagePen) {
const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting(); const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting();
ASSERT_TRUE(brush); ASSERT_TRUE(brush);
const ink::Brush& ink_brush = brush->GetInkBrush(); const ink::Brush& ink_brush = brush->ink_brush();
EXPECT_EQ(ink::Color::FromUint8(/*red=*/10, /*green=*/255, /*blue=*/50, EXPECT_EQ(ink::Color::FromUint8(/*red=*/10, /*green=*/255, /*blue=*/50,
/*alpha=*/255), /*alpha=*/255),
ink_brush.GetColor()); ink_brush.GetColor());
@@ -331,7 +331,7 @@ TEST_F(PdfInkModuleTest, HandleSetAnnotationBrushMessageHighlighter) {
const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting(); const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting();
ASSERT_TRUE(brush); ASSERT_TRUE(brush);
const ink::Brush& ink_brush = brush->GetInkBrush(); const ink::Brush& ink_brush = brush->ink_brush();
EXPECT_EQ(ink::Color::FromUint8(/*red=*/240, /*green=*/133, /*blue=*/0, EXPECT_EQ(ink::Color::FromUint8(/*red=*/240, /*green=*/133, /*blue=*/0,
/*alpha=*/255), /*alpha=*/255),
ink_brush.GetColor()); ink_brush.GetColor());
@@ -358,7 +358,7 @@ TEST_F(PdfInkModuleTest, HandleSetAnnotationBrushMessageColorZero) {
const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting(); const PdfInkBrush* brush = ink_module().GetPdfInkBrushForTesting();
ASSERT_TRUE(brush); ASSERT_TRUE(brush);
const ink::Brush& ink_brush = brush->GetInkBrush(); const ink::Brush& ink_brush = brush->ink_brush();
EXPECT_EQ(ink::Color::Black(), ink_brush.GetColor()); EXPECT_EQ(ink::Color::Black(), ink_brush.GetColor());
EXPECT_EQ(4.5f, ink_brush.GetSize()); EXPECT_EQ(4.5f, ink_brush.GetSize());
ASSERT_EQ(1u, ink_brush.CoatCount()); ASSERT_EQ(1u, ink_brush.CoatCount());
@@ -1306,10 +1306,10 @@ TEST_F(PdfInkModuleGetVisibleStrokesTest, MultiplePageStrokes) {
EXPECT_THAT( EXPECT_THAT(
collected_strokes, collected_strokes,
ElementsAre( ElementsAre(
Pair(0, Pointwise(InkStrokeEq(brush->GetInkBrush()), Pair(0, Pointwise(InkStrokeEq(brush->ink_brush()),
{expected_page0_horz_line_input_batch.value(), {expected_page0_horz_line_input_batch.value(),
expected_page0_vert_line_input_batch.value()})), expected_page0_vert_line_input_batch.value()})),
Pair(1, Pointwise(InkStrokeEq(brush->GetInkBrush()), Pair(1, Pointwise(InkStrokeEq(brush->ink_brush()),
{expected_page1_horz_line_input_batch.value()})))); {expected_page1_horz_line_input_batch.value()}))));
} }

@@ -130,7 +130,7 @@ TEST_P(PDFiumInkWriterTest, Basic) {
std::optional<ink::StrokeInputBatch> inputs = std::optional<ink::StrokeInputBatch> inputs =
CreateInkInputBatch(kBasicInputs); CreateInkInputBatch(kBasicInputs);
ASSERT_TRUE(inputs.has_value()); ASSERT_TRUE(inputs.has_value());
ink::Stroke stroke(brush->GetInkBrush(), inputs.value()); ink::Stroke stroke(brush->ink_brush(), inputs.value());
ASSERT_TRUE(WriteStrokeToPage(engine->doc(), page, stroke)); ASSERT_TRUE(WriteStrokeToPage(engine->doc(), page, stroke));
ASSERT_TRUE(FPDFPage_GenerateContent(page)); ASSERT_TRUE(FPDFPage_GenerateContent(page));
@@ -155,7 +155,7 @@ TEST_P(PDFiumInkWriterTest, EmptyStroke) {
auto brush = auto brush =
std::make_unique<PdfInkBrush>(PdfInkBrush::Type::kPen, kBasicBrushParams); std::make_unique<PdfInkBrush>(PdfInkBrush::Type::kPen, kBasicBrushParams);
ink::Stroke unused_stroke(brush->GetInkBrush()); ink::Stroke unused_stroke(brush->ink_brush());
ASSERT_FALSE(WriteStrokeToPage(engine->doc(), page, unused_stroke)); ASSERT_FALSE(WriteStrokeToPage(engine->doc(), page, unused_stroke));
} }
@@ -171,7 +171,7 @@ TEST_P(PDFiumInkWriterTest, NoDocumentNoPage) {
auto brush = auto brush =
std::make_unique<PdfInkBrush>(PdfInkBrush::Type::kPen, kBasicBrushParams); std::make_unique<PdfInkBrush>(PdfInkBrush::Type::kPen, kBasicBrushParams);
ink::Stroke unused_stroke(brush->GetInkBrush()); ink::Stroke unused_stroke(brush->ink_brush());
ASSERT_FALSE( ASSERT_FALSE(
WriteStrokeToPage(/*document=*/nullptr, /*page=*/nullptr, unused_stroke)); WriteStrokeToPage(/*document=*/nullptr, /*page=*/nullptr, unused_stroke));
ASSERT_FALSE(WriteStrokeToPage(/*document=*/nullptr, page, unused_stroke)); ASSERT_FALSE(WriteStrokeToPage(/*document=*/nullptr, page, unused_stroke));