0

Remove out-parameters from PDFiumEngine::GetRegion()

Return a struct instead. Then:
- Change PDFiumEngine::Highlight() to take the struct as a param in
  place of 2 separate params.
- Use raw_ptr in the struct.

Change-Id: I03304428d699dd311c9c5eeffdeead99c652a3a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4978018
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1218473}
This commit is contained in:
Lei Zhang
2023-11-01 22:03:13 +00:00
committed by Chromium LUCI CQ
parent f53aae45cb
commit 7ad30b36a9
2 changed files with 51 additions and 36 deletions

@ -3337,10 +3337,9 @@ void PDFiumEngine::DrawSelections(int progressive_index,
int page_index = progressive_paints_[progressive_index].page_index();
gfx::Rect dirty_in_screen = progressive_paints_[progressive_index].rect();
base::span<uint8_t> region;
size_t stride;
GetRegion(dirty_in_screen.origin(), image_data, region, stride);
if (region.empty()) {
const absl::optional<RegionData> region =
GetRegion(dirty_in_screen.origin(), image_data);
if (!region.has_value()) {
return;
}
@ -3359,7 +3358,7 @@ void PDFiumEngine::DrawSelections(int progressive_index,
continue;
visible_selection.Offset(-dirty_in_screen.OffsetFromOrigin());
Highlight(region, stride, visible_selection, kHighlightColorR,
Highlight(region.value(), visible_selection, kHighlightColorR,
kHighlightColorG, kHighlightColorB, highlighted_rects);
}
}
@ -3371,7 +3370,7 @@ void PDFiumEngine::DrawSelections(int progressive_index,
continue;
visible_selection.Offset(-dirty_in_screen.OffsetFromOrigin());
Highlight(region, stride, visible_selection, kHighlightColorR,
Highlight(region.value(), visible_selection, kHighlightColorR,
kHighlightColorG, kHighlightColorB, highlighted_rects);
}
}
@ -3405,16 +3404,15 @@ int PDFiumEngine::GetProgressiveIndex(int page_index) const {
ScopedFPDFBitmap PDFiumEngine::CreateBitmap(const gfx::Rect& rect,
bool has_alpha,
SkBitmap& image_data) const {
base::span<uint8_t> region;
size_t stride;
GetRegion(rect.origin(), image_data, region, stride);
if (region.empty()) {
const absl::optional<RegionData> region =
GetRegion(rect.origin(), image_data);
if (!region.has_value()) {
return nullptr;
}
int format = has_alpha ? FPDFBitmap_BGRA : FPDFBitmap_BGRx;
return ScopedFPDFBitmap(FPDFBitmap_CreateEx(rect.width(), rect.height(),
format, region.data(), stride));
return ScopedFPDFBitmap(
FPDFBitmap_CreateEx(rect.width(), rect.height(), format,
region.value().buffer.data(), region.value().stride));
}
void PDFiumEngine::GetPDFiumRect(int page_index,
@ -3488,15 +3486,12 @@ gfx::RectF PDFiumEngine::GetPageBoundingBox(int page_index) {
return page->GetBoundingBox();
}
void PDFiumEngine::Highlight(base::span<uint8_t> buffer,
size_t stride,
void PDFiumEngine::Highlight(const RegionData& region,
const gfx::Rect& rect,
int color_red,
int color_green,
int color_blue,
std::vector<gfx::Rect>& highlighted_rects) const {
CHECK(!buffer.empty());
gfx::Rect new_rect = rect;
for (const auto& highlighted : highlighted_rects)
new_rect.Subtract(highlighted);
@ -3516,7 +3511,8 @@ void PDFiumEngine::Highlight(base::span<uint8_t> buffer,
int h = new_rect.height();
for (int y = t; y < t + h; ++y) {
base::span<uint8_t> row = buffer.subspan(y * stride, stride);
base::span<uint8_t> row =
region.buffer.subspan(y * region.stride, region.stride);
for (int x = l; x < l + w; ++x) {
bool overlaps = false;
for (size_t i : overlapping_rect_indices) {
@ -3638,6 +3634,16 @@ bool PDFiumEngine::MouseDownState::Matches(
return true;
}
PDFiumEngine::RegionData::RegionData(base::span<uint8_t> buffer, size_t stride)
: buffer(buffer), stride(stride) {}
PDFiumEngine::RegionData::RegionData(RegionData&&) = default;
PDFiumEngine::RegionData& PDFiumEngine::RegionData::operator=(RegionData&&) =
default;
PDFiumEngine::RegionData::~RegionData() = default;
void PDFiumEngine::DeviceToPage(int page_index,
const gfx::Point& device_point,
double* page_x,
@ -3717,30 +3723,31 @@ void PDFiumEngine::DrawPageShadow(const gfx::Rect& page_rc,
DrawShadow(image_data, shadow_rect, page_rect, clip_rect, *page_shadow_);
}
void PDFiumEngine::GetRegion(const gfx::Point& location,
SkBitmap& image_data,
base::span<uint8_t>& region,
size_t& stride) const {
absl::optional<PDFiumEngine::RegionData> PDFiumEngine::GetRegion(
const gfx::Point& location,
SkBitmap& image_data) const {
if (image_data.isNull()) {
DCHECK(plugin_size().IsEmpty());
stride = 0;
return;
return absl::nullopt;
}
uint8_t* buffer = static_cast<uint8_t*>(image_data.getPixels());
stride = image_data.rowBytes();
if (!buffer) {
return absl::nullopt;
}
gfx::Point offset_location = location + page_offset_;
// TODO: update this when we support BIDI and scrollbars can be on the left.
if (!buffer ||
!gfx::Rect(gfx::PointAtOffsetFromOrigin(page_offset_), plugin_size())
if (!gfx::Rect(gfx::PointAtOffsetFromOrigin(page_offset_), plugin_size())
.Contains(offset_location)) {
return;
return absl::nullopt;
}
size_t stride = image_data.rowBytes();
base::span<uint8_t> buffer_span(buffer, image_data.height() * stride);
size_t x_offset = location.x() + page_offset_.x();
size_t offset = location.y() * stride + x_offset * 4;
region = buffer_span.subspan(offset);
return PDFiumEngine::RegionData(buffer_span.subspan(offset), stride);
}
void PDFiumEngine::OnSelectionTextChanged() {

@ -17,6 +17,7 @@
#include "base/containers/flat_map.h"
#include "base/containers/span.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_span.h"
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
@ -259,6 +260,16 @@ class PDFiumEngine : public PDFEngine,
PDFiumPage::LinkTarget target_;
};
struct RegionData {
RegionData(base::span<uint8_t> buffer, size_t stride);
RegionData(RegionData&&);
RegionData& operator=(RegionData&&);
~RegionData();
base::raw_span<uint8_t> buffer; // Never empty.
size_t stride;
};
friend class FormFillerTest;
friend class PDFiumEngineTabbingTest;
friend class PDFiumFormFiller;
@ -517,11 +528,10 @@ class PDFiumEngine : public PDFEngine,
// coordinates. (i.e. 0,0 is top left corner of plugin area)
gfx::Rect GetScreenRect(const gfx::Rect& rect) const;
// Given an image `buffer` with `stride`, highlights `rect`.
// Given an image `region`, highlights `rect`.
// `highlighted_rects` contains the already highlighted rectangles and will be
// updated to include `rect` if `rect` has not already been highlighted.
void Highlight(base::span<uint8_t> buffer,
size_t stride,
void Highlight(const RegionData& region,
const gfx::Rect& rect,
int color_red,
int color_green,
@ -548,10 +558,8 @@ class PDFiumEngine : public PDFEngine,
const gfx::Rect& clip_rect,
SkBitmap& image_data);
void GetRegion(const gfx::Point& location,
SkBitmap& image_data,
base::span<uint8_t>& region,
size_t& stride) const;
absl::optional<RegionData> GetRegion(const gfx::Point& location,
SkBitmap& image_data) const;
// Called when the selection changes.
void OnSelectionTextChanged();