On Sat, 23 Sep 2023 05:39:12 GMT, John Hendrikx <[email protected]> wrote:
>> what kind of FP error could option 1 lead to?
>>
>> I think rounding belongs to the UI, to the panel which displays the sizes
>> with precision given by the requirements. So this method, I think, should
>> return the unmodified value.
>>
>> Looking at AWT Paper, I see no rounding there. Where did it come from to FX?
>>
>> PS. my other comments are indeed unrelated to this PR. sorry.
>
> Some observations on the paper class:
>
> /**
> * Specifies the North American legal size, 8.5 inches by 14 inches.
> */
> public static final Paper LEGAL = new Paper("Legal", 8.4, 14, INCH);
>
>
> The comment and actual size contradict each other. 8.4 inches would not
> result in an integer number of points (604.8) and looking up the legal paper
> size confirms it should be 8.5 inches
>
> /**
> * Specifies the Monarch envelope size, 3.87 inch by 7.5 inch.
> */
> public static final Paper MONARCH_ENVELOPE = new Paper("Monarch
> Envelope", 3.87, 7.5, INCH);
>
> This seems incorrect, the size is 3-7/8” x 7-1/2”, which would be `3.875` x
> `7.5` -- the difference in this case is small enough that it will be rounded
> to the same value in points.
About the rounding; all these values are fixed values and could have been
entered in points as constants directly.
Also realize that all papers measured in inches can be expressed exactly in
points (after the above errors are fixed) so no rounding is needed for those at
all; the same is not true for the papers specified in millimeters.
Since the papers in inches never should need any rounding, the rounding can
therefore be removed for those. If there is a worry that the multiplication (*
72) may cause floating point errors, then I suggest to specify all papers in
points immediately, like this:
new Paper("Letter", 8.5 * 72, 11 * 72, POINT);
As for the papers specified in millimeters, these can't be expressed in whole
points, as a millimeter is equivalent to `2,83465` points. Rounding them to
points will require some guess work to find out what the actual size in
millimeters was, for example, when it says 3 points it must have been 1 mm,
while 1, 2, 4 or 5 points has no direct mm equivalent.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1244#discussion_r1334928344
