make calls to SubRun and AtlasSubRun const
In theory, all sub runs should be fully const because they are shared by
multiple threads. But, they are used in a single-threaded situation
when flushed to the GPU. AtlasSubRuns are mutate during the GPU flush
to convert PackedGlyphIDs to GrGlyph pointers. Because of that I
decided to make everything const, but label the GrGlyphVector (the
structure that holds the PackedGlyphIDs/GrGlyphs) to be mutable.
Change-Id: I328175a8933b64fda7fab2b3b3d8699683451e60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304216
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp
index 6a41ac5..90e6fa9 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -84,8 +84,8 @@
}
void GrAtlasTextOp::Geometry::fillVertexData(void *dst, int offset, int count) const {
- fSubRunPtr->fillVertexData(dst, offset, count, fColor.toBytes_RGBA(),
- fDrawMatrix, fDrawOrigin, fClipRect);
+ fSubRun.fillVertexData(dst, offset, count, fColor.toBytes_RGBA(),
+ fDrawMatrix, fDrawOrigin, fClipRect);
}
void GrAtlasTextOp::visitProxies(const VisitProxyFunc& func) const {
@@ -230,15 +230,15 @@
resetVertexBuffer();
for (const Geometry& geo : SkMakeSpan(fGeoData.get(), fGeoCount)) {
- GrAtlasSubRun* const subRun = geo.fSubRunPtr;
- SkASSERT((int)subRun->vertexStride() == vertexStride);
+ const GrAtlasSubRun& subRun = geo.fSubRun;
+ SkASSERT((int)subRun.vertexStride() == vertexStride);
- const int subRunEnd = subRun->glyphCount();
+ const int subRunEnd = subRun.glyphCount();
for (int subRunCursor = 0; subRunCursor < subRunEnd;) {
// Regenerate the atlas for the remainder of the glyphs in the run, or the remainder
// of the glyphs to fill the vertex buffer.
int regenEnd = subRunCursor + std::min(subRunEnd - subRunCursor, quadEnd - quadCursor);
- auto[ok, glyphsRegenerated] = subRun->regenerateAtlas(subRunCursor, regenEnd, target);
+ auto[ok, glyphsRegenerated] = subRun.regenerateAtlas(subRunCursor, regenEnd, target);
// There was a problem allocating the glyph in the atlas. Bail.
if (!ok) {
return;
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index 95ac80e..c275eb1 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -29,7 +29,7 @@
struct Geometry {
void fillVertexData(void* dst, int offset, int count) const;
- GrAtlasSubRun* const fSubRunPtr;
+ const GrAtlasSubRun& fSubRun;
const SkMatrix fDrawMatrix;
const SkPoint fDrawOrigin;
const SkIRect fClipRect;
diff --git a/src/gpu/text/GrTextBlob.cpp b/src/gpu/text/GrTextBlob.cpp
index 84c37eb..1df19ae 100644
--- a/src/gpu/text/GrTextBlob.cpp
+++ b/src/gpu/text/GrTextBlob.cpp
@@ -54,7 +54,7 @@
void GrPathSubRun::draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) {
+ GrRenderTargetContext* rtc) const {
SkASSERT(!fPaths.empty());
SkPoint drawOrigin = glyphRunList.origin();
const SkPaint& drawPaint = glyphRunList.paint();
@@ -142,8 +142,10 @@
return SkMakeSpan(reinterpret_cast<const GrGlyph**>(fGlyphs.data()), fGlyphs.size());
}
-std::tuple<bool, int> GrGlyphVector::regenerateAtlas(
- int begin, int end, GrMaskFormat maskFormat, int padding, GrMeshDrawOp::Target *target) {
+std::tuple<bool, int> GrGlyphVector::regenerateAtlas(int begin, int end,
+ GrMaskFormat maskFormat,
+ int padding,
+ GrMeshDrawOp::Target* target) {
GrAtlasManager* atlasManager = target->atlasManager();
GrDeferredUploadTarget* uploadTarget = target->deferredUploadTarget();
@@ -254,7 +256,7 @@
}
void GrDirectMaskSubRun::draw(const GrClip* clip, const SkMatrixProvider& viewMatrix,
- const SkGlyphRunList& glyphRunList, GrRenderTargetContext* rtc) {
+ const SkGlyphRunList& glyphRunList, GrRenderTargetContext* rtc) const{
auto[drawingClip, op] = this->makeAtlasTextOp(clip, viewMatrix, glyphRunList, rtc);
if (op != nullptr) {
rtc->priv().addDrawOp(drawingClip, std::move(op));
@@ -311,7 +313,7 @@
std::tuple<const GrClip*, std::unique_ptr<GrDrawOp>>
GrDirectMaskSubRun::makeAtlasTextOp(const GrClip* clip, const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) {
+ GrRenderTargetContext* rtc) const {
SkASSERT(this->glyphCount() != 0);
const SkMatrix& drawMatrix = viewMatrix.localToDevice();
@@ -348,7 +350,7 @@
const SkPMColor4f drawingColor =
calculate_colors(rtc, drawPaint, viewMatrix, fMaskFormat, &grPaint);
GrAtlasTextOp::Geometry geometry = {
- this,
+ *this,
drawMatrix,
drawOrigin,
clipRect,
@@ -368,7 +370,7 @@
}
std::tuple<bool, int>
-GrDirectMaskSubRun::regenerateAtlas(int begin, int end, GrMeshDrawOp::Target* target) {
+GrDirectMaskSubRun::regenerateAtlas(int begin, int end, GrMeshDrawOp::Target* target) const {
return fGlyphs.regenerateAtlas(begin, end, fMaskFormat, 0, target);
}
@@ -500,7 +502,7 @@
GrMaskSubRun::makeAtlasTextOp(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) {
+ GrRenderTargetContext* rtc) const {
SkASSERT(this->glyphCount() != 0);
SkPoint drawOrigin = glyphRunList.origin();
@@ -515,7 +517,7 @@
// We can clip geometrically using clipRect and ignore clip if we're not using SDFs or
// transformed glyphs, and we have an axis-aligned rectangular non-AA clip.
GrAtlasTextOp::Geometry geometry = {
- this,
+ *this,
drawMatrix,
drawOrigin,
SkIRect::MakeEmpty(),
@@ -575,7 +577,7 @@
void GrMaskSubRun::draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) {
+ GrRenderTargetContext* rtc) const {
auto[drawingClip, op] = this->makeAtlasTextOp(clip, viewMatrix, glyphRunList, rtc);
if (op != nullptr) {
rtc->priv().addDrawOp(drawingClip, std::move(op));
@@ -583,7 +585,7 @@
}
std::tuple<bool, int> GrMaskSubRun::regenerateAtlas(
- int begin, int end, GrMeshDrawOp::Target *target) {
+ int begin, int end, GrMeshDrawOp::Target *target) const {
return fGlyphs.regenerateAtlas(begin, end, this->maskFormat(), this->atlasPadding(), target);
}
diff --git a/src/gpu/text/GrTextBlob.h b/src/gpu/text/GrTextBlob.h
index 1d12fbb..47615b3 100644
--- a/src/gpu/text/GrTextBlob.h
+++ b/src/gpu/text/GrTextBlob.h
@@ -188,7 +188,7 @@
virtual void draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) = 0;
+ GrRenderTargetContext* rtc) const = 0;
private:
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrSubRun);
@@ -203,7 +203,7 @@
void draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) override;
+ GrRenderTargetContext* rtc) const override;
static GrSubRun* Make(const SkZip<SkGlyphVariant, SkPoint>& drawables,
bool isAntiAliased,
@@ -232,13 +232,17 @@
makeAtlasTextOp(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) = 0;
- virtual std::tuple<bool, int> regenerateAtlas(
- int begin, int end, GrMeshDrawOp::Target* target) = 0;
+ GrRenderTargetContext* rtc) const = 0;
virtual void fillVertexData(
void* vertexDst, int offset, int count,
GrColor color, const SkMatrix& drawMatrix, SkPoint drawOrigin,
SkIRect clip) const = 0;
+
+ // This call is not thread safe. It should only be called from GrDrawOp::onPrepare which
+ // is single threaded.
+ virtual std::tuple<bool, int> regenerateAtlas(
+ int begin, int end, GrMeshDrawOp::Target* target) const = 0;
+
protected:
using VertexData = std::tuple<
SkPoint, // glyph position.
@@ -286,13 +290,11 @@
public:
static GrGlyphVector Make(
const SkStrikeSpec& spec, SkSpan<SkGlyphVariant> glyphs, SkArenaAlloc* alloc);
-
- std::tuple<bool, int> regenerateAtlas(
- int begin, int end, GrMaskFormat maskFormat, int padding, GrMeshDrawOp::Target *target);
-
SkSpan<const GrGlyph*> glyphs() const;
SkScalar strikeToSourceRatio() const { return fStrikeSpec.strikeToSourceRatio(); }
+ std::tuple<bool, int> regenerateAtlas(
+ int begin, int end, GrMaskFormat maskFormat, int padding, GrMeshDrawOp::Target *target);
private:
GrGlyphVector(const SkStrikeSpec& spec, SkSpan<Variant> glyphs);
@@ -322,7 +324,7 @@
void draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) override;
+ GrRenderTargetContext* rtc) const override;
size_t vertexStride() const override;
@@ -332,10 +334,10 @@
makeAtlasTextOp(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) override;
+ GrRenderTargetContext* rtc) const override;
std::tuple<bool, int>
- regenerateAtlas(int begin, int end, GrMeshDrawOp::Target* target) override;
+ regenerateAtlas(int begin, int end, GrMeshDrawOp::Target* target) const override;
void fillVertexData(void* vertexDst, int offset, int count, GrColor color,
const SkMatrix& drawMatrix, SkPoint drawOrigin,
@@ -350,7 +352,9 @@
const SkRect fVertexBounds;
const SkSpan<const VertexData> fVertexData;
- GrGlyphVector fGlyphs;
+ // The regenerateAtlas method mutates fGlyphs. It should be called from onPrepare which must
+ // be single threaded.
+ mutable GrGlyphVector fGlyphs;
};
// -- GrMaskSubRun ---------------------------------------------------------------------------------
@@ -382,15 +386,15 @@
makeAtlasTextOp(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) override;
+ GrRenderTargetContext* rtc) const override;
void draw(const GrClip* clip,
const SkMatrixProvider& viewMatrix,
const SkGlyphRunList& glyphRunList,
- GrRenderTargetContext* rtc) override;
+ GrRenderTargetContext* rtc) const override;
std::tuple<bool, int> regenerateAtlas(
- int begin, int end, GrMeshDrawOp::Target* target) override;
+ int begin, int end, GrMeshDrawOp::Target* target) const override;
size_t vertexStride() const override;
void fillVertexData(
@@ -443,7 +447,9 @@
bool fUseLCDText{false};
bool fAntiAliased{false};
- GrGlyphVector fGlyphs;
+ // The regenerateAtlas method mutates fGlyphs. It should be called from onPrepare which must
+ // be single threaded.
+ mutable GrGlyphVector fGlyphs;
// The bounds in source space. The bounds are the joined rectangles of all the glyphs.
const SkRect fVertexBounds;