Hide Forgot
Created attachment 494741 [details] multiple bugfixes and enhancements While working on something completely different I bumped into several issues with the libucil drawing primitives: 1) Although ucil_draw_rect clips to the viewport, neither ucil_draw_box nor ucil_draw_circle do, which causes memory corruption and crashes when they partially or completely fall outside the buffer. 2) The internal HLINE implementation looks like a partial attempt to optimize for 8pp drawing on 32-bit word machines, but is called for 16/32bpp drawing and even for 8pp gets the bounds checking wrong and can draw outside the line endpoints 3) There is a separate partial attempt at a HLINE for 24bpp, but this isn't used. Instead the box drawers simply loop, but ucil_draw_box gets the bounds wrong and so never draws the bottom-right pixel of the box. 4) Although ucil_draw_line uses Bresenham, ucil_draw_circle uses trig which is slower and grottier This patch: 1) fixes draw_hline to handle 8/16/24/32bpp fully 2) refactors the colorspace support in draw_rect and draw_box so that 24bpp can be handled the same way as 8/16/32bpp 3) changes draw_circle to use a DDA 4) since (3) makes filled circles easy, I've added a new function ucil_draw_filled_circle 5) _box, _circle and _filled_circle now normalize and clip so they can draw shapes that lie only partially within the viewport.
Created attachment 494753 [details] multiple bugfixes and enhancements Oops. *this* is the correct patch
Kamil, are you able to review this patch (and place it upstream if it's ok)? All I could do is just copying this patch to the upstream tracker without any review, because I'm not a C coder.
John, thanks for your contribution. There is not much I can do with this patch since I do not know the libucil API, neither the Bresenham algorithm. I cannot see any obvious mistakes at first glance, only two minor nits: - your patch duplicates and misplaces the doxygen entry for ucil_create_font_object() - you should probably use the upstream coding style, no matter how uncommon it is: if( ... ) ^ ^ for( ... ) ^ ^ Should I forward you patch upstream, or will you do it yourself?
Created attachment 497930 [details] multiple bugfixes and enhancements remove duplicated comment, conform better to original style
Points taken on board - hopefully the new version addresses them. I could submit it myself, but I figure that if distro package maintainers have an existing relationship with upstream, then at least for smaller projects submitting through the distro is likely to go more smoothly. (I am of course available to answer any questions or fix any issues upstream may have with the patch.) FYI: Bresenham is *the* standard fast-but-crude line drawing algorithm and is what libucil already used. Wikipedia has a reasonable description: http://en.wikipedia.org/wiki/Bresenham%27s_line_algorithm There is a fairly straightforward extrapolation of it for drawing circles, which I used to replace the trigonometric version in libucil: http://en.wikipedia.org/wiki/Midpoint_circle_algorithm There are all sorts of additional extensions possible here: versions that can handle thick lines, arbitrary filled polygons, ellipses etc. All fairly well known back in the 8-bit days ;-)
(In reply to comment #5) > Points taken on board - hopefully the new version addresses them. Thank you for polishing the patch. I have forwarded your patch upstream: https://bugs.launchpad.net/unicap/+bug/780943
committed upstream: http://bazaar.launchpad.net/~arne-datafloater/unicap/trunk/revision/129
This message is a notice that Fedora 14 is now at end of life. Fedora has stopped maintaining and issuing updates for Fedora 14. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At this time, all open bugs with a Fedora 'version' of '14' have been closed as WONTFIX. (Please note: Our normal process is to give advanced warning of this occurring, but we forgot to do that. A thousand apologies.) Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, feel free to reopen this bug and simply change the 'version' to a later Fedora version. Bug Reporter: Thank you for reporting this issue and we are sorry that we were unable to fix it before Fedora 14 reached end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged to click on "Clone This Bug" (top right of this page) and open it against that version of Fedora. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping