From 84cd621670a357484e1674e06d3d8d6f929a4ab2 Mon Sep 17 00:00:00 2001 From: senorblanco Date: Tue, 4 Aug 2015 10:01:58 -0700 Subject: [PATCH] Implement caching of filled paths in the tessellated path renderer. Paths are cached as tessellated triangle meshes in vertex buffers on the GPU. Stroked paths are not (yet) cached. Paths containing no curved segments (linear paths) are reused at all scales. Paths containing curved segments are reused within a scale tolerance threshold. In order to invalidate the cache when an SkPath is changed or deleted, this required implementing genID change notification in SkPath. This is modelled almost exactly on SkPixelRef::GenIDChangeListener. However, It does not currently implement the check for unique genIDs, so notifiers will fire when the first instance of an SkPathRef using a given genID is destroyed. Another caveat is that you cannot successfully add a change notifier to an empty path, since it uses the "canonical" empty path which is never modified or destroyed. For this reason, we prevent adding listeners to it. BUG=skia:4121,skia:4122, 497403 DOCS_PREVIEW= https://skia.org/?cl=1114353004 Committed: https://skia.googlesource.com/skia/+/468dfa72eb6694145487be17876804dfca3b7adb Review URL: https://codereview.chromium.org/1114353004 --- include/core/SkPathRef.h | 25 +++-- include/gpu/GrResourceKey.h | 13 +++ src/core/SkPathPriv.h | 4 + src/core/SkPathRef.cpp | 44 +++++++-- src/gpu/GrTessellatingPathRenderer.cpp | 161 +++++++++++++++++++++++++-------- tests/PathTest.cpp | 60 ++++++++++++ tests/ResourceCacheTest.cpp | 31 +++++++ 7 files changed, 276 insertions(+), 62 deletions(-) diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index e7cc31c..c09f6e8 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -180,19 +180,7 @@ public: */ static void Rewind(SkAutoTUnref* pathRef); - virtual ~SkPathRef() { - SkDEBUGCODE(this->validate();) - sk_free(fPoints); - - SkDEBUGCODE(fPoints = NULL;) - SkDEBUGCODE(fVerbs = NULL;) - SkDEBUGCODE(fVerbCnt = 0x9999999;) - SkDEBUGCODE(fPointCnt = 0xAAAAAAA;) - SkDEBUGCODE(fPointCnt = 0xBBBBBBB;) - SkDEBUGCODE(fGenerationID = 0xEEEEEEEE;) - SkDEBUGCODE(fEditorsAttached = 0x7777777;) - } - + virtual ~SkPathRef(); int countPoints() const { SkDEBUGCODE(this->validate();) return fPointCnt; } int countVerbs() const { SkDEBUGCODE(this->validate();) return fVerbCnt; } int countWeights() const { SkDEBUGCODE(this->validate();) return fConicWeights.count(); } @@ -251,6 +239,13 @@ public: */ uint32_t genID() const; + struct GenIDChangeListener { + virtual ~GenIDChangeListener() {} + virtual void onChange() = 0; + }; + + void addGenIDChangeListener(GenIDChangeListener* listener); + SkDEBUGCODE(void validate() const;) private: @@ -422,6 +417,8 @@ private: return fPoints; } + void callGenIDChangeListeners(); + enum { kMinSize = 256, }; @@ -446,6 +443,8 @@ private: mutable uint32_t fGenerationID; SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time. + SkTDArray fGenIDChangeListeners; // pointers are owned + friend class PathRefTest_Private; typedef SkRefCnt INHERITED; }; diff --git a/include/gpu/GrResourceKey.h b/include/gpu/GrResourceKey.h index a353dc2..0a97c20 100644 --- a/include/gpu/GrResourceKey.h +++ b/include/gpu/GrResourceKey.h @@ -10,6 +10,7 @@ #define GrResourceKey_DEFINED #include "GrTypes.h" +#include "SkData.h" #include "SkOnce.h" #include "SkTemplates.h" @@ -237,6 +238,7 @@ public: GrUniqueKey& operator=(const GrUniqueKey& that) { this->INHERITED::operator=(that); + this->setCustomData(that.getCustomData()); return *this; } @@ -245,6 +247,14 @@ public: } bool operator!=(const GrUniqueKey& that) const { return !(*this == that); } + void setCustomData(const SkData* data) { + SkSafeRef(data); + fData.reset(data); + } + const SkData* getCustomData() const { + return fData.get(); + } + class Builder : public INHERITED::Builder { public: Builder(GrUniqueKey* key, Domain domain, int data32Count) @@ -268,6 +278,9 @@ public: return SkToInt((innerKey.dataSize() >> 2) + 1); } }; + +private: + SkAutoTUnref fData; }; /** diff --git a/src/core/SkPathPriv.h b/src/core/SkPathPriv.h index 934c730..51a1a80 100644 --- a/src/core/SkPathPriv.h +++ b/src/core/SkPathPriv.h @@ -59,6 +59,10 @@ public: int count = path.countVerbs(); return count >= 1 && path.fPathRef->verbs()[~(count - 1)] == SkPath::Verb::kClose_Verb; } + + static void AddGenIDChangeListener(const SkPath& path, SkPathRef::GenIDChangeListener* listener) { + path.fPathRef->addGenIDChangeListener(listener); + } }; #endif diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index 44838f9..38b4e71 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -23,12 +23,27 @@ SkPathRef::Editor::Editor(SkAutoTUnref* pathRef, pathRef->reset(copy); } fPathRef = *pathRef; + fPathRef->callGenIDChangeListeners(); fPathRef->fGenerationID = 0; SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);) } ////////////////////////////////////////////////////////////////////////////// +SkPathRef::~SkPathRef() { + this->callGenIDChangeListeners(); + SkDEBUGCODE(this->validate();) + sk_free(fPoints); + + SkDEBUGCODE(fPoints = NULL;) + SkDEBUGCODE(fVerbs = NULL;) + SkDEBUGCODE(fVerbCnt = 0x9999999;) + SkDEBUGCODE(fPointCnt = 0xAAAAAAA;) + SkDEBUGCODE(fPointCnt = 0xBBBBBBB;) + SkDEBUGCODE(fGenerationID = 0xEEEEEEEE;) + SkDEBUGCODE(fEditorsAttached = 0x7777777;) +} + // As a template argument, this must have external linkage. SkPathRef* sk_create_empty_pathref() { SkPathRef* empty = SkNEW(SkPathRef); @@ -154,6 +169,7 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer) { void SkPathRef::Rewind(SkAutoTUnref* pathRef) { if ((*pathRef)->unique()) { SkDEBUGCODE((*pathRef)->validate();) + (*pathRef)->callGenIDChangeListeners(); (*pathRef)->fBoundsIsDirty = true; // this also invalidates fIsFinite (*pathRef)->fVerbCnt = 0; (*pathRef)->fPointCnt = 0; @@ -215,13 +231,6 @@ bool SkPathRef::operator== (const SkPathRef& ref) const { SkASSERT(!genIDMatch); return false; } - // We've done the work to determine that these are equal. If either has a zero genID, copy - // the other's. If both are 0 then genID() will compute the next ID. - if (0 == fGenerationID) { - fGenerationID = ref.genID(); - } else if (0 == ref.fGenerationID) { - ref.fGenerationID = this->genID(); - } return true; } @@ -269,9 +278,6 @@ void SkPathRef::copy(const SkPathRef& ref, memcpy(this->verbsMemWritable(), ref.verbsMemBegin(), ref.fVerbCnt * sizeof(uint8_t)); memcpy(this->fPoints, ref.fPoints, ref.fPointCnt * sizeof(SkPoint)); fConicWeights = ref.fConicWeights; - // We could call genID() here to force a real ID (instead of 0). However, if we're making - // a copy then presumably we intend to make a modification immediately afterwards. - fGenerationID = ref.fGenerationID; fBoundsIsDirty = ref.fBoundsIsDirty; if (!fBoundsIsDirty) { fBounds = ref.fBounds; @@ -436,6 +442,24 @@ uint32_t SkPathRef::genID() const { return fGenerationID; } +void SkPathRef::addGenIDChangeListener(GenIDChangeListener* listener) { + if (NULL == listener || this == empty.get()) { + SkDELETE(listener); + return; + } + *fGenIDChangeListeners.append() = listener; +} + +// we need to be called *before* the genID gets changed or zerod +void SkPathRef::callGenIDChangeListeners() { + for (int i = 0; i < fGenIDChangeListeners.count(); i++) { + fGenIDChangeListeners[i]->onChange(); + } + + // Listeners get at most one shot, so whether these triggered or not, blow them away. + fGenIDChangeListeners.deleteAll(); +} + #ifdef SK_DEBUG void SkPathRef::validate() const { this->INHERITED::validate(); diff --git a/src/gpu/GrTessellatingPathRenderer.cpp b/src/gpu/GrTessellatingPathRenderer.cpp index d0c7d3d..282efe2 100644 --- a/src/gpu/GrTessellatingPathRenderer.cpp +++ b/src/gpu/GrTessellatingPathRenderer.cpp @@ -13,6 +13,8 @@ #include "GrDefaultGeoProcFactory.h" #include "GrPathUtils.h" #include "GrVertices.h" +#include "GrResourceCache.h" +#include "GrResourceProvider.h" #include "SkChunkAlloc.h" #include "SkGeometry.h" @@ -538,12 +540,13 @@ Vertex* generate_cubic_points(const SkPoint& p0, // Stage 1: convert the input path to a set of linear contours (linked list of Vertices). void path_to_contours(const SkPath& path, SkScalar tolerance, const SkRect& clipBounds, - Vertex** contours, SkChunkAlloc& alloc) { + Vertex** contours, SkChunkAlloc& alloc, bool *isLinear) { SkScalar toleranceSqd = tolerance * tolerance; SkPoint pts[4]; bool done = false; + *isLinear = true; SkPath::Iter iter(path, false); Vertex* prev = NULL; Vertex* head = NULL; @@ -571,6 +574,7 @@ void path_to_contours(const SkPath& path, SkScalar tolerance, const SkRect& clip toleranceSqd, prev, &head, pointsLeft, alloc); quadPts += 2; } + *isLinear = false; break; } case SkPath::kMove_Verb: @@ -590,12 +594,14 @@ void path_to_contours(const SkPath& path, SkScalar tolerance, const SkRect& clip int pointsLeft = GrPathUtils::quadraticPointCount(pts, tolerance); prev = generate_quadratic_points(pts[0], pts[1], pts[2], toleranceSqd, prev, &head, pointsLeft, alloc); + *isLinear = false; break; } case SkPath::kCubic_Verb: { int pointsLeft = GrPathUtils::cubicPointCount(pts, tolerance); prev = generate_cubic_points(pts[0], pts[1], pts[2], pts[3], toleranceSqd, prev, &head, pointsLeft, alloc); + *isLinear = false; break; } case SkPath::kClose_Verb: @@ -1329,6 +1335,25 @@ SkPoint* polys_to_triangles(Poly* polys, SkPath::FillType fillType, SkPoint* dat return d; } +struct TessInfo { + SkScalar fTolerance; + int fCount; +}; + +bool cache_match(GrVertexBuffer* vertexBuffer, SkScalar tol, int* actualCount) { + if (!vertexBuffer) { + return false; + } + const SkData* data = vertexBuffer->getUniqueKey().getCustomData(); + SkASSERT(data); + const TessInfo* info = static_cast(data->data()); + if (info->fTolerance == 0 || info->fTolerance < 3.0f * tol) { + *actualCount = info->fCount; + return true; + } + return false; +} + }; GrTessellatingPathRenderer::GrTessellatingPathRenderer() { @@ -1342,6 +1367,22 @@ GrPathRenderer::StencilSupport GrTessellatingPathRenderer::onGetStencilSupport( return GrPathRenderer::kNoSupport_StencilSupport; } +namespace { + +// When the SkPathRef genID changes, invalidate a corresponding GrResource described by key. +class PathInvalidator : public SkPathRef::GenIDChangeListener { +public: + explicit PathInvalidator(const GrUniqueKey& key) : fMsg(key) {} +private: + GrUniqueKeyInvalidatedMessage fMsg; + + void onChange() override { + SkMessageBus::Post(fMsg); + } +}; + +} // namespace + bool GrTessellatingPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const { // This path renderer can draw all fill styles, but does not do antialiasing. It can do convex // and concave paths, but we'll leave the convex ones to simpler algorithms. @@ -1377,7 +1418,9 @@ public: fPipelineInfo = init; } - void generateGeometry(GrBatchTarget* batchTarget, const GrPipeline* pipeline) override { + int tessellate(GrUniqueKey* key, + GrResourceProvider* resourceProvider, + SkAutoTUnref& vertexBuffer) { SkRect pathBounds = fPath.getBounds(); Comparator c; if (pathBounds.width() > pathBounds.height()) { @@ -1392,11 +1435,11 @@ public: int contourCnt; int maxPts = GrPathUtils::worstCasePointCount(fPath, &contourCnt, tol); if (maxPts <= 0) { - return; + return 0; } if (maxPts > ((int)SK_MaxU16 + 1)) { SkDebugf("Path not rendered, too many verts (%d)\n", maxPts); - return; + return 0; } SkPath::FillType fillType = fPath.getFillType(); if (SkPath::IsInverseFillType(fillType)) { @@ -1404,26 +1447,6 @@ public: } LOG("got %d pts, %d contours\n", maxPts, contourCnt); - SkAutoTUnref gp; - { - using namespace GrDefaultGeoProcFactory; - - Color color(fColor); - LocalCoords localCoords(fPipelineInfo.readsLocalCoords() ? - LocalCoords::kUsePosition_Type : - LocalCoords::kUnused_Type); - Coverage::Type coverageType; - if (fPipelineInfo.readsCoverage()) { - coverageType = Coverage::kSolid_Type; - } else { - coverageType = Coverage::kNone_Type; - } - Coverage coverage(coverageType); - gp.reset(GrDefaultGeoProcFactory::Create(color, coverage, localCoords, - fViewMatrix)); - } - batchTarget->initDraw(gp, pipeline); - SkAutoTDeleteArray contours(SkNEW_ARRAY(Vertex *, contourCnt)); // For the initial size of the chunk allocator, estimate based on the point count: @@ -1431,7 +1454,8 @@ public: // resulting Polys, since the same point may end up in two Polys. Assume minimal // connectivity of one Edge per Vertex (will grow for intersections). SkChunkAlloc alloc(maxPts * (3 * sizeof(Vertex) + sizeof(Edge))); - path_to_contours(fPath, tol, fClipBounds, contours.get(), alloc); + bool isLinear; + path_to_contours(fPath, tol, fClipBounds, contours.get(), alloc, &isLinear); Poly* polys; polys = contours_to_polys(contours.get(), contourCnt, c, alloc); int count = 0; @@ -1441,34 +1465,93 @@ public: } } if (0 == count) { - return; + return 0; } - size_t stride = gp->getVertexStride(); - SkASSERT(stride == sizeof(SkPoint)); - const GrVertexBuffer* vertexBuffer; - int firstVertex; - SkPoint* verts = static_cast( - batchTarget->makeVertSpace(stride, count, &vertexBuffer, &firstVertex)); - if (!verts) { + size_t size = count * sizeof(SkPoint); + if (!vertexBuffer.get() || vertexBuffer->gpuMemorySize() < size) { + vertexBuffer.reset(resourceProvider->createVertexBuffer( + size, GrResourceProvider::kStatic_BufferUsage, 0)); + } + if (!vertexBuffer.get()) { SkDebugf("Could not allocate vertices\n"); - return; + return 0; } - + SkPoint* verts = static_cast(vertexBuffer->map()); LOG("emitting %d verts\n", count); SkPoint* end = polys_to_triangles(polys, fillType, verts); + vertexBuffer->unmap(); int actualCount = static_cast(end - verts); LOG("actual count: %d\n", actualCount); SkASSERT(actualCount <= count); + if (!fPath.isVolatile()) { + TessInfo info; + info.fTolerance = isLinear ? 0 : tol; + info.fCount = actualCount; + SkAutoTUnref data(SkData::NewWithCopy(&info, sizeof(info))); + key->setCustomData(data.get()); + resourceProvider->assignUniqueKeyToResource(*key, vertexBuffer.get()); + SkPathPriv::AddGenIDChangeListener(fPath, SkNEW(PathInvalidator(*key))); + } + return actualCount; + } + + void generateGeometry(GrBatchTarget* batchTarget, const GrPipeline* pipeline) override { + // construct a cache key from the path's genID and the view matrix + static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain(); + GrUniqueKey key; + int clipBoundsSize32 = + fPath.isInverseFillType() ? sizeof(fClipBounds) / sizeof(uint32_t) : 0; + GrUniqueKey::Builder builder(&key, kDomain, 2 + clipBoundsSize32); + builder[0] = fPath.getGenerationID(); + builder[1] = fPath.getFillType(); + // For inverse fills, the tessellation is dependent on clip bounds. + if (fPath.isInverseFillType()) { + memcpy(&builder[2], &fClipBounds, sizeof(fClipBounds)); + } + builder.finish(); + GrResourceProvider* rp = batchTarget->resourceProvider(); + SkAutoTUnref vertexBuffer(rp->findAndRefTByUniqueKey(key)); + int actualCount; + SkScalar screenSpaceTol = GrPathUtils::kDefaultTolerance; + SkScalar tol = GrPathUtils::scaleToleranceToSrc( + screenSpaceTol, fViewMatrix, fPath.getBounds()); + if (!cache_match(vertexBuffer.get(), tol, &actualCount)) { + actualCount = tessellate(&key, rp, vertexBuffer); + } + + if (actualCount == 0) { + return; + } + + SkAutoTUnref gp; + { + using namespace GrDefaultGeoProcFactory; + + Color color(fColor); + LocalCoords localCoords(fPipelineInfo.readsLocalCoords() ? + LocalCoords::kUsePosition_Type : + LocalCoords::kUnused_Type); + Coverage::Type coverageType; + if (fPipelineInfo.readsCoverage()) { + coverageType = Coverage::kSolid_Type; + } else { + coverageType = Coverage::kNone_Type; + } + Coverage coverage(coverageType); + gp.reset(GrDefaultGeoProcFactory::Create(color, coverage, localCoords, + fViewMatrix)); + } + + batchTarget->initDraw(gp, pipeline); + SkASSERT(gp->getVertexStride() == sizeof(SkPoint)); + GrPrimitiveType primitiveType = WIREFRAME ? kLines_GrPrimitiveType : kTriangles_GrPrimitiveType; GrVertices vertices; - vertices.init(primitiveType, vertexBuffer, firstVertex, actualCount); + vertices.init(primitiveType, vertexBuffer.get(), 0, actualCount); batchTarget->draw(vertices); - - batchTarget->putBackVertices((size_t)(count - actualCount), stride); - return; } bool onCombineIfPossible(GrBatch*) override { diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 8272692..b0bd667 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -3715,6 +3715,21 @@ static void test_dump(skiatest::Reporter* reporter) { "path.lineTo(3, 4);\n"); } +namespace { + +class ChangeListener : public SkPathRef::GenIDChangeListener { +public: + ChangeListener(bool *changed) : fChanged(changed) { *fChanged = false; } + virtual ~ChangeListener() {} + void onChange() override { + *fChanged = true; + } +private: + bool* fChanged; +}; + +} + class PathTest_Private { public: static void TestPathTo(skiatest::Reporter* reporter) { @@ -3734,6 +3749,50 @@ public: SkRect reverseExpected = {-4, -4, 8, 8}; REPORTER_ASSERT(reporter, p.getBounds() == reverseExpected); } + + static void TestPathrefListeners(skiatest::Reporter* reporter) { + SkPath p; + + bool changed = false; + p.moveTo(0, 0); + + // Check that listener is notified on moveTo(). + + SkPathPriv::AddGenIDChangeListener(p, SkNEW(ChangeListener(&changed))); + REPORTER_ASSERT(reporter, !changed); + p.moveTo(10, 0); + REPORTER_ASSERT(reporter, changed); + + // Check that listener is notified on lineTo(). + SkPathPriv::AddGenIDChangeListener(p, SkNEW(ChangeListener(&changed))); + REPORTER_ASSERT(reporter, !changed); + p.lineTo(20, 0); + REPORTER_ASSERT(reporter, changed); + + // Check that listener is notified on reset(). + SkPathPriv::AddGenIDChangeListener(p, SkNEW(ChangeListener(&changed))); + REPORTER_ASSERT(reporter, !changed); + p.reset(); + REPORTER_ASSERT(reporter, changed); + + p.moveTo(0, 0); + + // Check that listener is notified on rewind(). + SkPathPriv::AddGenIDChangeListener(p, SkNEW(ChangeListener(&changed))); + REPORTER_ASSERT(reporter, !changed); + p.rewind(); + REPORTER_ASSERT(reporter, changed); + + // Check that listener is notified when pathref is deleted. + { + SkPath q; + q.moveTo(10, 10); + SkPathPriv::AddGenIDChangeListener(q, SkNEW(ChangeListener(&changed))); + REPORTER_ASSERT(reporter, !changed); + } + // q went out of scope. + REPORTER_ASSERT(reporter, changed); + } }; DEF_TEST(Paths, reporter) { @@ -3880,6 +3939,7 @@ DEF_TEST(Paths, reporter) { test_contains(reporter); PathTest_Private::TestPathTo(reporter); PathRefTest_Private::TestPathRef(reporter); + PathTest_Private::TestPathrefListeners(reporter); test_dump(reporter); test_path_crbug389050(reporter); test_path_crbugskia2820(reporter); diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index b795b01..43f0870 100644 --- a/tests/ResourceCacheTest.cpp +++ b/tests/ResourceCacheTest.cpp @@ -845,6 +845,21 @@ static void test_duplicate_unique_key(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, 0 == cache->getResourceCount()); REPORTER_ASSERT(reporter, 0 == cache->getResourceBytes()); REPORTER_ASSERT(reporter, 0 == TestResource::NumAlive()); + + { + GrUniqueKey key2; + make_unique_key<0>(&key2, 0); + SkAutoTUnref d(SkNEW_ARGS(TestResource, (context->getGpu()))); + int foo = 4132; + SkAutoTUnref data(SkData::NewWithCopy(&foo, sizeof(foo))); + key2.setCustomData(data.get()); + d->resourcePriv().setUniqueKey(key2); + } + + GrUniqueKey key3; + make_unique_key<0>(&key3, 0); + SkAutoTUnref d2(cache->findAndRefUniqueResource(key3)); + REPORTER_ASSERT(reporter, *(int*) d2->getUniqueKey().getCustomData()->data() == 4132); } static void test_purge_invalidated(skiatest::Reporter* reporter) { @@ -1223,6 +1238,21 @@ static void test_large_resource_count(skiatest::Reporter* reporter) { } } +static void test_custom_data(skiatest::Reporter* reporter) { + GrUniqueKey key1, key2; + make_unique_key<0>(&key1, 1); + make_unique_key<0>(&key2, 2); + int foo = 4132; + SkAutoTUnref data(SkData::NewWithCopy(&foo, sizeof(foo))); + key1.setCustomData(data.get()); + REPORTER_ASSERT(reporter, *(int*) key1.getCustomData()->data() == 4132); + REPORTER_ASSERT(reporter, key2.getCustomData() == nullptr); + + // Test that copying a key also takes a ref on its custom data. + GrUniqueKey key3 = key1; + REPORTER_ASSERT(reporter, *(int*) key3.getCustomData()->data() == 4132); +} + //////////////////////////////////////////////////////////////////////////////// DEF_GPUTEST(ResourceCache, reporter, factory) { for (int type = 0; type < GrContextFactory::kLastGLContextType; ++type) { @@ -1262,6 +1292,7 @@ DEF_GPUTEST(ResourceCache, reporter, factory) { test_timestamp_wrap(reporter); test_flush(reporter); test_large_resource_count(reporter); + test_custom_data(reporter); } #endif -- 2.7.4