Community
Participate
Working Groups
1. PrecisionPoint. Constructors PrecisionPoint(Point copy) and PrecisionPoint(double x, double y) may result in diffent x and y values for PrecisionPoint Suppose preciseX = N.999999... preciseY = M.99999.... Hence x = N + 1 and y = M + 1. If this is copied using PrecisionPoint(Point copy) then everything is correct. If PrecisionPoint(double x, double y) is used then copied x and y would be N and M instead of N + 1, M + 1. 2. Would like to have methods supporting PrecisionPoint(s): shrink(double, double) expand (double, double) union(double, double) getPreciseCenter() setLocation(Point) transpose() contains(Point) Patch will be uploaded.
Created attachment 84900 [details] Patch for PrecisionPoint and PrecisionRecatngle Hi Randy and Pratik. Could you please take a look at the patch and tell if it's ok to commit it? Thanks in advance.
Created attachment 85011 [details] patch Previous patch was not uploaded correctly. Here is the correctly uploaded patch. Sorry for any inconvenience.
Created attachment 85133 [details] Corrected patch Corrected the patch: 1. Added the correct @since # 2. In PrecisionRectangle#transpose() after double precise values are transposed it's better to call super to transpose the int values intsead of updateInts().
We have one non-standard signature on PrecisionRectangle already: union(PrecisionRectangle). But maybe we could avoid further confusion if we make some changes to Point, Dimension, and/or Rectangle first. What if we add: float preciseX() float preciseY() to the standard Point class? It could be package visibility, but I don't see any problem in their being public too. Then this new union(point) method would not behave differently based on which method gets called (i.e. union(Point) vs. union(PrecisePoint)), or we could avoid instance of checks, as with: public Rectangle setLocation(Point loc) { + if (loc instanceof PrecisionPoint) { + PrecisionPoint preciseLoc = (PrecisionPoint) loc; + this.preciseX = preciseLoc.preciseX; + this.preciseY = preciseLoc.preciseY; + updateInts(); + return this; + } else { + return super.setLocation(loc); + } +} But with new methods on Point, etc.: public Rectangle setLocation(Point loc) { + this.preciseX = loc.preciseX(); + this.preciseY = loc.preciseY(); + updateInts(); + return this; +} If the client could call union(Point) instead of union(double, double), that could minimize the number of new signatures.
Created attachment 85304 [details] Added methods to Point, Dimension and Rectangle to make cleaner API on precise geomteric objects I've added double preciseX() type of methods on Point, Dimension and Rectangle and reworked PrecisionPoint, PrecisionDimension and PrecisionRectangle accordingly. Also I removed "final" for Rectangle#union(Rectangle) and Rectangle#union(Point) methods such that they can be overriden on PrecisionRectangle. PrecisionRectangle#union(PrecisionRectangle) is deprecated therefore. Please let me know if this looks as you'd expect it. Thnanks in advance.
is getPreciseCenter still needed, or could PrecisionRectangle just override getCenter() now that x & y are polymorphic, or whatever it's called.
Created attachment 85466 [details] Corrected patch Indeed, it's not needed at all - Point#getCenter() must be overriden in this case. The corrected patch is uploaded.
So, if everything else looks ok, perhaps, Anthony could commit that last corrected patch?
Hi Anthony, Could you commit the last corrected patch please? Thanks in advance.
I am getting a single compile error in public PrecisionPoint getCenter() { return new PrecisionPoint(preciseX + preciseWidth / 2.0, preciseY + preciseHeight / 2.0); } PrecisionRectangle.java: line 300: The return type is incompatible with Rectangle.getCenter()
It looks like Pratik added *.prefs to the .cvsignore a while back, which can allow the code to be compiled with the wrong compatibility settings (1.5 instead of 1.4). It makes sense to me to force anyone creating patches to also check-out the proper project-specific compiler settings to avoid this in the future.
Created attachment 86005 [details] Corrected patch Corrected the compiler error
(In reply to comment #11) > It looks like Pratik added *.prefs to the .cvsignore a while back, which can > allow the code to be compiled with the wrong compatibility settings (1.5 > instead of 1.4). It makes sense to me to force anyone creating patches to also > check-out the proper project-specific compiler settings to avoid this in the > future. > Hmm, This is a problem since the line org.eclipse.jdt.core.compiler.compliance=1.4 is in the GEF plug-in but not Draw2d. I will fix when doing Bug 204605
Committed to HEAD.
