GNOME Bugzilla – Bug 567249
gsd-media-keys-window.c: cairo drawings are misaligned
Last modified: 2009-10-27 20:35:52 UTC
The cairo drawings are misaligned when the screen is composited. See the borders of the window, see the volume bar: the edges are not pixel-aligned. First of all I'm not sure you should use doubles, because double precision is not needed since we're drawing pixels (int). To fix, for example the window's borders, try to draw x+0.5, y+.5, width-1, height-1, (height-1)/10. You'll see how the borders will be aligned
I don't actually see anything... because I cannot use compositing. Now, since guessing what you mean probably isn't the best way to go about this, and since yu already seem to have an idea of how to fix it, how about a screenshot and a patch instead? Thanks.
Created attachment 128783 [details] [review] This patch fixes the problem. Cimi is right, following his suggestion I wrote the patch. Now curved_rectangle is fine.
Thanks for the patch. Since, as I mentioned before, I cannot actually test the patch, I'm going to take your word for it. 2009-02-15 Jens Granseuer <...> Patch by: Leo Iannacone <...> * plugins/media-keys/gsd-media-keys-window.c: (on_expose_event): fix alignment of the composited media window (bug #567249)
I guess the volume bar is not fixed yet... Leo, could you fix it too? The approach is similar: you should align the drawings to the cairo grid.
Andrea, I can't see the misalignment I'm afraid. Can you take a screenshot and show us where the bug is? To take a screenie, in gnome-settings-daemon/plugin/media-keys: - in gsd-media-keys-window.c, remove the code that sets fade_timeout_id: +// window->priv->fade_timeout_id = g_timeout_add (FADE_TIMEOUT, +// (GSourceFunc) fade_timeout, +// window); - in test-media-window.c, remove the g_timeout_add() code - and change the call to gsd_media_keys_window_set_volume_level() to set the level you know shows the problem (say, 50?) - launch test-media-window and take a screenshot
Created attachment 142813 [details] zoomed screenshot As you can see, the borders of the progressbar have 1px of "transparent" border, that's because it is not aligned to the cairo pixel grid. (0.5 px is drawn outside, and 0.5 on the border)
Created attachment 146271 [details] [review] Fix bluriness in level bar, and popup By shifting the top-left coordinates by half a pixel, to match the cairo grid. This makes the outter border of the popup, as well as the level bar's borders much sharper.
Created attachment 146272 [details] [review] Fix bluriness in level bar, and popup By shifting the top-left coordinates by half a pixel, to match the cairo grid. This makes the outter border of the popup, as well as the level bar's borders much sharper.
Waiting on Andrea, or Martin (on fedora-desktop-list) to confirm that the fix works for them.
(In reply to comment #8) > Created an attachment (id=146272) [details] [review] > Fix bluriness in level bar, and popup > > By shifting the top-left coordinates by half a pixel, to match > the cairo grid. > > This makes the outter border of the popup, as well as the > level bar's borders much sharper. On quick look: - x1 = x0 + width; - y1 = y0 + height; - if (!width || !height) { return; } + x1 = x0 + width; + y1 = y0 + height; + ^^^ this is irrelevant change The round () should work for the level bar, not sure if you're using the sizes for anything else (why are you rounding the icon_box_* as well?). - volume_box_x0, - volume_box_y0, + volume_box_x0 + 0.5, + volume_box_y0 + 0.5, ^^^ I'd prefer adjusting this in draw_volume_boxes() as you need to also adjust the value indication fill (that does not have border so it needs to be drawn on integer coordinates and with 0.5 smaller roundness). Don't forget to also width/height - 1 for the main rect and width/height - 2 for the value fill. - curved_rectangle (cr, 0.5, 0.5, width-1, height-1, height / 10); + curved_rectangle (cr, 0.5, 0.5, width-0.5, height-0.5, round (height / 10)); ^^^ this would break the previous patch. Width and heights are relative, not coordinates of the opposite corner, so you should be actually doing width/height - 2*0.5 (once from top/left and once from bottom/right). There's no need to round the radius -- non-integer radius does not make things blurry.
Created attachment 146305 [details] [review] Fix bluriness in level bar, and popup By shifting the top-left coordinates by half a pixel, to match the cairo grid. This makes the outter border of the popup, as well as the level bar's borders much sharper.
Had to make the width smaller as it would overflow onto the border otherwise. Looked at all that under a magnifying glass and it looks OK to me. Comments?
(In reply to comment #11) > Created an attachment (id=146305) [details] [review] > Fix bluriness in level bar, and popup > + height = round (height); ^^^ aren't you forgetting to round width as well? IMHO, both should be further decreased by one (the drawing function adds half of the border on each side making it effectively 1px larger) + x1 = round (width * percentage - 1.5); + x1 = (x1 >= 0 ? x1 : 0); ^^^ IMHO, should be just + x1 = round ((width - 1)*percentage); you should first correctly get available width (which is width - 1) and then derive the percentage. - curved_rectangle (cr, 0.5, 0.5, width-1, height-1, height / 10); + curved_rectangle (cr, 0.5, 0.5, width-1, height-1, round (height / 10)); ^^^ As I said in my previous comment, the rounding here isn't needed (it's corner radius setting which does not need to be integer)
Created attachment 146338 [details] [review] Fix bluriness in level bar, and popup By shifting the top-left coordinates by half a pixel, to match the cairo grid. This makes the outter border of the popup, as well as the level bar's borders much sharper.
(In reply to comment #13) > (In reply to comment #11) > > Created an attachment (id=146305) [details] [review] [details] [review] > > Fix bluriness in level bar, and popup > > > + height = round (height); > ^^^ aren't you forgetting to round width as well? No, the width is icon_box_width, which is already rounded. I'll round it to avoid this sort of confusion. > IMHO, both should be further > decreased by one (the drawing function adds half of the border on each side > making it effectively 1px larger) Done. > + x1 = round (width * percentage - 1.5); > + x1 = (x1 >= 0 ? x1 : 0); > ^^^ IMHO, should be just > + x1 = round ((width - 1)*percentage); > you should first correctly get available width (which is width - 1) and then > derive the percentage. Done. > - curved_rectangle (cr, 0.5, 0.5, width-1, height-1, height / 10); > + curved_rectangle (cr, 0.5, 0.5, width-1, height-1, round (height / > 10)); > ^^^ As I said in my previous comment, the rounding here isn't needed (it's > corner radius setting which does not need to be integer) Done.
Looks OK now, I don't have any further comments.
Attachment 146338 [details] pushed as fadc8f8 - Fix bluriness in level bar, and popup