From 4427d1eebf344c5962cd47b0e2fcddaf0e877a99 Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Mon, 25 Oct 2021 22:32:34 -0400 Subject: [PATCH] Remove some uses of direct fPtr access in VertexWriter. Ideally we could move fPtr on VertexWriter to be private to force all users of VertexWriter to go through the write functions. This CL at least removes all users that were using fPtr for validity checking of the VertexWriter. I also move fPtr to private and added a temporary public getter. This will allow us to make a base Writer class where fPtr is private and we'll only expose the getter on the VertexWriter. Change-Id: Ib389778d0f530fb31cef85460f00bd9f687c5219 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463456 Commit-Queue: Greg Daniel Auto-Submit: Greg Daniel Reviewed-by: Chris Dalton --- src/gpu/VertexWriter.h | 13 +++++++++---- src/gpu/geometry/GrTriangulator.cpp | 2 +- src/gpu/ops/AAConvexPathRenderer.cpp | 2 +- src/gpu/ops/DashOp.cpp | 2 +- src/gpu/ops/DrawVerticesOp.cpp | 4 ++-- src/gpu/ops/GrOvalOpFactory.cpp | 12 ++++++------ src/gpu/ops/LatticeOp.cpp | 2 +- src/gpu/ops/QuadPerEdgeAA.cpp | 2 +- src/gpu/ops/QuadPerEdgeAA.h | 2 +- src/gpu/ops/RegionOp.cpp | 2 +- src/gpu/ops/SmallPathRenderer.cpp | 2 +- src/gpu/ops/StrokeRectOp.cpp | 2 +- 12 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/gpu/VertexWriter.h b/src/gpu/VertexWriter.h index da051c3a0f..758623d765 100644 --- a/src/gpu/VertexWriter.h +++ b/src/gpu/VertexWriter.h @@ -26,8 +26,6 @@ namespace skgpu { struct VertexWriter { inline constexpr static uint32_t kIEEE_32_infinity = 0x7f800000; - void* fPtr; - VertexWriter() = default; VertexWriter(void* ptr) : fPtr(ptr) {} VertexWriter(const VertexWriter&) = delete; @@ -43,6 +41,11 @@ struct VertexWriter { bool operator==(const VertexWriter& that) const { return fPtr == that.fPtr; } operator bool() const { return fPtr != nullptr; } + // TODO: Remove this call. We want all users of VertexWriter to have to go through the vertex + // writer functions to write data. We do not want them to directly access fPtr and copy their + // own data. + void* ptr() const { return fPtr; } + VertexWriter makeOffset(ptrdiff_t offsetInBytes) const { return {SkTAddOffset(fPtr, offsetInBytes)}; } @@ -162,12 +165,14 @@ private: template void writeQuadVertex() {} + + void* fPtr; }; template inline VertexWriter& operator<<(VertexWriter& w, const T& val) { static_assert(std::is_pod::value, ""); - memcpy(w.fPtr, &val, sizeof(T)); + memcpy(w.ptr(), &val, sizeof(T)); w = w.makeOffset(sizeof(T)); return w; } @@ -189,7 +194,7 @@ inline VertexWriter& operator<<(VertexWriter& w, const VertexWriter::Skip& va template <> SK_MAYBE_UNUSED inline VertexWriter& operator<<(VertexWriter& w, const Sk4f& vector) { - vector.store(w.fPtr); + vector.store(w.ptr()); w = w.makeOffset(sizeof(vector)); return w; } diff --git a/src/gpu/geometry/GrTriangulator.cpp b/src/gpu/geometry/GrTriangulator.cpp index a26d76eb03..e3cb8f024b 100644 --- a/src/gpu/geometry/GrTriangulator.cpp +++ b/src/gpu/geometry/GrTriangulator.cpp @@ -88,7 +88,7 @@ static inline void* emit_vertex(Vertex* v, bool emitCoverage, void* data) { verts << GrNormalizeByteToFloat(v->fAlpha); } - return verts.fPtr; + return verts.ptr(); } static void* emit_triangle(Vertex* v0, Vertex* v1, Vertex* v2, bool emitCoverage, void* data) { diff --git a/src/gpu/ops/AAConvexPathRenderer.cpp b/src/gpu/ops/AAConvexPathRenderer.cpp index e6c77d6640..97f573fa05 100644 --- a/src/gpu/ops/AAConvexPathRenderer.cpp +++ b/src/gpu/ops/AAConvexPathRenderer.cpp @@ -807,7 +807,7 @@ private: &vertexBuffer, &firstVertex)}; - if (!verts.fPtr) { + if (!verts) { SkDebugf("Could not allocate vertices\n"); return; } diff --git a/src/gpu/ops/DashOp.cpp b/src/gpu/ops/DashOp.cpp index fe1da0ecf0..731adcd008 100644 --- a/src/gpu/ops/DashOp.cpp +++ b/src/gpu/ops/DashOp.cpp @@ -564,7 +564,7 @@ private: QuadHelper helper(target, fProgramInfo->geomProc().vertexStride(), totalRectCount); VertexWriter vertices{ helper.vertices() }; - if (!vertices.fPtr) { + if (!vertices) { return; } diff --git a/src/gpu/ops/DrawVerticesOp.cpp b/src/gpu/ops/DrawVerticesOp.cpp index acd8828979..3275bca8da 100644 --- a/src/gpu/ops/DrawVerticesOp.cpp +++ b/src/gpu/ops/DrawVerticesOp.cpp @@ -405,7 +405,7 @@ void DrawVerticesOpImpl::onPrepareDraws(GrMeshDrawTarget* target) { int firstVertex = 0; VertexWriter verts{ target->makeVertexSpace(vertexStride, fVertexCount, &vertexBuffer, &firstVertex)}; - if (!verts.fPtr) { + if (!verts) { SkDebugf("Could not allocate vertices\n"); return; } @@ -446,7 +446,7 @@ void DrawVerticesOpImpl::onPrepareDraws(GrMeshDrawTarget* target) { // TODO4F: Preserve float colors GrColor meshColor = mesh.fColor.toBytes_RGBA(); - SkPoint* posBase = (SkPoint*)verts.fPtr; + SkPoint* posBase = (SkPoint*)verts.ptr(); for (int i = 0; i < vertexCount; ++i) { verts << positions[i]; diff --git a/src/gpu/ops/GrOvalOpFactory.cpp b/src/gpu/ops/GrOvalOpFactory.cpp index 93051f0c1c..f7aa29ca7a 100644 --- a/src/gpu/ops/GrOvalOpFactory.cpp +++ b/src/gpu/ops/GrOvalOpFactory.cpp @@ -1281,7 +1281,7 @@ private: int firstVertex; VertexWriter vertices{target->makeVertexSpace(fProgramInfo->geomProc().vertexStride(), fVertCount, &vertexBuffer, &firstVertex)}; - if (!vertices.fPtr) { + if (!vertices) { SkDebugf("Could not allocate vertices\n"); return; } @@ -1653,7 +1653,7 @@ private: int firstVertex; VertexWriter vertices{target->makeVertexSpace(fProgramInfo->geomProc().vertexStride(), fVertCount, &vertexBuffer, &firstVertex)}; - if (!vertices.fPtr) { + if (!vertices) { SkDebugf("Could not allocate vertices\n"); return; } @@ -1987,7 +1987,7 @@ private: QuadHelper helper(target, fProgramInfo->geomProc().vertexStride(), fEllipses.count()); VertexWriter verts{helper.vertices()}; - if (!verts.fPtr) { + if (!verts) { SkDebugf("Could not allocate vertices\n"); return; } @@ -2263,7 +2263,7 @@ private: QuadHelper helper(target, fProgramInfo->geomProc().vertexStride(), fEllipses.count()); VertexWriter verts{helper.vertices()}; - if (!verts.fPtr) { + if (!verts) { return; } @@ -2684,7 +2684,7 @@ private: VertexWriter verts{target->makeVertexSpace(fProgramInfo->geomProc().vertexStride(), fVertCount, &vertexBuffer, &firstVertex)}; - if (!verts.fPtr) { + if (!verts) { SkDebugf("Could not allocate vertices\n"); return; } @@ -3014,7 +3014,7 @@ private: std::move(indexBuffer), kVertsPerStandardRRect, indicesPerInstance, fRRects.count(), kNumRRectsInIndexBuffer); VertexWriter verts{helper.vertices()}; - if (!verts.fPtr) { + if (!verts) { SkDebugf("Could not allocate vertices\n"); return; } diff --git a/src/gpu/ops/LatticeOp.cpp b/src/gpu/ops/LatticeOp.cpp index d6789c5f67..73bd352789 100644 --- a/src/gpu/ops/LatticeOp.cpp +++ b/src/gpu/ops/LatticeOp.cpp @@ -250,7 +250,7 @@ private: QuadHelper helper(target, kVertexStride, numRects); VertexWriter vertices{helper.vertices()}; - if (!vertices.fPtr) { + if (!vertices) { SkDebugf("Could not allocate vertices\n"); return; } diff --git a/src/gpu/ops/QuadPerEdgeAA.cpp b/src/gpu/ops/QuadPerEdgeAA.cpp index d60ccbd268..5e865093c6 100644 --- a/src/gpu/ops/QuadPerEdgeAA.cpp +++ b/src/gpu/ops/QuadPerEdgeAA.cpp @@ -368,7 +368,7 @@ void Tessellator::append(GrQuad* deviceQuad, GrQuad* localQuad, const SkPMColor4f& color, const SkRect& uvSubset, GrQuadAAFlags aaFlags) { // We allow Tessellator to be created with a null vertices pointer for convenience, but it is // assumed it will never actually be used in those cases. - SkASSERT(fVertexWriter.fPtr); + SkASSERT(fVertexWriter); SkASSERT(deviceQuad->quadType() <= fVertexSpec.deviceQuadType()); SkASSERT(localQuad || !fVertexSpec.hasLocalCoords()); SkASSERT(!fVertexSpec.hasLocalCoords() || localQuad->quadType() <= fVertexSpec.localQuadType()); diff --git a/src/gpu/ops/QuadPerEdgeAA.h b/src/gpu/ops/QuadPerEdgeAA.h index ae4f4177b0..c01031894d 100644 --- a/src/gpu/ops/QuadPerEdgeAA.h +++ b/src/gpu/ops/QuadPerEdgeAA.h @@ -146,7 +146,7 @@ namespace skgpu::v1::QuadPerEdgeAA { void append(GrQuad* deviceQuad, GrQuad* localQuad, const SkPMColor4f& color, const SkRect& uvSubset, GrQuadAAFlags aaFlags); - SkDEBUGCODE(char* vertices() const { return (char*) fVertexWriter.fPtr; }) + SkDEBUGCODE(char* vertices() const { return (char*) fVertexWriter.ptr(); }) private: // VertexSpec defines many unique ways to write vertex attributes, which can be handled diff --git a/src/gpu/ops/RegionOp.cpp b/src/gpu/ops/RegionOp.cpp index 7b65fce533..4575eb4b01 100644 --- a/src/gpu/ops/RegionOp.cpp +++ b/src/gpu/ops/RegionOp.cpp @@ -125,7 +125,7 @@ private: QuadHelper helper(target, fProgramInfo->geomProc().vertexStride(), numRects); VertexWriter vertices{helper.vertices()}; - if (!vertices.fPtr) { + if (!vertices) { SkDebugf("Could not allocate vertices\n"); return; } diff --git a/src/gpu/ops/SmallPathRenderer.cpp b/src/gpu/ops/SmallPathRenderer.cpp index 4a26213456..df2b771fa1 100644 --- a/src/gpu/ops/SmallPathRenderer.cpp +++ b/src/gpu/ops/SmallPathRenderer.cpp @@ -216,7 +216,7 @@ private: &flushInfo.fVertexBuffer, &flushInfo.fVertexOffset)}; flushInfo.fIndexBuffer = target->resourceProvider()->refNonAAQuadIndexBuffer(); - if (!vertices.fPtr || !flushInfo.fIndexBuffer) { + if (!vertices || !flushInfo.fIndexBuffer) { SkDebugf("Could not allocate vertices\n"); return; } diff --git a/src/gpu/ops/StrokeRectOp.cpp b/src/gpu/ops/StrokeRectOp.cpp index 558893c018..6a819d385a 100644 --- a/src/gpu/ops/StrokeRectOp.cpp +++ b/src/gpu/ops/StrokeRectOp.cpp @@ -642,7 +642,7 @@ void AAStrokeRectOp::onPrepareDraws(GrMeshDrawTarget* target) { fProgramInfo->geomProc().vertexStride(), std::move(indexBuffer), verticesPerInstance, indicesPerInstance, instanceCount, maxQuads); VertexWriter vertices{ helper.vertices() }; - if (!vertices.fPtr) { + if (!vertices) { SkDebugf("Could not allocate vertices\n"); return; }