[PDF] Optimize W-array generation with respect to subsetted fonts and add test.

[PDF] Optimize W-array generation with respect to subsetted fonts and add test.

Testing framework originally from http://codereview.appspot.com/4916044

Committed: http://code.google.com/p/skia/source/detail?r=2667

[arthurhsu, 2011-11-09 23:28:33]

On 2011/11/09 21:44:17, Steve VanDeBogart wrote:

Data {1, 0, 0, 0, 0, 2, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5}
subset {5, 7, 9, 11}

causes assertion error.  You may want to add my test cases in
http://codereview.appspot.com/4916044/diff/19002/tests/WArrayTest.cpp to
WArrayTest.cpp.

[Steve VanDeBogart, 2011-11-10 22:56:09]

On 2011/11/09 23:28:33, arthurhsu wrote:
> On 2011/11/09 21:44:17, Steve VanDeBogart wrote:
> 
> Data {1, 0, 0, 0, 0, 2, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5}
> subset {5, 7, 9, 11}
> 
> causes assertion error.  You may want to add my test cases in
> http://codereview.appspot.com/4916044/diff/19002/tests/WArrayTest.cpp to
> WArrayTest.cpp.

I fixed this case and a couple more I found by running the code in Chrome.  I
added tests for the cases that I fixed.  Adding the older tests doesn't make
sense because they are based on different assumptions about optimization. 
However, I did run them all and they all gave new expected output.

[arthurhsu, 2011-11-10 23:19:53]

On 2011/11/10 22:56:09, Steve VanDeBogart wrote:
> On 2011/11/09 23:28:33, arthurhsu wrote:
> > On 2011/11/09 21:44:17, Steve VanDeBogart wrote:
> > 
> > Data {1, 0, 0, 0, 0, 2, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5}
> > subset {5, 7, 9, 11}
> > 
> > causes assertion error.  You may want to add my test cases in
> > http://codereview.appspot.com/4916044/diff/19002/tests/WArrayTest.cpp to
> > WArrayTest.cpp.
> 
> I fixed this case and a couple more I found by running the code in Chrome.  I
> added tests for the cases that I fixed.  Adding the older tests doesn't make
> sense because they are based on different assumptions about optimization. 
> However, I did run them all and they all gave new expected output.

LGTM with nits.

Comments for test cases shall provide some mappings to the statements made in
getAdvanceData.  For example, "Removing 4 0's or don't care's is a win" is
validated in test case data 5.

[Steve VanDeBogart, 2011-11-11 19:37:26]

On 2011/11/10 23:19:53, arthurhsu wrote:
> On 2011/11/10 22:56:09, Steve VanDeBogart wrote:
> > On 2011/11/09 23:28:33, arthurhsu wrote:
> > > On 2011/11/09 21:44:17, Steve VanDeBogart wrote:
> > > 
> > > Data {1, 0, 0, 0, 0, 2, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5}
> > > subset {5, 7, 9, 11}
> > > 
> > > causes assertion error.  You may want to add my test cases in
> > > http://codereview.appspot.com/4916044/diff/19002/tests/WArrayTest.cpp to
> > > WArrayTest.cpp.
> > 
> > I fixed this case and a couple more I found by running the code in Chrome. 
I
> > added tests for the cases that I fixed.  Adding the older tests doesn't make
> > sense because they are based on different assumptions about optimization. 
> > However, I did run them all and they all gave new expected output.
> 
> LGTM with nits.
> 
> Comments for test cases shall provide some mappings to the statements made in
> getAdvanceData.  For example, "Removing 4 0's or don't care's is a win" is
> validated in test case data 5.

Done.

[arthurhsu, 2011-11-11 19:39:10]

On 2011/11/11 19:37:26, Steve VanDeBogart wrote:
> On 2011/11/10 23:19:53, arthurhsu wrote:
> > On 2011/11/10 22:56:09, Steve VanDeBogart wrote:
> > > On 2011/11/09 23:28:33, arthurhsu wrote:
> > > > On 2011/11/09 21:44:17, Steve VanDeBogart wrote:
> > > > 
> > > > Data {1, 0, 0, 0, 0, 2, 3, 3, 3, 3, 4, 5, 5, 5, 5, 5}
> > > > subset {5, 7, 9, 11}
> > > > 
> > > > causes assertion error.  You may want to add my test cases in
> > > > http://codereview.appspot.com/4916044/diff/19002/tests/WArrayTest.cpp to
> > > > WArrayTest.cpp.
> > > 
> > > I fixed this case and a couple more I found by running the code in Chrome.

> I
> > > added tests for the cases that I fixed.  Adding the older tests doesn't
make
> > > sense because they are based on different assumptions about optimization. 
> > > However, I did run them all and they all gave new expected output.
> > 
> > LGTM with nits.
> > 
> > Comments for test cases shall provide some mappings to the statements made
in
> > getAdvanceData.  For example, "Removing 4 0's or don't care's is a win" is
> > validated in test case data 5.
> 
> Done.

lgtm++