Reland "use a linked list for GrAtlasTextOp geometries"
This is a reland of 82a8130654bc6354cdaee9ce05e5d6118e1c5e33
Original change's description:
> use a linked list for GrAtlasTextOp geometries
>
> Instead of using a GrTBlockList<Geometry> to hold the geometries
> in the op. Allocate them on the RecordTimeAllocator, and
> link them together.
>
> Change-Id: I32af5724e7abeca1ddb6d38b26afbff7919cbc76
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341725
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I526d8b0b06fca13046c065cbc06f0c0e1103ce11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371996
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 3f217e7..c228a16 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -37,7 +37,7 @@
bool needsTransform,
int glyphCount,
SkRect deviceRect,
- const Geometry& geo,
+ Geometry* geo,
GrPaint&& paint)
: INHERITED{ClassID()}
, fProcessors(std::move(paint))
@@ -46,9 +46,10 @@
, fMaskType(static_cast<uint32_t>(maskType))
, fUsesLocalCoords(false)
, fNeedsGlyphTransform(needsTransform)
- , fHasPerspective(needsTransform && geo.fDrawMatrix.hasPerspective())
- , fUseGammaCorrectDistanceTable(false) {
- fGeometries.push_back(geo);
+ , fHasPerspective(needsTransform && geo->fDrawMatrix.hasPerspective())
+ , fUseGammaCorrectDistanceTable(false)
+ , fHead{geo}
+ , fTail{&fHead->fNext} {
// We don't have tight bounds on the glyph paths in device space. For the purposes of bounds
// we treat this as a set of non-AA rects rendered with a texture.
this->setBounds(deviceRect, HasAABloat::kNo, IsHairline::kNo);
@@ -61,7 +62,7 @@
SkColor luminanceColor,
bool useGammaCorrectDistanceTable,
uint32_t DFGPFlags,
- const Geometry& geo,
+ Geometry* geo,
GrPaint&& paint)
: INHERITED{ClassID()}
, fProcessors(std::move(paint))
@@ -70,15 +71,34 @@
, fMaskType(static_cast<uint32_t>(maskType))
, fUsesLocalCoords(false)
, fNeedsGlyphTransform(needsTransform)
- , fHasPerspective(needsTransform && geo.fDrawMatrix.hasPerspective())
+ , fHasPerspective(needsTransform && geo->fDrawMatrix.hasPerspective())
, fUseGammaCorrectDistanceTable(useGammaCorrectDistanceTable)
- , fLuminanceColor(luminanceColor) {
- fGeometries.push_back(geo);
+ , fLuminanceColor(luminanceColor)
+ , fHead{geo}
+ , fTail{&fHead->fNext} {
// We don't have tight bounds on the glyph paths in device space. For the purposes of bounds
// we treat this as a set of non-AA rects rendered with a texture.
this->setBounds(deviceRect, HasAABloat::kNo, IsHairline::kNo);
}
+auto GrAtlasTextOp::Geometry::Make(GrRecordingContext* rc,
+ const GrAtlasSubRun& subRun,
+ const SkMatrix& drawMatrix,
+ SkPoint drawOrigin,
+ SkIRect clipRect,
+ GrTextBlob* blob,
+ const SkPMColor4f& color) -> Geometry* {
+ auto arena = rc->priv().recordTimeAllocator();
+ return arena->make<Geometry>(subRun,
+ drawMatrix,
+ drawOrigin,
+ clipRect,
+ SkRef(blob),
+ color);
+}
+
+
+
void GrAtlasTextOp::Geometry::fillVertexData(void *dst, int offset, int count) const {
SkMatrix positionMatrix = fDrawMatrix;
positionMatrix.preTranslate(fDrawOrigin.x(), fDrawOrigin.y());
@@ -94,12 +114,12 @@
SkString GrAtlasTextOp::onDumpInfo() const {
SkString str;
int i = 0;
- for (const auto& g : fGeometries.items()) {
+ for(Geometry* geom = fHead; geom != nullptr; geom = geom->fNext) {
str.appendf("%d: Color: 0x%08x Trans: %.2f,%.2f\n",
i++,
- g.fColor.toBytes_RGBA(),
- g.fDrawOrigin.x(),
- g.fDrawOrigin.y());
+ geom->fColor.toBytes_RGBA(),
+ geom->fDrawOrigin.x(),
+ geom->fDrawOrigin.y());
}
str += fProcessors.dumpProcessors();
@@ -121,7 +141,7 @@
} else {
// finalize() is called before any merging is done, so at this point there's at most one
// Geometry with a color. Later, for non-bitmap ops, we may have mixed colors.
- color.setToConstant(fGeometries.front().fColor);
+ color.setToConstant(fHead->fColor);
}
switch (this->maskType()) {
@@ -142,7 +162,7 @@
auto analysis = fProcessors.finalize(
color, coverage, clip, &GrUserStencilSettings::kUnused, hasMixedSampledCoverage, caps,
- clampType, &fGeometries.front().fColor);
+ clampType, &fHead->fColor);
// TODO(michaelludwig): Once processor analysis can be done external to op creation/finalization
// the atlas op metadata can be fully const. This is okay for now since finalize() happens
// before the op is merged, so during combineIfPossible, metadata is effectively const.
@@ -158,7 +178,7 @@
// the matrix is identity. When the shaders require local coords, combineIfPossible requires all
// all geometries to have same draw matrix.
SkMatrix localMatrix = SkMatrix::I();
- if (fUsesLocalCoords && !fGeometries.front().fDrawMatrix.invert(&localMatrix)) {
+ if (fUsesLocalCoords && !fHead->fDrawMatrix.invert(&localMatrix)) {
return;
}
@@ -200,7 +220,7 @@
// Bitmap text uses a single color, combineIfPossible ensures all geometries have the same
// color, so we can use the first's without worry.
flushInfo.fGeometryProcessor = GrBitmapTextGeoProc::Make(
- target->allocator(), *target->caps().shaderCaps(), fGeometries.front().fColor,
+ target->allocator(), *target->caps().shaderCaps(), fHead->fColor,
false, views, numActiveViews, filter, maskFormat, localMatrix, fHasPerspective);
}
@@ -236,9 +256,9 @@
resetVertexBuffer();
- for (const Geometry& geo : fGeometries.items()) {
- const GrAtlasSubRun& subRun = geo.fSubRun;
- SkASSERT((int) subRun.vertexStride(geo.fDrawMatrix) == vertexStride);
+ for (const Geometry* geo = fHead; geo != nullptr; geo = geo->fNext) {
+ const GrAtlasSubRun& subRun = geo->fSubRun;
+ SkASSERT((int) subRun.vertexStride(geo->fDrawMatrix) == vertexStride);
const int subRunEnd = subRun.glyphCount();
for (int subRunCursor = 0; subRunCursor < subRunEnd;) {
@@ -251,7 +271,7 @@
return;
}
- geo.fillVertexData(vertices + quadCursor * quadSize, subRunCursor, glyphsRegenerated);
+ geo->fillVertexData(vertices + quadCursor * quadSize, subRunCursor, glyphsRegenerated);
subRunCursor += glyphsRegenerated;
quadCursor += glyphsRegenerated;
@@ -363,8 +383,8 @@
if (fUsesLocalCoords) {
// If the fragment processors use local coordinates, the GPs compute them using the inverse
// of the view matrix stored in a uniform, so all geometries must have the same matrix.
- const SkMatrix& thisFirstMatrix = fGeometries.front().fDrawMatrix;
- const SkMatrix& thatFirstMatrix = that->fGeometries.front().fDrawMatrix;
+ const SkMatrix& thisFirstMatrix = fHead->fDrawMatrix;
+ const SkMatrix& thatFirstMatrix = that->fHead->fDrawMatrix;
if (!SkMatrixPriv::CheapEqual(thisFirstMatrix, thatFirstMatrix)) {
return CombineResult::kCannotCombine;
}
@@ -377,7 +397,7 @@
}
} else {
if (this->maskType() == MaskType::kColorBitmap &&
- fGeometries.front().fColor != that->fGeometries.front().fColor) {
+ fHead->fColor != that->fHead->fColor) {
// This ensures all merged bitmap color text ops have a constant color
return CombineResult::kCannotCombine;
}
@@ -386,7 +406,8 @@
fNumGlyphs += that->fNumGlyphs;
// After concat, that's geometry list is emptied so it will not unref the blobs when destructed
- fGeometries.concat(std::move(that->fGeometries));
+ this->addGeometry(that->fHead);
+ that->fHead = nullptr;
return CombineResult::kMerged;
}
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index 29c5a54..7c3e191 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -20,8 +20,8 @@
DEFINE_OP_CLASS_ID
~GrAtlasTextOp() override {
- for (const auto& g : fGeometries.items()) {
- g.fBlob->unref();
+ for (const Geometry* g = fHead; g != nullptr; g = g->fNext) {
+ g->fBlob->unref();
}
}
@@ -41,11 +41,21 @@
, fClipRect{clipRect}
, fBlob{blob}
, fColor{color} {}
+
+ static Geometry* Make(GrRecordingContext* rc,
+ const GrAtlasSubRun& subRun,
+ const SkMatrix& drawMatrix,
+ SkPoint drawOrigin,
+ SkIRect clipRect,
+ GrTextBlob* blob,
+ const SkPMColor4f& color);
void fillVertexData(void* dst, int offset, int count) const;
const GrAtlasSubRun& fSubRun;
const SkMatrix fDrawMatrix;
const SkPoint fDrawOrigin;
+ // fClipRect is only used in the DirectMaskSubRun case to do geometric clipping.
+ // TransformedMaskSubRun, and SDFTSubRun don't use this field, and expect an empty rect.
const SkIRect fClipRect;
GrTextBlob* const fBlob; // mutable to make unref call in Op dtor.
@@ -53,6 +63,7 @@
// a constant color that we then evaluate on the CPU.
// TODO: This can be made const once processor analysis is separated from op creation.
SkPMColor4f fColor;
+ Geometry* fNext{nullptr};
};
const char* name() const override { return "AtlasTextOp"; }
@@ -104,7 +115,7 @@
bool needsTransform,
int glyphCount,
SkRect deviceRect,
- const Geometry& geo,
+ Geometry* geo,
GrPaint&& paint);
GrAtlasTextOp(MaskType maskType,
@@ -114,7 +125,7 @@
SkColor luminanceColor,
bool useGammaCorrectDistanceTable,
uint32_t DFGPFlags,
- const Geometry& geo,
+ Geometry* geo,
GrPaint&& paint);
GrProgramInfo* programInfo() override {
@@ -122,6 +133,14 @@
return nullptr;
}
+ void addGeometry(Geometry* geometry) {
+ *fTail = geometry;
+ // The geometry may have many entries. Find the end.
+ do {
+ fTail = &(*fTail)->fNext;
+ } while (*fTail != nullptr);
+ }
+
void onCreateProgramInfo(const GrCaps*,
SkArenaAlloc*,
const GrSurfaceProxyView& writeView,
@@ -193,14 +212,6 @@
const GrSurfaceProxyView* views,
unsigned int numActiveViews) const;
- // The minimum number of Geometry we will try to allocate as ops are merged together.
- // The atlas text op holds one Geometry inline. When combined with the linear growth policy,
- // the total number of geometries follows 6, 18, 36, 60, 90 (the deltas are 6*n).
- static constexpr auto kMinGeometryAllocated = 6;
-
- GrTBlockList<Geometry> fGeometries{kMinGeometryAllocated,
- GrBlockAllocator::GrowthPolicy::kLinear};
-
GrProcessorSet fProcessors;
int fNumGlyphs; // Sum of glyphs in each geometry's subrun
@@ -218,6 +229,9 @@
// for single-channel distance fields.
const SkColor fLuminanceColor{0};
+ Geometry* fHead{nullptr};
+ Geometry** fTail{&fHead};
+
using INHERITED = GrMeshDrawOp;
};
diff --git a/src/gpu/text/GrTextBlob.cpp b/src/gpu/text/GrTextBlob.cpp
index 1451c56..ead306f 100644
--- a/src/gpu/text/GrTextBlob.cpp
+++ b/src/gpu/text/GrTextBlob.cpp
@@ -686,16 +686,17 @@
const SkPaint& drawPaint = glyphRunList.paint();
const SkPMColor4f drawingColor =
calculate_colors(rtc, drawPaint, viewMatrix, fMaskFormat, &grPaint);
- GrAtlasTextOp::Geometry geometry = {
+
+ GrRecordingContext* const context = rtc->recordingContext();
+ GrAtlasTextOp::Geometry* geometry = GrAtlasTextOp::Geometry::Make(
+ context,
*this,
drawMatrix,
drawOrigin,
clipRect,
- SkRef(fBlob),
- drawingColor
- };
+ fBlob,
+ drawingColor);
- GrRecordingContext* const context = rtc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(context,
op_mask_type(fMaskFormat),
false,
@@ -974,18 +975,16 @@
GrPaint grPaint;
SkPMColor4f drawingColor = calculate_colors(rtc, drawPaint, viewMatrix, fMaskFormat, &grPaint);
- // 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 = {
+ GrRecordingContext* const context = rtc->recordingContext();
+ GrAtlasTextOp::Geometry* geometry = GrAtlasTextOp::Geometry::Make(
+ context,
*this,
drawMatrix,
drawOrigin,
SkIRect::MakeEmpty(),
- SkRef(fBlob),
- drawingColor
- };
+ fBlob,
+ drawingColor);
- GrRecordingContext* context = rtc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(
context,
op_mask_type(fMaskFormat),
@@ -1247,16 +1246,16 @@
DFGPFlags |= MT::kLCDBGRDistanceField == maskType ? kBGR_DistanceFieldEffectFlag : 0;
}
- GrAtlasTextOp::Geometry geometry = {
+ GrRecordingContext* const context = rtc->recordingContext();
+ GrAtlasTextOp::Geometry* geometry = GrAtlasTextOp::Geometry::Make(
+ context,
*this,
drawMatrix,
drawOrigin,
SkIRect::MakeEmpty(),
- SkRef(fBlob),
- drawingColor
- };
+ fBlob,
+ drawingColor);
- GrRecordingContext* context = rtc->recordingContext();
GrOp::Owner op = GrOp::Make<GrAtlasTextOp>(
context,
maskType,
