For shading types 4-7, evaluate function per pixel if present
CPDF_MeshStream::ReadColor() now no longer resolves function values
immediately (at the vertex level), but returns them in the red channel
of the returned color (mapped to the range Decode[4]..Decode[5]).
Over in cpdf_rendershading.cpp, DrawGouraud() (for types 4-5) and
PatchDrawer::Draw() (for types 6-7) now use the interpolated red
channel to evaluate funcs.
Evaluating the function per pixel was a ~8x slowdown for types 4-5,
measured by running
hyperfine 'pdfium_test --render-repeats=500 shade-gouraud-free.pdf'
On my system, on trunk this took around 92ms. With naively evaluating
the function per pixel, it took 1.0s instead. So instead of evaluating
the function per pixel, this now computes a 1D texture with 255 samples
of the function, and then looks up the function result per pixel. With
that, it takes 102ms, comparable to before. This is a minor quality
loss, but matches what we do for types 1-3 (it in fact uses the
already-existing GetShadingSteps() function to create the texture),
and it's worth it for the perf.
(I also tried editing that file and removing the part of it that uses
a function, so that only a function-less patch remains. In that case,
rendering takes around 54ms both before and after this patch, so
rendering time for function-less patches isn't changing at all.)
For patch rendering, I measured using
hyperfine 'pdfium_test --render-repeats=50 shade-tensor.pdf'
(Note: 10x fewer repetitions.)
That took 1.0s on trunk, with the naive patch, and with the 1D texture.
Our patch rendering is much slower than our gouraud triangle
rendering, and function evaluation is apparently cheap compared to
all the other work we're doing there. I still went with the 1D texture
here too, as the code is a bit nicer, and it's nice to be self-
consistent.
(This currently doesn't change kCoonColorThreshold for patches using
a function, which contributes to them looking a bit blocky -- see
new baselines in this patch. This could be addressed as part of
issue 418737845 later on.)
(Perf test iles from here:
https://github.com/SerenityOS/serenity/blob/master/Tests/LibPDF/shade-gouraud-free.pdf
https://github.com/SerenityOS/serenity/blob/master/Tests/LibPDF/shade-tensor.pdf
)
Fixes rendering of testing/resources/pixel/bug_1546.in and of the
left two columns of testing/resources/pixel/shade.in.
Bug: 406662638
Change-Id: Id70a62ea31afe29c368fa9ab007e40f69985d1be
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/132471
Commit-Queue: Nico Weber <thakis@chromium.org>
Auto-Submit: Nico Weber <thakis@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Nico Weber <thakis@google.com>
diff --git a/core/fpdfapi/page/cpdf_meshstream.cpp b/core/fpdfapi/page/cpdf_meshstream.cpp
index 03c54a0..380c642 100644
--- a/core/fpdfapi/page/cpdf_meshstream.cpp
+++ b/core/fpdfapi/page/cpdf_meshstream.cpp
@@ -217,18 +217,7 @@
if (funcs_.empty()) {
return cs_->GetRGBOrZerosOnError(color_value);
}
- float result[kMaxComponents] = {};
- pdfium::span<float> result_span = pdfium::span(result);
- for (const auto& func : funcs_) {
- if (func && func->OutputCount() <= kMaxComponents) {
- std::optional<uint32_t> nresults =
- func->Call(pdfium::span(color_value).first<1u>(), result_span);
- if (nresults.has_value()) {
- result_span = result_span.subspan(nresults.value());
- }
- }
- }
- return cs_->GetRGBOrZerosOnError(result);
+ return {color_value[0], 0.0f, 0.0f};
}
bool CPDF_MeshStream::ReadVertex(const CFX_Matrix& pObject2Bitmap,
diff --git a/core/fpdfapi/page/cpdf_meshstream.h b/core/fpdfapi/page/cpdf_meshstream.h
index dd2f449..fcac347 100644
--- a/core/fpdfapi/page/cpdf_meshstream.h
+++ b/core/fpdfapi/page/cpdf_meshstream.h
@@ -65,6 +65,13 @@
uint32_t ComponentBits() const { return component_bits_; }
uint32_t Components() const { return components_; }
+ float component_min(size_t component_index) const {
+ return color_min_[component_index];
+ }
+ float component_max(size_t component_index) const {
+ return color_max_[component_index];
+ }
+
private:
static constexpr uint32_t kMaxComponents = 8;
diff --git a/core/fpdfapi/render/cpdf_rendershading.cpp b/core/fpdfapi/render/cpdf_rendershading.cpp
index 3c69922..a36fec7 100644
--- a/core/fpdfapi/render/cpdf_rendershading.cpp
+++ b/core/fpdfapi/render/cpdf_rendershading.cpp
@@ -62,16 +62,20 @@
return funcs_outputs ? std::max(funcs_outputs, pCS->ComponentCount()) : 0;
}
-std::array<FX_ARGB, kShadingSteps> GetShadingSteps(
- float t_min,
- float t_max,
- const std::vector<std::unique_ptr<CPDF_Function>>& funcs,
- const RetainPtr<CPDF_ColorSpace>& pCS,
- int alpha,
- size_t results_count) {
+bool GetShadingSteps(float t_min,
+ float t_max,
+ const std::vector<std::unique_ptr<CPDF_Function>>& funcs,
+ const RetainPtr<CPDF_ColorSpace>& pCS,
+ int alpha,
+ std::array<FX_ARGB, kShadingSteps>* output) {
+ const uint32_t results_count = GetValidatedOutputsCount(funcs, pCS);
+ if (results_count == 0) {
+ return false;
+ }
+
CHECK_GE(results_count, CountOutputsFromFunctions(funcs));
CHECK_GE(results_count, pCS->ComponentCount());
- std::array<FX_ARGB, kShadingSteps> shading_steps;
+ std::array<FX_ARGB, kShadingSteps>& shading_steps = *output;
std::vector<float> result_array(results_count);
float diff = t_max - t_min;
for (int i = 0; i < kShadingSteps; ++i) {
@@ -92,7 +96,7 @@
ArgbEncode(alpha, FXSYS_roundf(rgb.red * 255),
FXSYS_roundf(rgb.green * 255), FXSYS_roundf(rgb.blue * 255));
}
- return shading_steps;
+ return true;
}
void DrawAxialShading(const RetainPtr<CFX_DIBitmap>& pBitmap,
@@ -103,11 +107,6 @@
int alpha) {
DCHECK_EQ(pBitmap->GetFormat(), FXDIB_Format::kBgra);
- const uint32_t total_results = GetValidatedOutputsCount(funcs, pCS);
- if (total_results == 0) {
- return;
- }
-
RetainPtr<const CPDF_Array> pCoords = dict->GetArrayFor("Coords");
if (!pCoords) {
return;
@@ -134,8 +133,10 @@
float y_span = end_y - start_y;
float axis_len_square = (x_span * x_span) + (y_span * y_span);
- std::array<FX_ARGB, kShadingSteps> shading_steps =
- GetShadingSteps(t_min, t_max, funcs, pCS, alpha, total_results);
+ std::array<FX_ARGB, kShadingSteps> shading_steps;
+ if (!GetShadingSteps(t_min, t_max, funcs, pCS, alpha, &shading_steps)) {
+ return;
+ }
CFX_Matrix matrix = mtObject2Bitmap.GetInverse();
for (int row = 0; row < height; row++) {
@@ -174,11 +175,6 @@
int alpha) {
DCHECK_EQ(pBitmap->GetFormat(), FXDIB_Format::kBgra);
- const uint32_t total_results = GetValidatedOutputsCount(funcs, pCS);
- if (total_results == 0) {
- return;
- }
-
RetainPtr<const CPDF_Array> pCoords = dict->GetArrayFor("Coords");
if (!pCoords) {
return;
@@ -201,8 +197,10 @@
const bool bStartExtend = pArray && pArray->GetBooleanAt(0, false);
const bool bEndExtend = pArray && pArray->GetBooleanAt(1, false);
- std::array<FX_ARGB, kShadingSteps> shading_steps =
- GetShadingSteps(t_min, t_max, funcs, pCS, alpha, total_results);
+ std::array<FX_ARGB, kShadingSteps> shading_steps;
+ if (!GetShadingSteps(t_min, t_max, funcs, pCS, alpha, &shading_steps)) {
+ return;
+ }
const float dx = end_x - start_x;
const float dy = end_y - start_y;
@@ -351,7 +349,8 @@
void DrawGouraud(const RetainPtr<CFX_DIBitmap>& pBitmap,
int alpha,
- pdfium::span<CPDF_MeshVertex, 3> triangle) {
+ pdfium::span<CPDF_MeshVertex, 3> triangle,
+ const std::array<FX_ARGB, kShadingSteps>* shading_steps) {
float min_y = triangle[0].position.y;
float max_y = triangle[0].position.y;
for (int i = 1; i < 3; i++) {
@@ -429,12 +428,22 @@
for (int x = start_x; x < end_x; x++) {
r_result += r_unit;
- g_result += g_unit;
- b_result += b_unit;
- UNSAFE_TODO(FXARGB_SetDIB(
- dib_span.data(), ArgbEncode(alpha, static_cast<int>(r_result * 255),
- static_cast<int>(g_result * 255),
- static_cast<int>(b_result * 255))));
+ if (shading_steps) {
+ int index = static_cast<int32_t>(r_result * (kShadingSteps - 1));
+ if (index < 0) {
+ index = 0;
+ } else if (index >= kShadingSteps) {
+ index = kShadingSteps - 1;
+ }
+ UNSAFE_TODO(FXARGB_SetDIB(dib_span.data(), (*shading_steps)[index]));
+ } else {
+ g_result += g_unit;
+ b_result += b_unit;
+ UNSAFE_TODO(FXARGB_SetDIB(
+ dib_span.data(), ArgbEncode(alpha, static_cast<int>(r_result * 255),
+ static_cast<int>(g_result * 255),
+ static_cast<int>(b_result * 255))));
+ }
dib_span = dib_span.subspan<4u>();
}
}
@@ -450,11 +459,19 @@
DCHECK_EQ(pBitmap->GetFormat(), FXDIB_Format::kBgra);
CPDF_MeshStream stream(kFreeFormGouraudTriangleMeshShading, funcs,
- std::move(pShadingStream), std::move(pCS));
+ std::move(pShadingStream), pCS);
if (!stream.Load()) {
return;
}
+ std::array<FX_ARGB, kShadingSteps> shading_steps;
+ if (!funcs.empty()) {
+ if (!GetShadingSteps(stream.component_min(0), stream.component_max(0),
+ funcs, pCS, alpha, &shading_steps)) {
+ return;
+ }
+ }
+
std::array<CPDF_MeshVertex, 3> triangle;
while (!stream.IsEOF()) {
CPDF_MeshVertex vertex;
@@ -479,7 +496,8 @@
triangle[1] = triangle[2];
triangle[2] = vertex;
}
- DrawGouraud(pBitmap, alpha, triangle);
+ DrawGouraud(pBitmap, alpha, triangle,
+ funcs.empty() ? nullptr : &shading_steps);
}
}
@@ -498,11 +516,19 @@
}
CPDF_MeshStream stream(kLatticeFormGouraudTriangleMeshShading, funcs,
- std::move(pShadingStream), std::move(pCS));
+ std::move(pShadingStream), pCS);
if (!stream.Load()) {
return;
}
+ std::array<FX_ARGB, kShadingSteps> shading_steps;
+ if (!funcs.empty()) {
+ if (!GetShadingSteps(stream.component_min(0), stream.component_max(0),
+ funcs, pCS, alpha, &shading_steps)) {
+ return;
+ }
+ }
+
std::array<std::vector<CPDF_MeshVertex>, 2> vertices;
vertices[0] = stream.ReadVertexRow(mtObject2Bitmap, row_verts);
if (vertices[0].empty()) {
@@ -521,9 +547,11 @@
triangle[0] = vertices[last_index][i];
triangle[1] = vertices[1 - last_index][i - 1];
triangle[2] = vertices[last_index][i - 1];
- DrawGouraud(pBitmap, alpha, triangle);
+ DrawGouraud(pBitmap, alpha, triangle,
+ funcs.empty() ? nullptr : &shading_steps);
triangle[2] = vertices[1 - last_index][i];
- DrawGouraud(pBitmap, alpha, triangle);
+ DrawGouraud(pBitmap, alpha, triangle,
+ funcs.empty() ? nullptr : &shading_steps);
}
last_index = 1 - last_index;
}
@@ -684,7 +712,8 @@
int y_scale,
int left,
int bottom,
- CubicBezierPatch patch) {
+ CubicBezierPatch patch,
+ const std::array<FX_ARGB, kShadingSteps>* shading_steps) {
bool bSmall = patch.IsSmall();
CoonColor div_colors[4];
@@ -726,11 +755,18 @@
if (bNoPathSmooth) {
fill_options.aliased_path = true;
}
- pDevice->DrawPath(
- path, nullptr, nullptr,
- ArgbEncode(alpha, div_colors[0].comp[0], div_colors[0].comp[1],
- div_colors[0].comp[2]),
- 0, fill_options);
+
+ if (shading_steps) {
+ pDevice->DrawPath(path, nullptr, nullptr,
+ (*shading_steps)[div_colors[0].comp[0]], 0,
+ fill_options);
+ } else {
+ pDevice->DrawPath(
+ path, nullptr, nullptr,
+ ArgbEncode(alpha, div_colors[0].comp[0], div_colors[0].comp[1],
+ div_colors[0].comp[2]),
+ 0, fill_options);
+ }
} else {
if (d_bottom < kCoonColorThreshold && d_top < kCoonColorThreshold) {
CubicBezierPatch top_patch;
@@ -738,8 +774,8 @@
patch.SubdivideVertical(top_patch, bottom_patch);
y_scale *= 2;
bottom *= 2;
- Draw(x_scale, y_scale, left, bottom, top_patch);
- Draw(x_scale, y_scale, left, bottom + 1, bottom_patch);
+ Draw(x_scale, y_scale, left, bottom, top_patch, shading_steps);
+ Draw(x_scale, y_scale, left, bottom + 1, bottom_patch, shading_steps);
} else if (d_left < kCoonColorThreshold &&
d_right < kCoonColorThreshold) {
CubicBezierPatch left_patch;
@@ -747,8 +783,8 @@
patch.SubdivideHorizontal(left_patch, right_patch);
x_scale *= 2;
left *= 2;
- Draw(x_scale, y_scale, left, bottom, left_patch);
- Draw(x_scale, y_scale, left + 1, bottom, right_patch);
+ Draw(x_scale, y_scale, left, bottom, left_patch, shading_steps);
+ Draw(x_scale, y_scale, left + 1, bottom, right_patch, shading_steps);
} else {
CubicBezierPatch top_left;
CubicBezierPatch bottom_left;
@@ -759,10 +795,11 @@
y_scale *= 2;
left *= 2;
bottom *= 2;
- Draw(x_scale, y_scale, left, bottom, top_left);
- Draw(x_scale, y_scale, left, bottom + 1, bottom_left);
- Draw(x_scale, y_scale, left + 1, bottom, top_right);
- Draw(x_scale, y_scale, left + 1, bottom + 1, bottom_right);
+ Draw(x_scale, y_scale, left, bottom, top_left, shading_steps);
+ Draw(x_scale, y_scale, left, bottom + 1, bottom_left, shading_steps);
+ Draw(x_scale, y_scale, left + 1, bottom, top_right, shading_steps);
+ Draw(x_scale, y_scale, left + 1, bottom + 1, bottom_right,
+ shading_steps);
}
}
}
@@ -791,12 +828,19 @@
CFX_DefaultRenderDevice device;
device.Attach(pBitmap);
- CPDF_MeshStream stream(type, funcs, std::move(pShadingStream),
- std::move(pCS));
+ CPDF_MeshStream stream(type, funcs, std::move(pShadingStream), pCS);
if (!stream.Load()) {
return;
}
+ std::array<FX_ARGB, kShadingSteps> shading_steps;
+ if (!funcs.empty()) {
+ if (!GetShadingSteps(stream.component_min(0), stream.component_max(0),
+ funcs, pCS, alpha, &shading_steps)) {
+ return;
+ }
+ }
+
PatchDrawer patch_drawer;
patch_drawer.alpha = alpha;
patch_drawer.pDevice = &device;
@@ -908,7 +952,8 @@
1.0f * patch.points[0][0]);
}
- patch_drawer.Draw(1, 1, 0, 0, patch);
+ patch_drawer.Draw(1, 1, 0, 0, patch,
+ funcs.empty() ? nullptr : &shading_steps);
}
}
diff --git a/testing/resources/pixel/bug_1258968_expected.pdf.0.png b/testing/resources/pixel/bug_1258968_expected.pdf.0.png
index 0770f77..a8f1a6b 100644
--- a/testing/resources/pixel/bug_1258968_expected.pdf.0.png
+++ b/testing/resources/pixel/bug_1258968_expected.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png b/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
index 8011bd4..cf8dbae 100644
--- a/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
+++ b/testing/resources/pixel/bug_1258968_expected_skia.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/bug_1546_expected.pdf.0.png b/testing/resources/pixel/bug_1546_expected.pdf.0.png
index e993bf1..8f6a0e0 100644
--- a/testing/resources/pixel/bug_1546_expected.pdf.0.png
+++ b/testing/resources/pixel/bug_1546_expected.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/shade_expected.pdf.0.png b/testing/resources/pixel/shade_expected.pdf.0.png
index e275f97..b881403 100644
--- a/testing/resources/pixel/shade_expected.pdf.0.png
+++ b/testing/resources/pixel/shade_expected.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/shade_expected_gdi_skia.pdf.0.png b/testing/resources/pixel/shade_expected_gdi_skia.pdf.0.png
index e2372c8..3dd71cd 100644
--- a/testing/resources/pixel/shade_expected_gdi_skia.pdf.0.png
+++ b/testing/resources/pixel/shade_expected_gdi_skia.pdf.0.png
Binary files differ
diff --git a/testing/resources/pixel/shade_expected_skia.pdf.0.png b/testing/resources/pixel/shade_expected_skia.pdf.0.png
index b8156c4..b5b2d5d 100644
--- a/testing/resources/pixel/shade_expected_skia.pdf.0.png
+++ b/testing/resources/pixel/shade_expected_skia.pdf.0.png
Binary files differ
