Skip to content

Conversation

@countingpine
Copy link
Collaborator

  • coordinates should be multiplied by e.g. 'width - 1', not just 'width'.
  • CINT() macro was not rounding positive values properly

Note: CINT() is also used in https://github.com/freebasic/fbc/blob/master/src/gfxlib2/gfx_circle.c#L96 and https://github.com/freebasic/fbc/blob/master/src/gfxlib2/gfx_draw.c#L247, so we need to make sure that they were not relying on the buggy rounding.

- coordinates should be multiplied by e.g. 'width - 1', not just 'width'.
- CINT() macro was not rounding positive values properly
@countingpine
Copy link
Collaborator Author

I think this change yields a positive effect in the rounding of DRAW, shown in this code:

screen 13
draw "bm160, 90 TA5" & string(50, "U")
draw "bm160,110 TA5" & string(50, "D")
draw "bm150,100 TA5" & string(50, "L")
draw "bm170,100 TA5" & string(50, "R")
sleep

Where previously there was a small "hook" at the start of the up/right spokes, this now no longer happens. (This is because it now starts in the middle of the "step" instead of the start/end.)
Before:
unpatched
After:
patched

@countingpine
Copy link
Collaborator Author

Similar improvements can be seen when drawing arcs with Circle. Arcs are quite ugly anyway (no Bresenham), so I don't think the implementer would have cared much about the rounding method.
arc-unpatched
arc-patched
Note: in this case the patch only changes the rounding on positive x,y (the right/top sections of the circle).

I conclude that the patch is sound, and fixes less-than-ideal behaviour in other use cases, where the bug has not been accounted for.

@countingpine countingpine merged commit e8c7305 into freebasic:master Nov 22, 2018
@countingpine countingpine deleted the issue_113 branch November 22, 2018 16:03
@dkl
Copy link
Member

dkl commented Nov 22, 2018

Even though I don't know the Draw code, the patch seemed logical. Especially the change to CINT(), which changes the casting (if I understood that correctly...). It makes sense to cast to int after the + 0.5, instead of before, for rounding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants