[PDF] Fix multi-level page count calculation.

[PDF] Fix multi-level page count calculation.

BUG=1091

Committed: https://code.google.com/p/skia/source/detail?r=7478

[reed1, 2013-01-28 14:40:21]

https://codereview.appspot.com/7220045/diff/2001/src/pdf/SkPDFPage.cpp
File src/pdf/SkPDFPage.cpp (right):

https://codereview.appspot.com/7220045/diff/2001/src/pdf/SkPDFPage.cpp#newcod...
src/pdf/SkPDFPage.cpp:115: newNode->insert(kidsName.get(), kids.get());
The following 3 lines are opaque to me. Can you add a comment that explains what
is going on? (including how treecapacity fits in)

[Steve VanDeBogart, 2013-01-28 22:47:23]

https://codereview.appspot.com/7220045/diff/2001/src/pdf/SkPDFPage.cpp
File src/pdf/SkPDFPage.cpp (right):

https://codereview.appspot.com/7220045/diff/2001/src/pdf/SkPDFPage.cpp#newcod...
src/pdf/SkPDFPage.cpp:115: newNode->insert(kidsName.get(), kids.get());
On 2013/01/28 14:40:21, reed1 wrote:
> The following 3 lines are opaque to me. Can you add a comment that explains
what
> is going on? (including how treecapacity fits in)

Done.

[edisonn, 2013-01-29 13:43:48]

https://codereview.appspot.com/7220045/diff/7001/src/pdf/SkPDFPage.cpp
File src/pdf/SkPDFPage.cpp (right):

https://codereview.appspot.com/7220045/diff/7001/src/pdf/SkPDFPage.cpp#newcod...
src/pdf/SkPDFPage.cpp:116: // of the tree the code is at (i.e. 8, 64, 512, ...).
If the
can we refer to it as the subtree instead of tree? It gives the impression that
we talk about the whole global tree, while here we deal with the current
subtree.

Note please that depth is the depth of the new subtree we build on top of
subtrees already build.

also mention that this is just an optimization, so we don't have to count how
many children we have, since in a balanced tree, all nodes in a row, except the
last one, will have k^n  children, where k is number of kids of a node, and n is
the depth of the subtree from this node.

[edisonn, 2013-01-29 13:48:25]

BTW, Note to myself mostly, we should make this a parameter and run
performance tests to determine optimum children_per_node


On Tue, Jan 29, 2013 at 8:43 AM, <edisonn@google.com> wrote:

>
>
https://codereview.appspot.**com/7220045/diff/7001/src/pdf/**SkPDFPage.cpp<ht...
> File src/pdf/SkPDFPage.cpp (right):
>
> https://codereview.appspot.**com/7220045/diff/7001/src/pdf/**
>
SkPDFPage.cpp#newcode116<https://codereview.appspot.com/7220045/diff/7001/src/pdf/SkPDFPage.cpp#newcode116>
> src/pdf/SkPDFPage.cpp:116: // of the tree the code is at (i.e. 8, 64,
> 512, ...). If the
> can we refer to it as the subtree instead of tree? It gives the
> impression that we talk about the whole global tree, while here we deal
> with the current subtree.
>
> Note please that depth is the depth of the new subtree we build on top
> of subtrees already build.
>
> also mention that this is just an optimization, so we don't have to
> count how many children we have, since in a balanced tree, all nodes in
> a row, except the last one, will have k^n  children, where k is number
> of kids of a node, and n is the depth of the subtree from this node.
>
>
https://codereview.appspot.**com/7220045/<https://codereview.appspot.com/7220...
>

[Steve VanDeBogart, 2013-01-29 20:09:49]

On 2013/01/29 13:48:25, edisonn wrote:
> BTW, Note to myself mostly, we should make this a parameter and run
> performance tests to determine optimum children_per_node

This is going to be specific to each viewer, so there is no optimal.  I guess we
could see what value Acrobat Distiller uses - that's what most viewers will
expect..


> On Tue, Jan 29, 2013 at 8:43 AM, <mailto:edisonn@google.com> wrote:
> 
> >
> >
>
https://codereview.appspot.**com/7220045/diff/7001/src/pdf/**SkPDFPage.cpp%3C...>
> > File src/pdf/SkPDFPage.cpp (right):
> >
> > https://codereview.appspot.**com/7220045/diff/7001/src/pdf/**
> >
>
SkPDFPage.cpp#newcode116<https://codereview.appspot.com/7220045/diff/7001/src/pdf/SkPDFPage.cpp#newcode116>
> > src/pdf/SkPDFPage.cpp:116: // of the tree the code is at (i.e. 8, 64,
> > 512, ...). If the
> > can we refer to it as the subtree instead of tree? It gives the
> > impression that we talk about the whole global tree, while here we deal
> > with the current subtree.
> >
> > Note please that depth is the depth of the new subtree we build on top
> > of subtrees already build.
> >
> > also mention that this is just an optimization, so we don't have to
> > count how many children we have, since in a balanced tree, all nodes in
> > a row, except the last one, will have k^n  children, where k is number
> > of kids of a node, and n is the depth of the subtree from this node.

Comment updated based on your feedback.  If you'd like further changes, please
suggest alternate text.

[edisonn, 2013-01-30 19:21:14]

lgtm