Improve SkVerticesPriv ergonomics
Rather than two separate (partially overlapping) ways of accessing the
private portions of SkVertices, use a single privileged helper class
(similar to GrContextPriv).
Change-Id: I76b14b63088658ed8726719cce126577e5a52078
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/280601
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/gm/3d.cpp b/gm/3d.cpp
index 0625042..99e65e9 100644
--- a/gm/3d.cpp
+++ b/gm/3d.cpp
@@ -7,6 +7,7 @@
#include "gm/gm.h"
#include "include/core/SkCanvas.h"
+#include "include/core/SkData.h"
struct Info {
float fNear = 0.05f;
diff --git a/gm/vertices.cpp b/gm/vertices.cpp
index 715ee0d..196e504 100644
--- a/gm/vertices.cpp
+++ b/gm/vertices.cpp
@@ -352,17 +352,16 @@
{256, 256}, {171, 256}, {85, 256}, {0, 256}, {0, 171}, {0, 85}};
auto patchVerts = SkPatchUtils::MakeVertices(pts, nullptr, nullptr, 12, 12);
- SkVertices::Info info;
- patchVerts->getInfo(&info);
+ SkVerticesPriv pv(patchVerts->priv());
SkVertices::CustomLayout customLayout { 1 };
- SkVertices::Builder builder(info.fMode, info.fVertexCount, info.fIndexCount, customLayout);
+ SkVertices::Builder builder(pv.mode(), pv.vertexCount(), pv.indexCount(), customLayout);
- memcpy(builder.positions(), info.fPositions, info.fVertexCount * sizeof(SkPoint));
- memcpy(builder.indices(), info.fIndices, info.fIndexCount * sizeof(uint16_t));
+ memcpy(builder.positions(), pv.positions(), pv.vertexCount() * sizeof(SkPoint));
+ memcpy(builder.indices(), pv.indices(), pv.indexCount() * sizeof(uint16_t));
SkRandom rnd;
- for (int i = 0; i < info.fVertexCount; ++i) {
+ for (int i = 0; i < pv.vertexCount(); ++i) {
builder.perVertexData()[i] = rnd.nextBool() ? 1.0f : 0.0f;
}
diff --git a/include/core/SkVertices.h b/include/core/SkVertices.h
index f789cc8..2a4c299 100644
--- a/include/core/SkVertices.h
+++ b/include/core/SkVertices.h
@@ -9,11 +9,13 @@
#define SkVertices_DEFINED
#include "include/core/SkColor.h"
-#include "include/core/SkData.h"
-#include "include/core/SkPoint.h"
#include "include/core/SkRect.h"
#include "include/core/SkRefCnt.h"
+class SkData;
+struct SkPoint;
+class SkVerticesPriv;
+
/**
* An immutable set of vertex data that can be used with SkCanvas::drawVertices.
*/
@@ -120,23 +122,20 @@
*/
sk_sp<SkData> encode() const;
- struct Info;
- void getInfo(Info*) const;
+ // Provides access to functions that aren't part of the public API.
+ SkVerticesPriv priv();
+ const SkVerticesPriv priv() const;
private:
SkVertices() {}
friend class SkVerticesPriv;
friend class SkDraw;
- friend class SkGpuDevice;
// these are needed since we've manually sized our allocation (see Builder::init)
friend class SkNVRefCnt<SkVertices>;
void operator delete(void* p);
- static sk_sp<SkVertices> Alloc(int vCount, int iCount, uint32_t builderFlags,
- size_t* arraySize);
-
Sizes getSizes() const;
VertexMode mode() const { return fMode; }
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index c6a25d6..85a1bcb 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1986,16 +1986,13 @@
TRACE_EVENT0("skia", TRACE_FUNC);
RETURN_ON_NULL(vertices);
- SkVertices::Info info;
- vertices->getInfo(&info);
-
// We expect fans to be converted to triangles when building or deserializing SkVertices.
- SkASSERT(info.fMode != SkVertices::kTriangleFan_VertexMode);
+ SkASSERT(vertices->priv().mode() != SkVertices::kTriangleFan_VertexMode);
// If the vertices contain custom attributes, ensure they line up with the paint's shader
const SkRuntimeEffect* effect =
paint.getShader() ? as_SB(paint.getShader())->asRuntimeEffect() : nullptr;
- if (info.fPerVertexDataCount != (effect ? effect->varyingCount() : 0)) {
+ if (vertices->priv().perVertexDataCount() != (effect ? effect->varyingCount() : 0)) {
return;
}
diff --git a/src/core/SkVertices.cpp b/src/core/SkVertices.cpp
index 94fc5b3..f2abaad 100644
--- a/src/core/SkVertices.cpp
+++ b/src/core/SkVertices.cpp
@@ -415,21 +415,6 @@
return builder.detach();
}
-void SkVertices::operator delete(void* p)
-{
+void SkVertices::operator delete(void* p) {
::operator delete(p);
}
-
-void SkVertices::getInfo(Info* info) const {
- info->fMode = fMode;
-
- info->fVertexCount = fVertexCount;
- info->fIndexCount = fIndexCount;
- info->fPerVertexDataCount = fPerVertexDataCount;
-
- info->fPositions = fPositions;
- info->fIndices = fIndices;
- info->fTexCoords = fTexs;
- info->fColors = fColors;
- info->fPerVertexData = fPerVertexData;
-}
diff --git a/src/core/SkVerticesPriv.h b/src/core/SkVerticesPriv.h
index 5366188..fd7cbef 100644
--- a/src/core/SkVerticesPriv.h
+++ b/src/core/SkVerticesPriv.h
@@ -14,38 +14,46 @@
struct SkVertices_DeprecatedBoneWeights { float values[4]; };
struct SkVertices_DeprecatedBone { float values[6]; };
-struct SkVertices::Info {
- VertexMode fMode;
- bool fIsVolatile;
-
- int fVertexCount,
- fIndexCount,
- fPerVertexDataCount;
-
- const SkPoint* fPositions;
- const uint16_t* fIndices;
-
- // old version
- const SkPoint* fTexCoords; // may be null
- const SkColor* fColors; // may be null
-
- // new, per-vertex-data, version
- const float* fPerVertexData;
-
- bool hasTexCoords() const { return fTexCoords != nullptr; }
- bool hasColors() const { return fColors != nullptr; }
- bool hasPerVertexData() const { return fPerVertexData != nullptr; }
-};
-
+/** Class that adds methods to SkVertices that are only intended for use internal to Skia.
+ This class is purely a privileged window into SkVertices. It should never have additional
+ data members or virtual methods. */
class SkVerticesPriv {
public:
- static int VertexCount(const SkVertices* v) { return v->fVertexCount; }
+ SkVertices::VertexMode mode() const { return fVertices->fMode; }
- static bool HasTexCoords(const SkVertices* v) { return v->fTexs != nullptr; }
- static bool HasColors(const SkVertices* v) { return v->fColors != nullptr; }
- static bool HasIndices(const SkVertices* v) { return v->fIndices != nullptr; }
+ bool hasColors() const { return fVertices->hasColors(); }
+ bool hasTexCoords() const { return fVertices->hasTexCoords(); }
+ bool hasIndices() const { return fVertices->hasIndices(); }
+ bool hasPerVertexData() const { return fVertices->hasPerVertexData(); }
- static SkVertices::VertexMode Mode(const SkVertices* v) { return v->fMode; }
+ int vertexCount() const { return fVertices->fVertexCount; }
+ int indexCount() const { return fVertices->fIndexCount; }
+ int perVertexDataCount() const { return fVertices->fPerVertexDataCount; }
+
+ const SkPoint* positions() const { return fVertices->fPositions; }
+ const float* perVertexData() const { return fVertices->fPerVertexData; }
+ const SkPoint* texCoords() const { return fVertices->fTexs; }
+ const SkColor* colors() const { return fVertices->fColors; }
+ const uint16_t* indices() const { return fVertices->fIndices; }
+
+private:
+ explicit SkVerticesPriv(SkVertices* vertices) : fVertices(vertices) {}
+ SkVerticesPriv(const SkVerticesPriv&) = delete;
+ SkVerticesPriv& operator=(const SkVerticesPriv&) = delete;
+
+ // No taking addresses of this type
+ const SkVerticesPriv* operator&() const = delete;
+ SkVerticesPriv* operator&() = delete;
+
+ SkVertices* fVertices;
+
+ friend class SkVertices; // to construct this type
};
+inline SkVerticesPriv SkVertices::priv() { return SkVerticesPriv(this); }
+
+inline const SkVerticesPriv SkVertices::priv() const {
+ return SkVerticesPriv(const_cast<SkVertices*>(this));
+}
+
#endif
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 6ce878d..cceefdb 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1034,12 +1034,11 @@
GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice", "drawVertices", fContext.get());
SkASSERT(vertices);
- SkVertices::Info info;
- vertices->getInfo(&info);
+ SkVerticesPriv info(vertices->priv());
const SkRuntimeEffect* effect =
paint.getShader() ? as_SB(paint.getShader())->asRuntimeEffect() : nullptr;
- SkASSERT(info.fPerVertexDataCount == (effect ? effect->varyingCount() : 0));
+ SkASSERT(info.perVertexDataCount() == (effect ? effect->varyingCount() : 0));
// Pretend that we have tex coords when using custom per-vertex data. The shader is going to
// use those (rather than local coords), but our paint conversion remains the same.
@@ -1048,9 +1047,8 @@
bool hasTexs = info.hasTexCoords() || info.hasPerVertexData();
if ((!hasTexs || !paint.getShader()) && !hasColors) {
// The dreaded wireframe mode. Fallback to drawVertices and go so slooooooow.
- this->wireframeVertices(vertices->mode(), vertices->vertexCount(), vertices->positions(),
- mode, vertices->indices(), vertices->indexCount(),
- paint);
+ this->wireframeVertices(info.mode(), info.vertexCount(), info.positions(), mode,
+ info.indices(), info.indexCount(), paint);
return;
}
if (!init_vertices_paint(fContext.get(), fRenderTargetContext->colorInfo(), paint,
diff --git a/src/gpu/ops/GrDrawVerticesOp.cpp b/src/gpu/ops/GrDrawVerticesOp.cpp
index d5d464b..87c15ae 100644
--- a/src/gpu/ops/GrDrawVerticesOp.cpp
+++ b/src/gpu/ops/GrDrawVerticesOp.cpp
@@ -301,13 +301,13 @@
bool fIgnoreColors;
bool hasPerVertexColors() const {
- return SkVerticesPriv::HasColors(fVertices.get()) && !fIgnoreColors;
+ return fVertices->priv().hasColors() && !fIgnoreColors;
}
};
bool isIndexed() const {
// Consistency enforced in onCombineIfPossible.
- return SkVerticesPriv::HasIndices(fMeshes[0].fVertices.get());
+ return fMeshes[0].fVertices->priv().hasIndices();
}
bool requiresPerVertexColors() const {
@@ -319,13 +319,10 @@
}
size_t vertexStride() const {
- SkVertices::Info info;
- fMeshes[0].fVertices->getInfo(&info);
-
return sizeof(SkPoint) +
(this->requiresPerVertexColors() ? sizeof(uint32_t) : 0) +
(this->requiresPerVertexLocalCoords() ? sizeof(SkPoint) : 0) +
- info.fPerVertexDataCount * sizeof(float);
+ fMeshes[0].fVertices->priv().perVertexDataCount() * sizeof(float);
}
Helper fHelper;
@@ -358,23 +355,22 @@
, fColorSpaceXform(std::move(colorSpaceXform)) {
SkASSERT(vertices);
- SkVertices::Info info;
- vertices->getInfo(&info);
+ SkVerticesPriv info(vertices->priv());
- fVertexCount = info.fVertexCount;
- fIndexCount = info.fIndexCount;
+ fVertexCount = info.vertexCount();
+ fIndexCount = info.indexCount();
fColorArrayType = info.hasColors() ? ColorArrayType::kSkColor
: ColorArrayType::kUnused;
fLocalCoordsType = info.hasTexCoords() ? LocalCoordsType::kExplicit
: LocalCoordsType::kUsePosition;
if (effect) {
- SkASSERT(info.fPerVertexDataCount == effect->varyingCount());
+ SkASSERT(info.perVertexDataCount() == effect->varyingCount());
for (const auto& v : effect->varyings()) {
fAttrWidths.push_back(v.fWidth);
}
} else {
- SkASSERT(info.fPerVertexDataCount == 0);
+ SkASSERT(info.perVertexDataCount() == 0);
}
Mesh& mesh = fMeshes.push_back();
@@ -483,22 +479,23 @@
int vertexOffset = 0;
for (const auto& mesh : fMeshes) {
- SkVertices::Info info;
- mesh.fVertices->getInfo(&info);
+ SkVerticesPriv info(mesh.fVertices->priv());
+
// Copy data into the index buffer.
if (indices) {
- int indexCount = info.fIndexCount;
+ int indexCount = info.indexCount();
for (int i = 0; i < indexCount; ++i) {
- *indices++ = info.fIndices[i] + vertexOffset;
+ *indices++ = info.indices()[i] + vertexOffset;
}
}
// Copy data into the vertex buffer.
- int vertexCount = info.fVertexCount;
- const SkPoint* positions = info.fPositions;
- const SkColor* colors = info.fColors;
- const SkPoint* localCoords = info.fTexCoords ? info.fTexCoords : positions;
- const float* custom = info.fPerVertexData;
+ int vertexCount = info.vertexCount();
+ const SkPoint* positions = info.positions();
+ const SkColor* colors = info.colors();
+ const SkPoint* localCoords = info.texCoords() ? info.texCoords() : positions;
+ const float* custom = info.perVertexData();
+ int customCount = info.perVertexDataCount();
// TODO4F: Preserve float colors
GrColor meshColor = mesh.fColor.toBytes_RGBA();
@@ -513,7 +510,7 @@
if (hasLocalCoordsAttribute) {
verts.write(localCoords[i]);
}
- for (int j = 0; j < info.fPerVertexDataCount; ++j) {
+ for (int j = 0; j < customCount; ++j) {
verts.write(*custom++);
}
}
@@ -643,8 +640,8 @@
const SkRuntimeEffect* effect) {
SkASSERT(vertices);
GrPrimitiveType primType = overridePrimType
- ? *overridePrimType
- : SkVertexModeToGrPrimitiveType(SkVerticesPriv::Mode(vertices.get()));
+ ? *overridePrimType
+ : SkVertexModeToGrPrimitiveType(vertices->priv().mode());
return GrSimpleMeshDrawOpHelper::FactoryHelper<DrawVerticesOp>(context, std::move(paint),
std::move(vertices),
primType, aaType,
diff --git a/src/pdf/SkDocument_PDF_None.cpp b/src/pdf/SkDocument_PDF_None.cpp
index 1b482ef..9593a5b 100644
--- a/src/pdf/SkDocument_PDF_None.cpp
+++ b/src/pdf/SkDocument_PDF_None.cpp
@@ -6,6 +6,7 @@
*/
#include "include/core/SkCanvas.h"
+#include "include/core/SkData.h"
#include "include/docs/SkPDFDocument.h"
class SkPDFArray {};
diff --git a/src/utils/SkShadowUtils.cpp b/src/utils/SkShadowUtils.cpp
index 73005fe..9d272e1 100644
--- a/src/utils/SkShadowUtils.cpp
+++ b/src/utils/SkShadowUtils.cpp
@@ -589,7 +589,7 @@
void SkBaseDevice::drawShadow(const SkPath& path, const SkDrawShadowRec& rec) {
auto drawVertsProc = [this](const SkVertices* vertices, SkBlendMode mode, const SkPaint& paint,
SkScalar tx, SkScalar ty, bool hasPerspective) {
- if (SkVerticesPriv::VertexCount(vertices)) {
+ if (vertices->priv().vertexCount()) {
// For perspective shadows we've already computed the shadow in world space,
// and we can't translate it without changing it. Otherwise we concat the
// change in translation from the cached version.
diff --git a/tests/ShadowTest.cpp b/tests/ShadowTest.cpp
index 20bb826..8594010 100644
--- a/tests/ShadowTest.cpp
+++ b/tests/ShadowTest.cpp
@@ -26,9 +26,9 @@
expectSuccess ? "succeed" : "fail");
}
if (SkToBool(verts)) {
- if (kDont_ExpectVerts == expectVerts && SkVerticesPriv::VertexCount(verts.get())) {
+ if (kDont_ExpectVerts == expectVerts && verts->priv().vertexCount()) {
ERRORF(reporter, "Expected shadow tessellation to generate no vertices but it did.");
- } else if (kDo_ExpectVerts == expectVerts && !SkVerticesPriv::VertexCount(verts.get())) {
+ } else if (kDo_ExpectVerts == expectVerts && !verts->priv().vertexCount()) {
ERRORF(reporter, "Expected shadow tessellation to generate vertices but it didn't.");
}
}
diff --git a/tests/VerticesTest.cpp b/tests/VerticesTest.cpp
index 399bff2..7a7bff3 100644
--- a/tests/VerticesTest.cpp
+++ b/tests/VerticesTest.cpp
@@ -13,56 +13,54 @@
#include "tools/ToolUtils.h"
static bool equal(const SkVertices* vert0, const SkVertices* vert1) {
- SkVertices::Info v0, v1;
- vert0->getInfo(&v0);
- vert1->getInfo(&v1);
+ SkVerticesPriv v0(vert0->priv()), v1(vert1->priv());
- if (v0.fMode != v1.fMode) {
+ if (v0.mode() != v1.mode()) {
return false;
}
- if (v0.fVertexCount != v1.fVertexCount) {
+ if (v0.vertexCount() != v1.vertexCount()) {
return false;
}
- if (v0.fIndexCount != v1.fIndexCount) {
+ if (v0.indexCount() != v1.indexCount()) {
return false;
}
- if (v0.fPerVertexDataCount != v1.fPerVertexDataCount) {
+ if (v0.perVertexDataCount() != v1.perVertexDataCount()) {
return false;
}
- if (!!v0.fPerVertexData != !!v1.fPerVertexData) {
+ if (!!v0.perVertexData() != !!v1.perVertexData()) {
return false;
}
- if (!!v0.fTexCoords != !!v1.fTexCoords) {
+ if (!!v0.texCoords() != !!v1.texCoords()) {
return false;
}
- if (!!v0.fColors != !!v1.fColors) {
+ if (!!v0.colors() != !!v1.colors()) {
return false;
}
- for (int i = 0; i < v0.fVertexCount; ++i) {
- if (v0.fPositions[i] != v1.fPositions[i]) {
+ for (int i = 0; i < v0.vertexCount(); ++i) {
+ if (v0.positions()[i] != v1.positions()[i]) {
return false;
}
- if (v0.fTexCoords) {
- if (v0.fTexCoords[i] != v1.fTexCoords[i]) {
+ if (v0.texCoords()) {
+ if (v0.texCoords()[i] != v1.texCoords()[i]) {
return false;
}
}
- if (v0.fColors) {
- if (v0.fColors[i] != v1.fColors[i]) {
+ if (v0.colors()) {
+ if (v0.colors()[i] != v1.colors()[i]) {
return false;
}
}
}
- int totalVertexDataCount = v0.fVertexCount * v0.fPerVertexDataCount;
+ int totalVertexDataCount = v0.vertexCount() * v0.perVertexDataCount();
for (int i = 0; i < totalVertexDataCount; ++i) {
- if (v0.fPerVertexData[i] != v1.fPerVertexData[i]) {
+ if (v0.perVertexData()[i] != v1.perVertexData()[i]) {
return false;
}
}
- for (int i = 0; i < v0.fIndexCount; ++i) {
- if (v0.fIndices[i] != v1.fIndices[i]) {
+ for (int i = 0; i < v0.indexCount(); ++i) {
+ if (v0.indices()[i] != v1.indices()[i]) {
return false;
}
}
