[PDF] Fix PDF primitives test for fixed scalars.

[PDF] Fix PDF primitives test for fixed scalars.

110999 is out of range for SkFixed (+/- 32767)

BUG=269

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

[epoger, 2011-05-18 13:21:45]

http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp
File tests/PDFPrimitivesTest.cpp (right):

http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp#newc...
tests/PDFPrimitivesTest.cpp:125: #if defined(SK_SCALAR_IS_FLOAT)
So for SK_SCALAR_IS_FIXED, we just won't check the output at all?  That seems
unfortunate.  Is there some reasonably simple way we could just change the test
to output a value that's less than 32767?

If not, I'm OK with this approach.

[Steve VanDeBogart, 2011-05-18 16:55:06]

http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp
File tests/PDFPrimitivesTest.cpp (right):

http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp#newc...
tests/PDFPrimitivesTest.cpp:125: #if defined(SK_SCALAR_IS_FLOAT)
On 2011/05/18 13:21:46, epoger wrote:
> So for SK_SCALAR_IS_FIXED, we just won't check the output at all?  That seems
> unfortunate.  Is there some reasonably simple way we could just change the
test
> to output a value that's less than 32767?

Correct, we won't run this test for SK_SCALAR_IS_FIXED.  See the definition of
SkIntToFixed; at least for debug builds, we assert that the input is in range. 
The point of this particular test is to test values larger than 32767, so it
just doesn't apply when SK_SCALAR_IS_FIXED.

When I added the test, apparently I compiled but didn't run it for the
SK_SCALAR_IS_FIXED case.  As you noted, it runs but fails, the implied
SkScalar(110999.75) effectively gets reinterpreted cast to SkFixed, giving it a
value of 1.6 something instead of the intended large result.

[epoger, 2011-05-18 17:03:48]

On 2011/05/18 16:55:06, Steve VanDeBogart wrote:
> http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp
> File tests/PDFPrimitivesTest.cpp (right):
> 
>
http://codereview.appspot.com/4530051/diff/1/tests/PDFPrimitivesTest.cpp#newc...
> tests/PDFPrimitivesTest.cpp:125: #if defined(SK_SCALAR_IS_FLOAT)
> On 2011/05/18 13:21:46, epoger wrote:
> > So for SK_SCALAR_IS_FIXED, we just won't check the output at all?  That
seems
> > unfortunate.  Is there some reasonably simple way we could just change the
> test
> > to output a value that's less than 32767?
> 
> Correct, we won't run this test for SK_SCALAR_IS_FIXED.  See the definition of
> SkIntToFixed; at least for debug builds, we assert that the input is in range.

> The point of this particular test is to test values larger than 32767, so it
> just doesn't apply when SK_SCALAR_IS_FIXED.
> 
> When I added the test, apparently I compiled but didn't run it for the
> SK_SCALAR_IS_FIXED case.  As you noted, it runs but fails, the implied
> SkScalar(110999.75) effectively gets reinterpreted cast to SkFixed, giving it
a
> value of 1.6 something instead of the intended large result.

Or you may not have been able to compile it with SK_SCALAR_IS_FIXED at all...
that had been suffering from compile-time breakage for some time, I think,
before I committed http://codereview.appspot.com/4532064/ yesterday.

[epoger, 2011-05-18 17:03:57]

LGTM