add matrix compare to text blob key
In multi-threaded use, the cache would return a blob with direct sub runs without comparing matrices. If two direct blobs have different matrices, then they are incompatible. The original code would find that the blob did not exist in the cache. It would then make a new blob and insert it into the cache. During that time, a different thread would create an incompatible blob and insert it into the cache. The original thread would insert the new blob into the cache, but the cache finding an existing blob would return the incompatible blob. The solution was to make the matrix, and drawing type part of the key. Now, only compatible blobs are returned from the cache. Change-Id: I318c756f07c84dd48b353b25639f9dcbe80c7b4b Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377976 Commit-Queue: Herb Derby <herb@google.com> Reviewed-by: Robert Phillips <robertphillips@google.com>
This commit is contained in:
parent
4633c9149b
commit
e4ef35caa1
@ -386,6 +386,9 @@ void GrSurfaceDrawContext::drawGlyphRunList(const GrClip* clip,
|
||||
SkScalerContextFlags scalerContextFlags = this->colorInfo().isLinearlyBlended()
|
||||
? SkScalerContextFlags::kBoostContrast
|
||||
: SkScalerContextFlags::kFakeGammaAndBoostContrast;
|
||||
SkMatrix drawMatrix(viewMatrix.localToDevice());
|
||||
SkPoint drawOrigin = glyphRunList.origin();
|
||||
drawMatrix.preTranslate(drawOrigin.x(), drawOrigin.y());
|
||||
|
||||
sk_sp<GrTextBlob> blob;
|
||||
GrTextBlob::Key key;
|
||||
@ -412,14 +415,33 @@ void GrSurfaceDrawContext::drawGlyphRunList(const GrClip* clip,
|
||||
}
|
||||
key.fCanonicalColor = canonicalColor;
|
||||
key.fScalerContextFlags = scalerContextFlags;
|
||||
|
||||
// Calculate the set of drawing types.
|
||||
key.fSetOfDrawingTypes = 0;
|
||||
for (auto& run : glyphRunList) {
|
||||
key.fSetOfDrawingTypes |= options.drawingType(run.font(), drawPaint, drawMatrix);
|
||||
}
|
||||
|
||||
if (key.fSetOfDrawingTypes & GrSDFTOptions::kDirect) {
|
||||
// Store the fractional offset of the position. We know that the matrix can't be
|
||||
// perspective at this point.
|
||||
SkPoint mappedOrigin = drawMatrix.mapOrigin();
|
||||
key.fDrawMatrix = drawMatrix;
|
||||
key.fDrawMatrix.setTranslateX(
|
||||
mappedOrigin.x() - SkScalarFloorToScalar(mappedOrigin.x()));
|
||||
key.fDrawMatrix.setTranslateY(
|
||||
mappedOrigin.y() - SkScalarFloorToScalar(mappedOrigin.y()));
|
||||
} else {
|
||||
// For path and SDFT, the matrix doesn't matter.
|
||||
key.fDrawMatrix = SkMatrix::I();
|
||||
}
|
||||
|
||||
blob = textBlobCache->find(key);
|
||||
}
|
||||
|
||||
SkMatrix drawMatrix(viewMatrix.localToDevice());
|
||||
SkPoint drawOrigin = glyphRunList.origin();
|
||||
drawMatrix.preTranslate(drawOrigin.x(), drawOrigin.y());
|
||||
if (blob == nullptr || !blob->canReuse(drawPaint, drawMatrix)) {
|
||||
if (blob != nullptr) {
|
||||
SkASSERT(!drawMatrix.hasPerspective());
|
||||
// We have to remake the blob because changes may invalidate our masks.
|
||||
// TODO we could probably get away with reuse most of the time if the pointer is unique,
|
||||
// but we'd have to clear the SubRun information
|
||||
|
@ -18,10 +18,10 @@ class GrSDFTOptions {
|
||||
public:
|
||||
GrSDFTOptions(bool ableToUseSDFT, bool useSDFTForSmallText, SkScalar min, SkScalar max);
|
||||
|
||||
enum DrawingType {
|
||||
kDirect,
|
||||
kSDFT,
|
||||
kPath
|
||||
enum DrawingType : uint8_t {
|
||||
kDirect = 1,
|
||||
kSDFT = 2,
|
||||
kPath = 4
|
||||
};
|
||||
|
||||
DrawingType drawingType(
|
||||
|
@ -199,7 +199,6 @@ private:
|
||||
SkPoint fOrigin;
|
||||
};
|
||||
|
||||
const GrTextBlob& fBlob;
|
||||
const bool fIsAntiAliased;
|
||||
const SkStrikeSpec fStrikeSpec;
|
||||
const SkSpan<const PathGlyph> fPaths;
|
||||
@ -211,8 +210,7 @@ PathSubRun::PathSubRun(bool isAntiAliased,
|
||||
const GrTextBlob& blob,
|
||||
SkSpan<PathGlyph> paths,
|
||||
std::unique_ptr<PathGlyph[], GrSubRunAllocator::ArrayDestroyer> pathData)
|
||||
: fBlob{blob}
|
||||
, fIsAntiAliased{isAntiAliased}
|
||||
: fIsAntiAliased{isAntiAliased}
|
||||
, fStrikeSpec{strikeSpec}
|
||||
, fPaths{paths}
|
||||
, fPathData{std::move(pathData)} {}
|
||||
@ -272,19 +270,8 @@ void PathSubRun::draw(const GrClip* clip,
|
||||
}
|
||||
}
|
||||
|
||||
// This is the odd one. Intuition would lead you to believe that this should just return true
|
||||
// because it can handle all cases. The original code forced the check_integer_translate() for
|
||||
// paths explicitly. This check is needed because if the blob was drawn large, and then small, the
|
||||
// path would be reused when the blob should be rendered with masks.
|
||||
// TODO(herb): rethink when paths can be reused.
|
||||
bool PathSubRun::canReuse(const SkPaint& paint, const SkMatrix& drawMatrix) const {
|
||||
const SkMatrix initialMatrix = fBlob.initialMatrix();
|
||||
if (initialMatrix.hasPerspective() && !SkMatrixPriv::CheapEqual(initialMatrix, drawMatrix)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
auto [reuse, _] = check_integer_translate(fBlob.initialMatrix(), drawMatrix);
|
||||
return reuse;
|
||||
return true;
|
||||
}
|
||||
|
||||
auto PathSubRun::Make(
|
||||
@ -579,10 +566,6 @@ void DirectMaskSubRun::draw(const GrClip* clip, const SkMatrixProvider& viewMatr
|
||||
|
||||
bool
|
||||
DirectMaskSubRun::canReuse(const SkPaint& paint, const SkMatrix& drawMatrix) const {
|
||||
if (drawMatrix.hasPerspective()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
auto [reuse, translation] = check_integer_translate(fBlob->initialMatrix(), drawMatrix);
|
||||
|
||||
// If glyphs were excluded because of position bounds, then this subrun can only be reused if
|
||||
@ -1291,9 +1274,6 @@ void SDFTSubRun::draw(const GrClip* clip,
|
||||
|
||||
bool SDFTSubRun::canReuse(const SkPaint& paint, const SkMatrix& drawMatrix) const {
|
||||
const SkMatrix& initialMatrix = fBlob->initialMatrix();
|
||||
if (drawMatrix.hasPerspective()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// A scale outside of [blob.fMaxMinScale, blob.fMinMaxScale] would result in a different
|
||||
// distance field being generated, so we have to regenerate in those cases
|
||||
@ -1375,6 +1355,21 @@ bool GrTextBlob::Key::operator==(const GrTextBlob::Key& that) const {
|
||||
}
|
||||
}
|
||||
if (fScalerContextFlags != that.fScalerContextFlags) { return false; }
|
||||
|
||||
// Just punt on perspective.
|
||||
if (fDrawMatrix.hasPerspective()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (fSetOfDrawingTypes != that.fSetOfDrawingTypes) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (fSetOfDrawingTypes & GrSDFTOptions::kDirect) {
|
||||
auto [compatible, _] = check_integer_translate(fDrawMatrix, that.fDrawMatrix);
|
||||
return compatible;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -364,6 +364,7 @@ struct GrSubRunList {
|
||||
//
|
||||
class GrTextBlob final : public SkNVRefCnt<GrTextBlob>, public SkGlyphRunPainterInterface {
|
||||
public:
|
||||
SK_BEGIN_REQUIRE_DENSE
|
||||
struct Key {
|
||||
Key();
|
||||
uint32_t fUniqueID;
|
||||
@ -372,17 +373,21 @@ public:
|
||||
// luminance. For each luminance bucket there is a "canonical color" that
|
||||
// represents the bucket. This functionality is currently only supported for A8
|
||||
SkColor fCanonicalColor;
|
||||
SkPaint::Style fStyle;
|
||||
SkScalar fFrameWidth;
|
||||
SkScalar fMiterLimit;
|
||||
SkPaint::Join fJoin;
|
||||
SkPixelGeometry fPixelGeometry;
|
||||
bool fHasBlur;
|
||||
SkMaskFilterBase::BlurRec fBlurRec;
|
||||
uint32_t fScalerContextFlags;
|
||||
SkMatrix fDrawMatrix;
|
||||
// Below here fields are of size 1 byte.
|
||||
uint8_t fSetOfDrawingTypes;
|
||||
bool fHasBlur;
|
||||
SkPaint::Style fStyle;
|
||||
SkPaint::Join fJoin;
|
||||
|
||||
bool operator==(const Key& other) const;
|
||||
};
|
||||
SK_END_REQUIRE_DENSE
|
||||
|
||||
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrTextBlob);
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user