Fix some c++20 warnings
In some of our benchmarks, we use volatile to try to make sure
the compiler doesn't optimize away certain operations. If there is a better way to do that, I'm open to that.
Implicit capturing of this in lambdas has been deprecated due to the
potentially confusing behavior. [1] Changing [=] -> [=, this] won't
work due to the latter being a c++20 extension. However, for many
uses, we can improve the style/readability of the code anyway [2]
by making the captures explicit.
Finally, there was a warning about non-const operator== being ambiguous,
so I fixed those I could find with grep.
[1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0806r2.html
[2] https://google.github.io/styleguide/cppguide.html#Lambda_expressions "Prefer explicit captures when the lambda will escape the current scope."
Bug: b/311238827
Change-Id: I1725adaa2c258e0fdee2fe398647c15d2cbc9966
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/833811
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/bench/ColorPrivBench.cpp b/bench/ColorPrivBench.cpp
index 66e2bbc..a00355b 100644
--- a/bench/ColorPrivBench.cpp
+++ b/bench/ColorPrivBench.cpp
@@ -58,13 +58,13 @@
const unsigned scale = fScales[j];
if (kFast && kScale) {
- junk ^= SkFastFourByteInterp(src, dst, scale);
+ junk = SkFastFourByteInterp(src, dst, scale);
} else if (kFast) {
- junk ^= SkFastFourByteInterp256(src, dst, scale);
+ junk = SkFastFourByteInterp256(src, dst, scale);
} else if (kScale) {
- junk ^= SkFourByteInterp(src, dst, scale);
+ junk = SkFourByteInterp(src, dst, scale);
} else {
- junk ^= SkFourByteInterp256(src, dst, scale);
+ junk = SkFourByteInterp256(src, dst, scale);
}
}
}
diff --git a/bench/ControlBench.cpp b/bench/ControlBench.cpp
index 20713b9..0b7683a 100644
--- a/bench/ControlBench.cpp
+++ b/bench/ControlBench.cpp
@@ -18,8 +18,8 @@
// Nothing terribly useful: force a memory read, a memory write, and some math.
[[maybe_unused]] volatile uint32_t rand = 0;
for (int i = 0; i < 1000*loops; i++) {
- rand *= 1664525;
- rand += 1013904223;
+ uint32_t val = rand * 1664525 + 1013904223;
+ rand = val;
}
}
};
diff --git a/bench/GeometryBench.cpp b/bench/GeometryBench.cpp
index bd7eaa6..23c73ab 100644
--- a/bench/GeometryBench.cpp
+++ b/bench/GeometryBench.cpp
@@ -35,7 +35,9 @@
* needed somewhere, and since this method is not const, the member fields cannot
* be assumed to be const before and after the call.
*/
- virtual void virtualCallToFoilOptimizers(int n) { fVolatileInt += n; }
+ virtual void virtualCallToFoilOptimizers(int n) {
+ fVolatileInt = n;
+ }
private:
SkString fName;
diff --git a/bench/MathBench.cpp b/bench/MathBench.cpp
index 48c730d..d84c938 100644
--- a/bench/MathBench.cpp
+++ b/bench/MathBench.cpp
@@ -594,46 +594,6 @@
///////////////////////////////////////////////////////////////////////////////
-template <typename T>
-class DivModBench : public Benchmark {
- SkString fName;
-public:
- explicit DivModBench(const char* name) {
- fName.printf("divmod_%s", name);
- }
-
- bool isSuitableFor(Backend backend) override {
- return backend == Backend::kNonRendering;
- }
-
-protected:
- const char* onGetName() override {
- return fName.c_str();
- }
-
- void onDraw(int loops, SkCanvas*) override {
- volatile T a = 0, b = 0;
- T div = 0, mod = 0;
- for (int i = 0; i < loops; i++) {
- if ((T)i == 0) continue; // Small T will wrap around.
- SkTDivMod((T)(i+1), (T)i, &div, &mod);
- a ^= div;
- b ^= mod;
- }
- }
-};
-DEF_BENCH(return new DivModBench<uint8_t>("uint8_t"))
-DEF_BENCH(return new DivModBench<uint16_t>("uint16_t"))
-DEF_BENCH(return new DivModBench<uint32_t>("uint32_t"))
-DEF_BENCH(return new DivModBench<uint64_t>("uint64_t"))
-
-DEF_BENCH(return new DivModBench<int8_t>("int8_t"))
-DEF_BENCH(return new DivModBench<int16_t>("int16_t"))
-DEF_BENCH(return new DivModBench<int32_t>("int32_t"))
-DEF_BENCH(return new DivModBench<int64_t>("int64_t"))
-
-///////////////////////////////////////////////////////////////////////////////
-
DEF_BENCH( return new NoOpMathBench(); )
DEF_BENCH( return new SkRSqrtMathBench(); )
DEF_BENCH( return new SlowISqrtMathBench(); )
diff --git a/bench/MatrixBench.cpp b/bench/MatrixBench.cpp
index 1e5a4c2..c3e4945 100644
--- a/bench/MatrixBench.cpp
+++ b/bench/MatrixBench.cpp
@@ -40,28 +40,6 @@
using INHERITED = Benchmark;
};
-
-class EqualsMatrixBench : public MatrixBench {
-public:
- EqualsMatrixBench() : INHERITED("equals") {}
-protected:
- void performTest() override {
- SkMatrix m0, m1, m2;
-
- m0.reset();
- m1.reset();
- m2.reset();
-
- // xor into a volatile prevents these comparisons from being optimized away.
- [[maybe_unused]] volatile bool junk = false;
- junk ^= (m0 == m1);
- junk ^= (m1 == m2);
- junk ^= (m2 == m0);
- }
-private:
- using INHERITED = MatrixBench;
-};
-
class ScaleMatrixBench : public MatrixBench {
public:
ScaleMatrixBench() : INHERITED("scale") {
@@ -93,53 +71,6 @@
}
}
-class GetTypeMatrixBench : public MatrixBench {
-public:
- GetTypeMatrixBench()
- : INHERITED("gettype") {
- fArray[0] = (float) fRnd.nextS();
- fArray[1] = (float) fRnd.nextS();
- fArray[2] = (float) fRnd.nextS();
- fArray[3] = (float) fRnd.nextS();
- fArray[4] = (float) fRnd.nextS();
- fArray[5] = (float) fRnd.nextS();
- fArray[6] = (float) fRnd.nextS();
- fArray[7] = (float) fRnd.nextS();
- fArray[8] = (float) fRnd.nextS();
- }
-protected:
- // Putting random generation of the matrix inside performTest()
- // would help us avoid anomalous runs, but takes up 25% or
- // more of the function time.
- void performTest() override {
- fMatrix.setAll(fArray[0], fArray[1], fArray[2],
- fArray[3], fArray[4], fArray[5],
- fArray[6], fArray[7], fArray[8]);
- // xoring into a volatile prevents the compiler from optimizing these away
- [[maybe_unused]] volatile int junk = 0;
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- fMatrix.dirtyMatrixTypeCache();
- junk ^= (fMatrix.getType());
- }
-private:
- SkMatrix fMatrix;
- float fArray[9];
- SkRandom fRnd;
- using INHERITED = MatrixBench;
-};
-
class DecomposeMatrixBench : public MatrixBench {
public:
DecomposeMatrixBench() : INHERITED("decompose") {}
@@ -225,9 +156,7 @@
///////////////////////////////////////////////////////////////////////////////
-DEF_BENCH( return new EqualsMatrixBench(); )
DEF_BENCH( return new ScaleMatrixBench(); )
-DEF_BENCH( return new GetTypeMatrixBench(); )
DEF_BENCH( return new DecomposeMatrixBench(); )
DEF_BENCH( return new InvertMapRectMatrixBench("invert_maprect_identity", 0); )
diff --git a/bench/ScalarBench.cpp b/bench/ScalarBench.cpp
deleted file mode 100644
index 021b911..0000000
--- a/bench/ScalarBench.cpp
+++ /dev/null
@@ -1,169 +0,0 @@
-/*
- * Copyright 2011 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-#include "bench/Benchmark.h"
-#include "include/core/SkRect.h"
-#include "include/core/SkString.h"
-#include "include/private/base/SkFloatBits.h"
-#include "src/base/SkRandom.h"
-
-class ScalarBench : public Benchmark {
- SkString fName;
-public:
- ScalarBench(const char name[]) {
- fName.printf("scalar_%s", name);
- }
-
- bool isSuitableFor(Backend backend) override {
- return backend == Backend::kNonRendering;
- }
-
- virtual void performTest() = 0;
-
-protected:
- virtual int mulLoopCount() const { return 1; }
-
- const char* onGetName() override {
- return fName.c_str();
- }
-
- void onDraw(int loops, SkCanvas* canvas) override {
- for (int i = 0; i < loops; i++) {
- this->performTest();
- }
- }
-
-private:
- using INHERITED = Benchmark;
-};
-
-// having unknown values in our arrays can throw off the timing a lot, perhaps
-// handling NaN values is a lot slower. Anyway, this is just meant to put
-// reasonable values in our arrays.
-template <typename T> void init9(T array[9]) {
- SkRandom rand;
- for (int i = 0; i < 9; i++) {
- array[i] = rand.nextSScalar1();
- }
-}
-
-class FloatComparisonBench : public ScalarBench {
-public:
- FloatComparisonBench() : INHERITED("compare_float") {
- init9(fArray);
- }
-protected:
- int mulLoopCount() const override { return 4; }
- void performTest() override {
- // xoring into a volatile prevents the compiler from optimizing these checks away.
- [[maybe_unused]] volatile bool junk = false;
- junk ^= (fArray[6] != 0.0f || fArray[7] != 0.0f || fArray[8] != 1.0f);
- junk ^= (fArray[2] != 0.0f || fArray[5] != 0.0f);
- }
-private:
- float fArray[9];
- using INHERITED = ScalarBench;
-};
-
-class ForcedIntComparisonBench : public ScalarBench {
-public:
- ForcedIntComparisonBench()
- : INHERITED("compare_forced_int") {
- init9(fArray);
- }
-protected:
- int mulLoopCount() const override { return 4; }
- void performTest() override {
- // xoring into a volatile prevents the compiler from optimizing these checks away.
- [[maybe_unused]] volatile int32_t junk = 0;
- junk ^= (SkScalarAs2sCompliment(fArray[6]) |
- SkScalarAs2sCompliment(fArray[7]) |
- (SkScalarAs2sCompliment(fArray[8]) - kPersp1Int));
- junk ^= (SkScalarAs2sCompliment(fArray[2]) |
- SkScalarAs2sCompliment(fArray[5]));
- }
-private:
- static const int32_t kPersp1Int = 0x3f800000;
- SkScalar fArray[9];
- using INHERITED = ScalarBench;
-};
-
-class IsFiniteScalarBench : public ScalarBench {
-public:
- IsFiniteScalarBench() : INHERITED("isfinite") {
- SkRandom rand;
- for (size_t i = 0; i < ARRAY_N; ++i) {
- fArray[i] = rand.nextSScalar1();
- }
- }
-protected:
- int mulLoopCount() const override { return 1; }
- void performTest() override {
- int sum = 0;
- for (size_t i = 0; i < ARRAY_N; ++i) {
- // We pass -fArray[i], so the compiler can't cheat and treat the
- // value as an int (even though we tell it that it is a float)
- sum += SkScalarIsFinite(-fArray[i]);
- }
- // we do this so the compiler won't optimize our loop away...
- this->doSomething(fArray, sum);
- }
-
- virtual void doSomething(SkScalar array[], int sum) {}
-private:
- enum {
- ARRAY_N = 64
- };
- SkScalar fArray[ARRAY_N];
-
- using INHERITED = ScalarBench;
-};
-
-///////////////////////////////////////////////////////////////////////////////
-
-class RectBoundsBench : public Benchmark {
- enum {
- PTS = 100,
- };
- SkPoint fPts[PTS];
-
-public:
- RectBoundsBench() {
- SkRandom rand;
- for (int i = 0; i < PTS; ++i) {
- fPts[i].fX = rand.nextSScalar1();
- fPts[i].fY = rand.nextSScalar1();
- }
- }
-
- bool isSuitableFor(Backend backend) override {
- return backend == Backend::kNonRendering;
- }
-
-protected:
- const char* onGetName() override {
- return "rect_bounds";
- }
-
- void onDraw(int loops, SkCanvas* canvas) override {
- SkRect r;
- for (int loop = 0; loop < loops; ++loop) {
- for (int i = 0; i < 1000; ++i) {
- r.setBounds(fPts, PTS);
- }
- }
- }
-
-private:
- using INHERITED = Benchmark;
-};
-
-///////////////////////////////////////////////////////////////////////////////
-
-DEF_BENCH( return new FloatComparisonBench(); )
-DEF_BENCH( return new ForcedIntComparisonBench(); )
-DEF_BENCH( return new RectBoundsBench(); )
-DEF_BENCH( return new IsFiniteScalarBench(); )
diff --git a/fuzz/FuzzDDLThreading.cpp b/fuzz/FuzzDDLThreading.cpp
index 9b477876..0179487 100644
--- a/fuzz/FuzzDDLThreading.cpp
+++ b/fuzz/FuzzDDLThreading.cpp
@@ -266,7 +266,7 @@
canvas->drawImage(fPromiseImages[j].fImage, xOffset, 0);
}
sk_sp<GrDeferredDisplayList> ddl = recorder.detach();
- fGpuTaskGroup.add([=, ddl{std::move(ddl)}]{
+ fGpuTaskGroup.add([ddl{std::move(ddl)}, this] {
bool success = skgpu::ganesh::DrawDDL(fSurface, std::move(ddl));
if (!success) {
fFuzz->signalBug();
@@ -278,16 +278,14 @@
if (!fSurface) {
return;
}
- fRecordingTaskGroup.batch(kIterationCount, [=](int i) {
- this->recordAndPlayDDL();
- });
+ fRecordingTaskGroup.batch(kIterationCount, [this](int i) { this->recordAndPlayDDL(); });
fRecordingTaskGroup.wait();
- fGpuTaskGroup.add([=] { fContext->flushAndSubmit(fSurface.get(), GrSyncCpu::kYes); });
+ fGpuTaskGroup.add([this] { fContext->flushAndSubmit(fSurface.get(), GrSyncCpu::kYes); });
fGpuTaskGroup.wait();
- fGpuTaskGroup.add([=] {
+ fGpuTaskGroup.add([this] {
while (!fReusableTextures.empty()) {
sk_sp<GrPromiseImageTexture> gpuTexture = std::move(fReusableTextures.front());
fContext->deleteBackendTexture(gpuTexture->backendTexture());
diff --git a/gn/bench.gni b/gn/bench.gni
index 7b26afa..64e928c 100644
--- a/gn/bench.gni
+++ b/gn/bench.gni
@@ -119,7 +119,6 @@
"$_bench/SKPAnimationBench.h",
"$_bench/SKPBench.cpp",
"$_bench/SKPBench.h",
- "$_bench/ScalarBench.cpp",
"$_bench/ShaderMaskFilterBench.cpp",
"$_bench/ShadowBench.cpp",
"$_bench/ShapesBench.cpp",
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 6ed7659..ea0a98f 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2274,15 +2274,13 @@
};
SkPathConvexity SkPath::computeConvexity() const {
- auto setComputedConvexity = [=](SkPathConvexity convexity){
+ auto setComputedConvexity = [&](SkPathConvexity convexity) {
SkASSERT(SkPathConvexity::kUnknown != convexity);
this->setConvexity(convexity);
return convexity;
};
- auto setFail = [=](){
- return setComputedConvexity(SkPathConvexity::kConcave);
- };
+ auto setFail = [&]() { return setComputedConvexity(SkPathConvexity::kConcave); };
if (!this->isFinite()) {
return setFail();
diff --git a/src/core/SkTaskGroup.cpp b/src/core/SkTaskGroup.cpp
index 672756e..cd5c50f 100644
--- a/src/core/SkTaskGroup.cpp
+++ b/src/core/SkTaskGroup.cpp
@@ -25,7 +25,7 @@
// TODO: I really thought we had some sort of more clever chunking logic.
fPending.fetch_add(+N, std::memory_order_relaxed);
for (int i = 0; i < N; i++) {
- fExecutor.add([=] {
+ fExecutor.add([fn, i, this] {
fn(i);
fPending.fetch_add(-1, std::memory_order_release);
});
diff --git a/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp b/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp
index ddf4448..8bdd277 100644
--- a/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp
+++ b/src/gpu/ganesh/glsl/GrGLSLShaderBuilder.cpp
@@ -150,8 +150,9 @@
// function, one for the (inverse) destination transfer function, and one for the gamut xform.
// Any combination of these may be present, although some configurations are much more likely.
- auto emitTFFunc = [=](const char* name, GrGLSLProgramDataManager::UniformHandle uniform,
- skcms_TFType tfType) {
+ auto emitTFFunc = [this, &uniformHandler](const char* name,
+ GrGLSLProgramDataManager::UniformHandle uniform,
+ skcms_TFType tfType) {
const GrShaderVar gTFArgs[] = { GrShaderVar("x", SkSLType::kFloat) };
const char* coeffs = uniformHandler->getUniformCStr(uniform);
SkString body;
diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index 579434b..cfa9986 100644
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -36,16 +36,16 @@
struct SkPDFIndirectReference {
int fValue = -1;
explicit operator bool() const { return fValue != -1; }
+
+ bool operator==(SkPDFIndirectReference v) const {
+ return fValue == v.fValue;
+ }
+
+ bool operator!=(SkPDFIndirectReference v) const {
+ return fValue != v.fValue;
+ }
};
-inline static bool operator==(SkPDFIndirectReference u, SkPDFIndirectReference v) {
- return u.fValue == v.fValue;
-}
-
-inline static bool operator!=(SkPDFIndirectReference u, SkPDFIndirectReference v) {
- return u.fValue != v.fValue;
-}
-
/** \class SkPDFObject
A PDF Object is the base class for primitive elements in a PDF file. A
diff --git a/src/ports/SkFontMgr_fuchsia.cpp b/src/ports/SkFontMgr_fuchsia.cpp
index d752f50..e37a277 100644
--- a/src/ports/SkFontMgr_fuchsia.cpp
+++ b/src/ports/SkFontMgr_fuchsia.cpp
@@ -221,7 +221,7 @@
uint32_t bufferId;
uint32_t ttcIndex;
- bool operator==(TypefaceId& other) {
+ bool operator==(TypefaceId& other) const {
return std::tie(bufferId, ttcIndex) == std::tie(other.bufferId, other.ttcIndex);
}
}
diff --git a/tests/TArrayTest.cpp b/tests/TArrayTest.cpp
index 06143ef..ec4be36 100644
--- a/tests/TArrayTest.cpp
+++ b/tests/TArrayTest.cpp
@@ -199,7 +199,7 @@
struct MoveOnlyInt {
MoveOnlyInt(int i) : fInt(i) {}
MoveOnlyInt(MoveOnlyInt&& that) : fInt(that.fInt) {}
- bool operator==(int i) { return fInt == i; }
+ bool operator==(int i) const { return fInt == i; }
int fInt;
};
} // anonymous
diff --git a/tools/viewer/Viewer.cpp b/tools/viewer/Viewer.cpp
index fd629e8..f60edb0 100644
--- a/tools/viewer/Viewer.cpp
+++ b/tools/viewer/Viewer.cpp
@@ -1719,16 +1719,6 @@
break;
}
- auto make_surface = [=](int w, int h) {
- SkSurfaceProps props(fWindow->getRequestedDisplayParams().fSurfaceProps);
- slideCanvas->getProps(&props);
-
- SkImageInfo info = SkImageInfo::Make(w, h, colorType, kPremul_SkAlphaType, colorSpace);
- return Window::kRaster_BackendType == this->fBackendType
- ? SkSurfaces::Raster(info, &props)
- : slideCanvas->makeSurface(info, &props);
- };
-
// We need to render offscreen if we're...
// ... in fake perspective or zooming (so we have a snapped copy of the results)
// ... in any raster mode, because the window surface is actually GL
@@ -1741,8 +1731,15 @@
Window::kRaster_BackendType == fBackendType ||
colorSpace != nullptr ||
FLAGS_offscreen) {
+ SkSurfaceProps props(fWindow->getRequestedDisplayParams().fSurfaceProps);
+ slideCanvas->getProps(&props);
- offscreenSurface = make_surface(fWindow->width(), fWindow->height());
+ SkImageInfo info = SkImageInfo::Make(
+ fWindow->width(), fWindow->height(), colorType, kPremul_SkAlphaType, colorSpace);
+ offscreenSurface = Window::kRaster_BackendType == this->fBackendType
+ ? SkSurfaces::Raster(info, &props)
+ : slideCanvas->makeSurface(info, &props);
+
slideSurface = offscreenSurface.get();
slideCanvas = offscreenSurface->getCanvas();
}
@@ -2135,7 +2132,7 @@
ImGui::RadioButton("Direct3D", &newBackend, sk_app::Window::kDirect3D_BackendType);
#endif
if (newBackend != fBackendType) {
- fDeferredActions.push_back([=]() {
+ fDeferredActions.push_back([newBackend, this]() {
this->setBackend(static_cast<sk_app::Window::BackendType>(newBackend));
});
}
@@ -2821,7 +2818,7 @@
: GrContextOptions::ShaderCacheStrategy::kBackendSource;
displayParamsChanged = true;
- fDeferredActions.push_back([=]() {
+ fDeferredActions.push_back([doDump, this]() {
// Reset the cache.
fPersistentCache.reset();
sDoDeferredView = true;
@@ -2919,7 +2916,7 @@
}
}
if (displayParamsChanged || uiParamsChanged) {
- fDeferredActions.push_back([=]() {
+ fDeferredActions.push_back([displayParamsChanged, params, this]() {
if (displayParamsChanged) {
fWindow->setRequestedDisplayParams(params);
}
