I was aware that the code was playing with overflow fire when I ported
it a while back. I figured the real fix was to eventually switch to
float instead, as per your last suggestion - at least for the line
widening code. Any takers?
...jim
Roman Kennke wrote:
Hi there,
4. StrokeShapeTest: createStrokedShape() behaves differently.
It turns out that there is an arithmetic overflow here. The pisces
stroker does a stupid thing here. First it initializes the
scaledLineWidth like this:
this.scaledLineWidth2 = ((long)transform.m00*lineWidth2);
which is infact wrong, because in fixed point arithmetics you need to
apply >> 16, because the decimal moves.
However, in another place it uses this factor like this:
dx = (int)( (ly*scaledLineWidth2)/ilen >> 16);
dy = (int)(-(lx*scaledLineWidth2)/ilen >> 16);
which makes it ok in the end. The only problem is, that we start with an
incredibly large number, then multiply another incredibly large number
and _after_ that (when the overflow already occured) do the >> 16. The
patch moves the >> 16 to the initialization of scaledLineWidth, which
makes it clearer, more correct and even a tad faster, because we do the
second calculation at least 2x.
Of course, the real fix here would be to turn the whole implementation
into floating point. This particular testcase is fixed by this patch,
and the 'range of correct behaviour' is much large now, but if you deal
with very large numbers in your shapes, then you will get trouble again.
/Roman
