Open vicuna opened 7 years ago
Comment author: @diml
I agree that case 1 should be the expected behavior; if you ask for a rectangle of width 10, you should get a rectangle of width 10, not 11.
I tried fix1.patch and it seems to fix the issue, however there are some problems with edge cases on Windows:
On Linux, [draw_rect 10 10 0 0] ends up passing negative values to XDrawRectangle. This doesn't cause a failure or bad behavior but should probably be avoided.
Comment author: iesvs
I forgot to test edge cases. So I just dit it with fix1.
On Windows when drawing a rect of size 0, the windows API invert the coordinates to always use nondecreasing coordinates. So a rect of size 2 is drawn. I didn't observe such behavior with fill rect.
With Linux I noticed no problems with fill_rect, but draw rect draw a very big rect when trying to draw a rect of size 0.
Maybe disable draw_rect and fill_rect when width or height is 0 will prevent strange behaviors.
Comment author: @diml
Yh I suppose we could have special cases when width is zero and/or height is zero. We could do this in the OCaml code, to avoid having to rely on the behavior of the underlying API for edge cases.
BTW, any chance you could submit a PR on github? This would make the review easier.
Original bug ID: 7498 Reporter: iesvs Assigned to: @diml Status: assigned (set by @diml on 2017-04-21T12:43:38Z) Resolution: open Priority: normal Severity: minor Version: 4.04.0 Category: standard library
Bug description
Graphics.draw_rect and Graphics.fill_rect have strange behaviors.
I will suppose that they use the same meaning for their arguments. I see two behaviors
1) how I understand the documentation fill_rect x y z h means { px, py : x <= px < x + w and y <= py < y + h }
2) what is currently implemented on Linux fill_rect x y z h means { px, py : x <= px <= x + w and y <= py <= y + h }
In any cases the behavior of windows must be the same. I use the C compiler of VS 2010 to compile ocaml.
For the case 1, Linux must be changed and Windows too I have the fix fix1.patch
For the case 2, Windows does not do the same thing as Linux I have the fix fix2.patch
I also add a test file (test.ml).
The documentation of get_image states that the arguments have the same meaning than fill_rect. That's why case 1 is probably the expected behavior.
By the way, there is the commit 9f20c9c8a10307f40406890352ba3fbb2e4bd735 which looks like the opposite of fix1 (for the Linux part). That's strange.
I don't have Mac OS X to test, sorry.
File attachments