Hi Jim. > Actually I was thinking it would be OK to leave the endpoints in local > variables, but it's not worth twiddling with at this point. The > changes > look great and good to go.
Nice! I will push later today. > Were these intended to be separate pushes? Are the independent bugs? > Do you need me to massage any bugids for you? They are 2 separate, independent bugs, so I think it would be best if they were separate pushes. I think the bugids I already have are ok. Thanks for reviewing, Denis. ----- Original Message ----- > Hi Denis, > > > > ...jim > > On 1/13/11 9:50 AM, Denis Lila wrote: > > Hi Jim. > > > >> One general comment about the new helper method. I probably > >> wouldn't > >> bother loading the control points into local variables since you > >> only > >> use them once in the function. It might be wasted effort if the > >> cubic > >> function isn't called, and meanwhile you are forcing the compiler > >> to > >> find some local storage to stuff them into for no good reason (the > >> compiler can't optimize those fetches out or around since it has no > >> concept of the potential side effects, or lack thereof, of calling > >> the > >> abstract getters)... > > > >> Also, on the testing of the return value, I wouldn't bother with > >> testing > >> "% 2". If you look at Path2D it just assumes that it is an even > >> number > >> (or the INTERSECT constant) and does the test based on whether it > >> is > >> INTERSECT or non-zero (for WIND_NON_ZERO which is compatible with > >> CubicCurve and QuadCurve - I don't think there can be interior > >> holes > >> in > >> a either single curve's outline)... > > > > I did both. I updated the webrevs: > > http://icedtea.classpath.org/~dlila/webrevs/containsFix/webrev/ > > http://icedtea.classpath.org/~dlila/webrevs/intersectsFix/webrev/ > > > > Thank you, > > Denis.
