Jim, Here is another webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144938.3/
Changes: - restored pathClosed flag to fix the 'hour glass' issue below - added pathClosedTest in CrashNaNTest Comments below: 2016-03-15 23:45 GMT+01:00 Jim Graham <[email protected]>: > The new tests look good. > Thanks. > There is one logic error in the new version of the degenerate coordinate > filter. > > It is valid to have a PATH_CLOSE immediately followed by a <geom>To call > and the subpath will start at the previous close/moveto point. For example: > > moveto 40, 40 > lineto 0, 0 > lineto 80, 0 > closepath > lineto 80, 80 > lineto 0, 80 > closepath > > should draw an hourglass, but with your fix it will lose the lower half > because the lineto after the close will be turned into a moveto (and the > second subpath then becomes empty. > I tested my previous patch and you were right: it was buggy. > I think it should be fine just getting rid of the subpath=false in the > close case. > I prefered reverting to the ductus approach even if your proposal seems working. I agree my patch is a bit tricky but it explicitely emits a moveTo() after pathClosed() instead of relying to the path consumer to implement the same behaviour (in dasher / stroker)... It is arguable, though, if we have closepath followed by a NaN if perhaps > that NaN should break the subpath from the previous close, but I don't > think that is in keeping with the current philosophy of simply pretending > that bad coordinates don't exist. Essentially, the close of a previously > valid path gives us a valid start to the new subpath regardless if it > immediately feeds into more bad coordinates - it still started its run on a > valid point. So, I'd argue that simply passing along the prior value of > the subpath flag would be appropriate, whether it is true or false (and, > obviously, also closing the path in the consumer if it is true)... > Please tell me what solution do you prefer. I can adopt yours which seems really simple. Laurent
