GNOME Bugzilla – Bug 363608
The tooltip draw sytem of the ringschart widget does not work properly
Last modified: 2006-11-05 17:46:17 UTC
Please describe the problem: Sometimes the line from the selected sector of the ringschart to the tip showing the information is not well drawn. - The line should go from the center of the sector to the center of the tip. With some tips, the line reaches the edge of the window and then returns bck to the center of the tip, which is a strange behavior. - The line is always drawn, even if it is behind the tip, which is not optimal. Steps to reproduce: 1. Explore a folder with a lot of files and show it in the ringschart. The best choice is a folder that completelly fills the circle. 2. Move the pointer arround the ringschar and see how in some cases the line reaches the edge of the window and then returns back to the center of the tip. 3. Actual results: In some sectors the line reaches the edge of the window ant then goes to the enter of the tip instead of going directly to the center of the tip. Expected results: The line should go directly to the center of the tip. Does this happen every time? Only when the selected sector is in some angles. Other information:
Created attachment 75060 [details] [review] Patch to improve the tooltip draw system of the ringschart This patch replaces completely the tooltip draw method of the ringschart. Now, the lines drawn from the center of the selected sector to the center of the tip are always straight (no more cornered lines), avoiding the problem of reaching the edge of the window. BTW, the resulting code is simpler and faster than before. The patch also adds a check to avoid drawing the lines which are behind he tip and are not visible, improving the performance of the system.
Thanks Miguel. Please attach the diff created with "cvs diff -up > patch" otherwise it's almost impossible to review.
Created attachment 75065 [details] [review] Patch to improve the tooltip draw system of the ringschart Now the patch created with "cvs diff -up" from the gnome-utils directory. I hope this time is ok ;)
Created attachment 75075 [details] screenshot Tested, but something's wrong. When the mouse is over charts drawn on the lest side, the tooltip goes out of the window. Pls see attached screenshot
I can confirm the problems with the current draw tooltip, it would be nice to modify it and have a better proposal soon. Regarding this patch I have some comments: * I think that doing the tooltip size calculation before it is the better way, as you did. This calculation is a little bit more complex because you can paint the tooltip in the four sides of the widget window, although it simplifies the calculation of the line, I'm not sure if it is worthwhile after reviewing the operations. * The center tooltip is not being drawn, not sure why. * You should use tooltip_x, tooltip_with, etc. in the cairo_rentagle functions, add the paddings before, IMHO it is safer. I'll try to send a proposal drawing the tooltips in just two sides and see what it looks like.
Created attachment 75462 [details] [review] New proposal of the tip draw system patch Ok, I think I've found the problem with the last patch. I've fixed the problem (at least I think I did) and made some improvements to the code. I've removed the if to check the sector of the circle we were working in: * Using absolute values I don't have to care about negative results. * Knowing that the center of the circle is always in the center of the window, the distance to both left and right edges is the same (and the same for up and down edges), so those distances can easily be calculated as center - tip_size. I hope this solves the problem Fabio reported. Tell me your opinions ;) Cheers!
Works great for me now. Wait for Paolo's comment before committing.
I'd like Alejandro to review it, but for me it's ok. Note: I haven't had the chance to actually test the patch... Not really strictly related to this patch, but with regard to tooltip drawing, I was noticing that sometimes the tooltip border is blurry: I think this is due to how cairo coords work: if you draw a vertical line of width 1 pixel at coordinate 10, the real line will be drawin from 9.5 to 10.5 (think of drawing with a pencil on a squared paper along on of the lines); now since the line spans two pixel columns, half on each and since cairo uses antialiasing: the result is a blurred line. If you make sure to always draw lines of width 1 to a coordinate X.5, the line will span from X to X+1 and will fill exactly one pixel column without any blurring.
Created attachment 75605 [details] [review] Proposed modification to the patch It took me some time to understand the equations to calculate the hypotenuses :-), it is really nice, and it works really well. Anyway I found a couple of small things that maybe we can change improve it: - TOOLTIP_PADDING and WINDOW_PADDING were coupled, we were using the WINDOW_PADDING to calculate the width and height of the tooltip. This caused some little problems changing that values. - The position and real size of the tooltip was not calculated until we draw so the if to manage the draw of the rect was weird, although I think it worked because the tip was bigger than the final position - I think we could use ABS glib macro What do you think Miguel? I've uploaded a modification of the patch, please review it and change what you think it is wrong because I don't have the same experience than you have in the problem.
(In reply to comment #8) > I'd like Alejandro to review it, but for me it's ok. > > Note: I haven't had the chance to actually test the patch... > > > Not really strictly related to this patch, but with regard to tooltip drawing, > I was noticing that sometimes the tooltip border is blurry: I think this is due > to how cairo coords work: if you draw a vertical line of width 1 pixel at > coordinate 10, the real line will be drawin from 9.5 to 10.5 (think of drawing > with a pencil on a squared paper along on of the lines); now since the line > spans two pixel columns, half on each and since cairo uses antialiasing: the > result is a blurred line. If you make sure to always draw lines of width 1 to a > coordinate X.5, the line will span from X to X+1 and will fill exactly one > pixel column without any blurring. > This is very interesting, we can review this and try to fix this issue.
Created attachment 75877 [details] [review] New proposal of the patch including Alex's changes and Paolo's suggestions Alex, I tested the patch and I think all is ok. You were right with your three suggestions, so the result is fine. I made some modifications in order to avoid the blurry effect Paolo commented. Rectangle borders don't look blurry anymore. Added the changes to the Changelog (again, not sure about whose name should put. If you want to change it, feel free to do it). Cheers :)
IMO it is ok, I'll commit it as soon as Paolo and Fabio give their approval, just a couple of small format changes. There is a tab inside one indentation that I can change: +get_color (Color *color, + gdouble angle, + gint circle_number, + gboolean highlighted) And two spaces instead of four in this case, I would also change them: + if ((priv->draw_center) && (tooltip->depth == -1)) + draw_circle (cr, priv->center_x, priv->center_y, priv->min_radius, TRUE); I'll change also the name of the committer in the ChangeLog and add a comment about the author of the patch. Anything else?
Excuse me, last post (12) was about the bug #363613, not about this one. Forget it. I'm sorry for the inconvenience.
IMO the patch is ok, the final code looks good and the tests that I did worked well. The lines not look better without the blur. I'll upload it as soon as Fabio and Paolo send their approval.
(In reply to comment #14) > IMO the patch is ok, the final code looks good and the tests that I did worked > well. The lines not look better without the blur. I'll upload it as soon as ^--- now Just to clarify the sentence, the lines look better now :-).
yeah, get this committed.
alex committed this. Thanks!
thanks alex