Changes to SkTDStackNester.

SkTDStackNester is a class used by PdfViewer to assist in saving
and restoring the PDF state. Clean up and test this class.

Add some documentation.

Add FIXME's where I have questions to resolve.

Fix a bug where fNestingLevel was not initialized.

Remove a commented out line of code copied over from
SkTDStack.

Rename SkTDStackNester::nests() to nestingLevel() and make it const.

Remove unnecessary predeclaration and friend declaration.

Remove index() (both const and non-const versions). They were
unused, return something that may not be expected (index from
the top, rather than from the bottom), and don't work to get any
elements in earlier Recs once the first one is full.

Report a warning if the nesting level goes above the maximum level,
or if we attempt to bring it below zero.

Prevent fNestingLevel from dropping below zero.

Add kUnusedObject_SkPdfIssue, and use it where appropriate.

Depends on https://codereview.chromium.org/64093009/

R=mtklein@google.com

Review URL: https://codereview.chromium.org/68843006

git-svn-id: http://skia.googlecode.com/svn/trunk@12328 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
scroggo@google.com 2013-11-20 21:40:57 +00:00
parent 178acd25b0
commit 7d8013f306
8 changed files with 218 additions and 32 deletions

View File

@ -34,6 +34,7 @@ enum SkPdfIssue {
kNoIssue_SkPdfIssue,
kNullObject_SkPdfIssue,
kUnusedObject_SkPdfIssue,
kUnexpectedArraySize_SkPdfIssue,
kMissingEncoding_SkPdfIssue,
kNYI_SkPdfIssue,

View File

@ -110,7 +110,7 @@ bool SkPdfNativeObject::filterStream() {
void SkPdfNativeObject::releaseData() {
#ifdef PDF_TRACK_OBJECT_USAGE
SkPdfReportIf(!fUsed, kInfo_SkPdfIssueSeverity, kNoIssue_SkPdfIssue,
SkPdfReportIf(!fUsed, kInfo_SkPdfIssueSeverity, kUnusedObject_SkPdfIssue,
"Unused object in rendering", this, NULL);
#endif // PDF_TRACK_OBJECT_USAGE

View File

@ -1133,7 +1133,7 @@ SkPdfResult PdfOp_Q(SkPdfContext* pdfContext, SkCanvas* canvas, PdfTokenLooper**
pdfContext->fStateStack.pop();
canvas->restore();
if (pdfContext->fObjectStack.nests() == 0) {
if (pdfContext->fObjectStack.nestingLevel() == 0) {
SkPdfReport(kIgnoreError_SkPdfIssueSeverity, kStackNestingOverflow_SkPdfIssue,
"stack nesting overflow (q/Q)", NULL, pdfContext);
return kIgnoreError_SkPdfResult;

View File

@ -9,25 +9,34 @@
#define SkTDStackNester_DEFINED
#include "SkTypes.h"
#include "SkPdfReporter.h"
// Adobe limits it to 28, so 256 should be more than enough
// Adobe limits it to 28. Allow deeper nesting in case a file does not quite meet the
// spec. 256 should be more than enough.
#define MAX_NESTING 256
/** \class SkTDStackNester
*
* Specialized version of SkTDStack which allows a stack of stacks.
* FIXME (scroggo): Could this be a subclass of SkTDStack? Could it have-a SkTDStack?
* The difference between SkTDStackNester and SkTDStack is that:
* - SkTDStackNester uses new/delete to manage initializations
* FIXME (scroggo): Why use new rather than malloc?
* - Supports nest/unnest which simulates a stack of stack. unnest will pop all the
* objects pushed since the last nest
* - kSlotCount is 64, instead of 8.
* FIXME (scroggo): How did we arrive at this number?
*/
template <typename T> class SkTDStackNester : SkNoncopyable {
public:
SkTDStackNester() : fCount(0), fTotalCount(0), fLocalCount(0) {
SkTDStackNester()
: fCount(0)
, fLocalCount(0)
, fNestingLevel(0) {
fInitialRec.fNext = NULL;
fRec = &fInitialRec;
// fCount = kSlotCount;
SkDEBUGCODE(fTotalCount = 0;)
}
~SkTDStackNester() {
@ -39,36 +48,73 @@ public:
}
}
/**
* Return the number of objects in the current nesting level.
*/
int count() const { return fLocalCount; }
/**
* Whether the current nesting level is empty.
*/
bool empty() const { return fLocalCount == 0; }
int nests() {
/**
* The current nesting level.
*/
int nestingLevel() const {
return fNestingLevel;
}
/**
* Analogous to an SkCanvas::save(). When unnest() is called, the state of this SkTDStackNester
* will return to its state when nest() was called.
*
* After a call to nest(), fLocalCount is reset to 0, since the stack is on a new nesting
* level.
*/
void nest() {
// We are are past max nesting levels, we will still continue to work, but we might fail
// to properly ignore errors. Ideally it should only mean poor rendering in exceptional
// cases
if (fNestingLevel >= 0 && fNestingLevel < MAX_NESTING) {
SkASSERT(fNestingLevel >= 0);
if (fNestingLevel < MAX_NESTING) {
fNestings[fNestingLevel] = fLocalCount;
fLocalCount = 0;
} else {
// We are are past max nesting levels. We will still continue to work, but we might fail
// to properly ignore errors. Ideally it should only mean poor rendering in exceptional
// cases.
SkPdfReport(kWarning_SkPdfIssueSeverity, kStackNestingOverflow_SkPdfIssue,
"Past maximum nesting level", NULL, NULL);
}
fNestingLevel++;
}
/**
* Analagous to an SkCanvas::restore(). Will revert this stack to the state it was in the last
* time nest() was called. It is an error to call unnest() more times than nest() has been
* called.
*/
void unnest() {
SkASSERT(fNestingLevel > 0);
SkASSERT(fNestingLevel >= 0);
if (0 == fNestingLevel) {
SkPdfReport(kWarning_SkPdfIssueSeverity, kStackNestingOverflow_SkPdfIssue,
"Nesting underflow", NULL, NULL);
return;
}
fNestingLevel--;
if (fNestingLevel >= 0 && fNestingLevel < MAX_NESTING) {
// TODO(edisonn): warn if fLocal > 0
if (fNestingLevel < MAX_NESTING) {
while (fLocalCount > 0) {
pop();
// FIXME (scroggo): Pass the object?
SkPdfReport(kInfo_SkPdfIssueSeverity, kUnusedObject_SkPdfIssue,
"Unused object when calling unnest!", NULL, NULL);
this->pop();
}
fLocalCount = fNestings[fNestingLevel];
}
}
/**
* Add an object to the stack, and return a pointer to it for modification.
*/
T* push() {
SkASSERT(fCount <= kSlotCount);
if (fCount == kSlotCount) {
@ -77,33 +123,35 @@ public:
fRec = rec;
fCount = 0;
}
++fTotalCount;
SkDEBUGCODE(++fTotalCount;)
++fLocalCount;
return &fRec->fSlots[fCount++];
}
/**
* Add an object to the stack, copied from elem.
*/
void push(const T& elem) { *this->push() = elem; }
const T& index(int idx) const {
SkASSERT(fRec && fCount > idx);
return fRec->fSlots[fCount - idx - 1];
}
T& index(int idx) {
SkASSERT(fRec && fCount > idx);
return fRec->fSlots[fCount - idx - 1];
}
/**
* Return the top element.
*/
const T& top() const {
SkASSERT(fRec && fCount > 0);
return fRec->fSlots[fCount - 1];
}
/**
* Return the top element.
*/
T& top() {
SkASSERT(fRec && fCount > 0);
return fRec->fSlots[fCount - 1];
}
/**
* Pop an object off the stack (via pop()), and copy its members into elem.
*/
void pop(T* elem) {
if (elem) {
*elem = fRec->fSlots[fCount - 1];
@ -111,10 +159,15 @@ public:
this->pop();
}
/**
* Pop an object off the stack. It is an error to call pop() more times
* than push() has been called in total or since the last call to nest().
*/
void pop() {
SkASSERT(fCount > 0 && fRec);
SkASSERT(fLocalCount > 0);
--fLocalCount;
--fTotalCount;
SkDEBUGCODE(--fTotalCount;)
if (--fCount == 0) {
if (fRec != &fInitialRec) {
Rec* rec = fRec->fNext;
@ -129,20 +182,33 @@ public:
private:
enum {
// Number of objects held per Rec. Storing multiple objects in one Rec
// means that we call new less often.
kSlotCount = 64
};
struct Rec;
friend struct Rec;
struct Rec {
Rec* fNext;
T fSlots[kSlotCount];
};
// First Rec, requiring no allocation.
Rec fInitialRec;
// The Rec on top of the stack.
Rec* fRec;
int fCount, fTotalCount, fLocalCount;
// Number of objects in fRec.
int fCount;
// Number of objects in the current nesting level.
int fLocalCount;
// Array of counts of objects per nesting level.
// Only valid for fNestings[0] through fNestings[fNestingLevel-1].
int fNestings[MAX_NESTING];
// Current nesting level.
int fNestingLevel;
// Total number of objects in this SkTDStackNester.
SkDEBUGCODE(int fTotalCount;)
// For testing.
friend class SkTDStackNesterTester;
};
#endif // SkTDStackNester_DEFINED

View File

@ -1,3 +1,4 @@
# GYP file to build experimental directory.
{
'targets': [
{

View File

@ -17,6 +17,10 @@
'../src/pipe/utils',
'../src/utils',
'../tools/',
# Needed for TDStackNesterTest.
'../experimental/PdfViewer',
'../experimental/PdfViewer/src',
],
'includes': [
'pathops_unittest.gypi',
@ -152,6 +156,9 @@
'../tests/Writer32Test.cpp',
'../tests/XfermodeTest.cpp',
'../experimental/PdfViewer/src/SkTDStackNester.h',
'../tests/TDStackNesterTest.cpp',
# Needed for PipeTest.
'../src/pipe/utils/SamplePipeControllers.cpp',
],

111
tests/TDStackNesterTest.cpp Normal file
View File

@ -0,0 +1,111 @@
/*
* Copyright 2013 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "SkTDStackNester.h"
#include "Test.h"
#include "TestClassDef.h"
/**
* Test SkTDStackNester<int>::push(). Pushes the current count onto the stack,
* and checks that the count has increased by one.
*/
static void test_push(skiatest::Reporter* reporter, SkTDStackNester<int>* nester) {
SkASSERT(nester);
const int count = nester->count();
// test_pop depends on this value.
nester->push(count);
REPORTER_ASSERT(reporter, nester->count() == count + 1);
}
/**
* Test SkTDStackNester<int>::pop(). Pops the top element off the stack, and
* checks that the new count is one smaller, and that the popped element
* matches the new count (as was pushed by test_push).
*/
static void test_pop(skiatest::Reporter* reporter, SkTDStackNester<int>* nester) {
SkASSERT(nester);
const int count = nester->count();
// This test should not be called with a count <= 0.
SkASSERT(count > 0);
const int top = nester->top();
int value = -1;
nester->pop(&value);
REPORTER_ASSERT(reporter, top == value);
const int newCount = nester->count();
REPORTER_ASSERT(reporter, newCount == count - 1);
// Since test_push always pushes the count prior to the push, value should
// always be one less than count.
REPORTER_ASSERT(reporter, newCount == value);
}
/**
* Test nest() and unnest(). nest() is called, and it is confirmed that the
* count is now zero. Then test_push() is called inc times, followed by a call to
* unnest(). After this call, check that the count has returned to the initial count, and
* that nestingLevel() has returned to its initial value.
*/
static void test_nest(skiatest::Reporter* reporter, SkTDStackNester<int>* nester, int inc) {
SkASSERT(nester);
SkASSERT(inc > 0);
const int initialCount = nester->count();
const int initialNesting = nester->nestingLevel();
nester->nest();
REPORTER_ASSERT(reporter, nester->count() == 0);
REPORTER_ASSERT(reporter, nester->nestingLevel() == initialNesting + 1);
for (int i = 0; i < inc; ++i) {
test_push(reporter, nester);
}
nester->unnest();
REPORTER_ASSERT(reporter, nester->count() == initialCount);
REPORTER_ASSERT(reporter, nester->nestingLevel() == initialNesting);
}
class SkTDStackNesterTester {
public:
static int GetSlotCount() {
return SkTDStackNester<int>::kSlotCount;
}
};
static void test_stack_nester(skiatest::Reporter* reporter) {
SkTDStackNester<int> nester;
int count = nester.count();
REPORTER_ASSERT(reporter, 0 == count);
REPORTER_ASSERT(reporter, nester.nestingLevel() == 0);
REPORTER_ASSERT(reporter, nester.empty());
// Test nesting (with arbitrary number of pushes) from the beginning.
test_nest(reporter, &nester, 3);
const int slotCount = SkTDStackNesterTester::GetSlotCount();
// Test pushing beyond the boundary of the first Rec.
for (; count < 2 * slotCount; ++count) {
if (3 == count) {
// Test nesting (an arbitrary number of pushes) early on.
test_nest(reporter, &nester, 7);
} else if (slotCount - 4 == count) {
// Test nesting across the boundary of a Rec.
test_nest(reporter, &nester, 6);
}
test_push(reporter, &nester);
}
// Pop everything off the stack except for the last one, to confirm
// that the destructor handles a remaining object.
while (nester.count() > 1) {
test_pop(reporter, &nester);
}
}
DEF_TEST(TDStackNester, reporter) {
test_stack_nester(reporter);
}