Update GrTTopoSort to use pre-existing memory ...

... rather than allocating an extra "result" array.

This CL also changes the sort algorithm to soldier on even after
it has found a loop.

Change-Id: I03fe8da1aade6c9461eb42e1b7d79fae562210f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335824
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Robert Phillips 2020-11-18 12:16:31 -05:00 committed by Skia Commit-Bot
parent 0d66b25fc1
commit 4dbff75f6a
3 changed files with 63 additions and 27 deletions

View File

@ -169,6 +169,17 @@ protected:
return SkToBool(fFlags & flag);
}
void setIndex(uint32_t index) {
SkASSERT(!this->isSetFlag(kWasOutput_Flag));
SkASSERT(index < (1 << 28));
fFlags |= index << 4;
}
uint32_t getIndex() const {
SkASSERT(this->isSetFlag(kWasOutput_Flag));
return fFlags >> 4;
}
private:
// for TopoSortTraits, fTextureResolveTask, closeThoseWhoDependOnMe, addDependency
friend class GrDrawingManager;
@ -186,7 +197,11 @@ private:
static uint32_t CreateUniqueID();
struct TopoSortTraits {
static void Output(GrRenderTask* renderTask, int /* index */) {
static uint32_t GetIndex(GrRenderTask* renderTask) {
return renderTask->getIndex();
}
static void Output(GrRenderTask* renderTask, uint32_t index) {
renderTask->setIndex(index);
renderTask->setFlag(kWasOutput_Flag);
}
static bool WasOutput(const GrRenderTask* renderTask) {

View File

@ -25,6 +25,7 @@ void GrTTopoSort_CleanExit(const SkTArray<sk_sp<T>>& graph) {
for (int i = 0; i < graph.count(); ++i) {
SkASSERT(!Traits::IsTempMarked(graph[i].get()));
SkASSERT(Traits::WasOutput(graph[i].get()));
SkASSERT(Traits::GetIndex(graph[i].get()) == (uint32_t) i);
}
}
#endif
@ -32,12 +33,14 @@ void GrTTopoSort_CleanExit(const SkTArray<sk_sp<T>>& graph) {
// Recursively visit a node and all the other nodes it depends on.
// Return false if there is a loop.
template <typename T, typename Traits = T>
bool GrTTopoSort_Visit(T* node, SkTArray<sk_sp<T>>* result) {
bool GrTTopoSort_Visit(T* node, uint32_t* counter) {
if (Traits::IsTempMarked(node)) {
// There is a loop.
return false;
}
bool succeeded = true;
// If the node under consideration has been already been output it means it
// (and all the nodes it depends on) are already in 'result'.
if (!Traits::WasOutput(node)) {
@ -45,17 +48,16 @@ bool GrTTopoSort_Visit(T* node, SkTArray<sk_sp<T>>* result) {
// nodes it depends on outputing them first.
Traits::SetTempMark(node);
for (int i = 0; i < Traits::NumDependencies(node); ++i) {
if (!GrTTopoSort_Visit<T, Traits>(Traits::Dependency(node, i), result)) {
return false;
if (!GrTTopoSort_Visit<T, Traits>(Traits::Dependency(node, i), counter)) {
succeeded = false;
}
}
Traits::Output(node, result->count()); // mark this node as output
Traits::Output(node, *counter); // mark this node as output
++(*counter);
Traits::ResetTempMark(node);
result->push_back(sk_ref_sp(node));
}
return true;
return succeeded;
}
// Topologically sort the nodes in 'graph'. For this sort, when node 'i' depends
@ -64,8 +66,9 @@ bool GrTTopoSort_Visit(T* node, SkTArray<sk_sp<T>>* result) {
// be in some arbitrary state.
//
// Traits requires:
// static void Output(T* t, int index) { ... } // 'index' is 't's position in the result
// static void Output(T* t, uint32_t index) { ... } // 'index' is 't's position in the result
// static bool WasOutput(const T* t) { ... }
// static uint32_t GetIndex() { ... }
//
// static void SetTempMark(T* t) { ... } // transiently used during toposort
// static void ResetTempMark(T* t) { ... }
@ -80,13 +83,13 @@ bool GrTTopoSort_Visit(T* node, SkTArray<sk_sp<T>>* result) {
// flush a GrRenderTask DAG.
template <typename T, typename Traits = T>
bool GrTTopoSort(SkTArray<sk_sp<T>>* graph) {
SkTArray<sk_sp<T>> result;
uint32_t counter = 0;
#ifdef SK_DEBUG
GrTTopoSort_CheckAllUnmarked<T, Traits>(*graph);
#endif
result.reserve_back(graph->count());
bool succeeded = true;
for (int i = 0; i < graph->count(); ++i) {
if (Traits::WasOutput((*graph)[i].get())) {
@ -96,18 +99,26 @@ bool GrTTopoSort(SkTArray<sk_sp<T>>* graph) {
}
// Output this node after all the nodes it depends on have been output.
if (!GrTTopoSort_Visit<T, Traits>((*graph)[i].get(), &result)) {
return false;
if (!GrTTopoSort_Visit<T, Traits>((*graph)[i].get(), &counter)) {
succeeded = false;
}
}
SkASSERT(graph->count() == result.count());
graph->swap(result);
SkASSERT(counter == (uint32_t) graph->count());
// Reorder the array given the output order
for (uint32_t i = 0; i < (uint32_t) graph->count(); ++i) {
for (uint32_t correctIndex = Traits::GetIndex((*graph)[i].get());
correctIndex != i;
correctIndex = Traits::GetIndex((*graph)[i].get())) {
(*graph)[i].swap((*graph)[correctIndex]);
}
}
#ifdef SK_DEBUG
GrTTopoSort_CleanExit<T, Traits>(*graph);
#endif
return true;
return succeeded;
}
#endif

View File

@ -150,23 +150,30 @@ SkPath make_big_path();
// A helper object to test the topological sorting code (TopoSortBench.cpp & TopoSortTest.cpp)
class TopoTestNode : public SkRefCnt {
public:
TopoTestNode(int id) : fID(id), fOutputPos(-1), fTempMark(false) {}
TopoTestNode(int id) : fID(id) {}
void dependsOn(TopoTestNode* src) { *fDependencies.append() = src; }
int id() const { return fID; }
void reset() { fOutputPos = -1; }
void reset() {
fOutputPos = 0;
fTempMark = false;
fWasOutput = false;
}
int outputPos() const { return fOutputPos; }
uint32_t outputPos() const {
SkASSERT(fWasOutput);
return fOutputPos;
}
// check that the topological sort is valid for this node
bool check() {
if (-1 == fOutputPos) {
if (!fWasOutput) {
return false;
}
for (int i = 0; i < fDependencies.count(); ++i) {
if (-1 == fDependencies[i]->outputPos()) {
if (!fDependencies[i]->fWasOutput) {
return false;
}
// This node should've been output after all the nodes on which it depends
@ -182,11 +189,13 @@ public:
static void SetTempMark(TopoTestNode* node) { node->fTempMark = true; }
static void ResetTempMark(TopoTestNode* node) { node->fTempMark = false; }
static bool IsTempMarked(TopoTestNode* node) { return node->fTempMark; }
static void Output(TopoTestNode* node, int outputPos) {
SkASSERT(-1 != outputPos);
static void Output(TopoTestNode* node, uint32_t outputPos) {
SkASSERT(!node->fWasOutput);
node->fOutputPos = outputPos;
node->fWasOutput = true;
}
static bool WasOutput(TopoTestNode* node) { return (-1 != node->fOutputPos); }
static bool WasOutput(TopoTestNode* node) { return node->fWasOutput; }
static uint32_t GetIndex(TopoTestNode* node) { return node->outputPos(); }
static int NumDependencies(TopoTestNode* node) { return node->fDependencies.count(); }
static TopoTestNode* Dependency(TopoTestNode* node, int index) {
return node->fDependencies[index];
@ -220,9 +229,10 @@ public:
}
private:
int fID;
int fOutputPos;
bool fTempMark;
int fID;
uint32_t fOutputPos = 0;
bool fTempMark = false;
bool fWasOutput = false;
SkTDArray<TopoTestNode*> fDependencies;
};