On Fri, 22 Sep 2023 18:21:56 GMT, Kevin Rushforth <[email protected]> wrote:
>>> yeah, I also don't quite understand why we have the rounding (I mean,
>>> floor'ing) here. I would vote to remove the typecast as the OP proposes.
>>
>> Phil has some concerns that will need to be looked into.
>>
>>> But there is more:
>>>
>>> 1. why in the world the constructor is not public? what if I need to print
>>> a Commercial 8-5/8 envelope?
>>
>> That's the wrong question to ask, and this PR is the wrong place to ask it.
>> If I could reformulate your question, I might rather state it as "would it
>> be a useful enhancement to allow Paper to be customizable?". Maybe (or maybe
>> not), but as with any other new feature, it needs proper motivation, etc.
>>
>>> 2. what is going on in hashCode??
>>>
>>> ```
>>> public final int hashCode() {
>>> return (int)width+((int)height<<16)+units.hashCode();
>>> }
>>> ```
>>
>> This is also unrelated to this PR. It may or may not be a great `hashCode`
>> implementation, but It meets the contract of hasCode w.r.t. equals. And if
>> it didn't, it would need a separate bug.
>
>> > yeah, I also don't quite understand why we have the rounding (I mean,
>> > floor'ing) here. I would vote to remove the typecast as the OP proposes.
>>
>> Phil has some concerns that will need to be looked into.
>
> To follow-on, it seems that there are a few options.
>
> 1. Pass back the unmodified value. The concern that Phil raised is that this
> could lead to floating-point error. Of course the current code that truncates
> could also lead to errors. If the value ends up as, say, 10.9999 inches, we
> would want to return 11 rather than 10 even if we did want to limit it to
> integers.
> 2. Round the value to whole integers. This doesn't really solve the problem
> that the PR is trying to solve
> 3. Round to some number of decimal places, e.g., round to the nearest 0.01 or
> 0.001 inches.
> 4. Something else.
>
> Option 3 seems like it might be a reasonable solution, but I'd want Phil to
> weigh in.
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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1244#discussion_r1334708237
