Hi Jennifer, Do you mind pasting the URL links to the patches in this thread? I'd like to backport them into our openjdk6 tree.
Thanks, Hiroshi On Mon, Apr 6, 2009 at 12:23 PM, Jennifer Godinez <jennifer.godi...@sun.com> wrote: > Hi Roman, > > Thanks for submitting the patches. I got all 4 now. I'm working on it but > Jim needs to review the changes. He's on vacation now so don't expect to > get anything very soon until he gets back. I will keep you updated. > > Jennifer > > Roman Kennke wrote: >> >> Hi Jennifer, >> >>> Apologies for not replying to you sooner. >> >> No problem. >> >>> As far as I know, Jim made comments on the fix. Have you looked into >>> these? >> >> Jim commented on the fix for bug #3 that I haven't looked at yet. I will >> do so tomorrow. >> >>> Whether or not you have, please go ahead and enter the patch into >>> Bugzilla so we can better keep track on them. >> >> I entered the 3 patches for the other 3 problems into bugzilla, and will >> do the same for the other as soon as I implemented Jim's suggestions. >> >> The bugs are 100030, 100031 and 100032 in bugzilla. >> >> Thanks, Roman >> >>> Jennifer >>> >>> Roman Kennke wrote: >>>> >>>> Hi, >>>> >>>> Is it possible to get an update on these patches or did the whole Java2D >>>> team just disappear? ;-) Should I enter the patches into Bugzilla or >>>> should I simply wait? What's happening with them now? >>>> >>>> /Roman >>>> >>>>> so what happens now with this patch and the others? Should I enter them >>>>> into Bugzilla, so they don't get lost? Are they already in the process >>>>> of beeing integrated? >>>>> >>>>> /Roman >>>>> >>>>> Am Dienstag, den 03.03.2009, 16:17 +0100 schrieb Roman Kennke: >>>>>> >>>>>> 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 >
