GNOME Bugzilla – Bug 569654
Metacity enforces silly restrictions or border-radius
Last modified: 2009-02-04 08:26:15 UTC
Please describe the problem: Imagine, if you will, a theme whose left border is N px wide, and whose bottom border is M px wide. If you want the bottom-left corner to be a circular curve, and not run the risk of the curve intersecting the area of the client window, then the corner radius must be somewhere between 0 and min(N,M) pixels. However, Metacity enforces rather different and far less logical restrictions on the border-radius setting. In theme.c, in meta_frame_layout_calc_geometry around line 935 in revision 4092, we see the following code: if (fgeom->bottom_height + fgeom->left_width >= min_size_for_rounding) fgeom->bottom_left_corner_rounded_radius = layout->bottom_left_corner_rounded_radius; "min_size_for_rounding" is set to 0 for shaded windows and 5 otherwise. This leads to the following illogical behaviours: - If N+M < 5 on an ordinary window, no border-radius is applied even if it would make sense. For example, if both borders are 1px, it would make sense to have a 1px-radius border to just clip off the corner pixel, but Metacity disallows that. - If N+M >= 5 on an ordinary window, any border-radius is allowed, even if it would hide part of the client window. I have personally tested this as high as 2000px, at which point entire windows had become invisible. For a theme format designed to limit the havoc a theme can wreak, this seems an oversight. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 127447 [details] Test theme for this bug report Attached is a test-case theme for this bug. After setting Metacity to use this theme, the bottom-left corner of every window will have a 50px radius, even though that often obscures the window contents. Meanwhile, the bottom-right corner has no border-radius applied, even though the theme asks for a 1px-radius corner and the bottom and right borders are sufficient for such a radius.
Created attachment 127449 [details] Screenshot demonstrating the fun one can have with ludicrous border-radii Here's a screenshot from the development of another theme I was working on when I found this bug. I think this has a 500px radius or so.
Thinking about this a little harder, "0 <= radius <= min(N,M)" is stricter than absolutely necessary: all we care about is that the border-radius not intersect with the corner of the client window, or in otherwords that the radius be smaller than the diagonal distance of the border-width. I think that corresponds to: 0 <= radius^2 <= min(N,M)^2
The corner case is when the rounded courner touches the window contents (assuming n and m are border widths): radius_max = sqrt((radius - n)^2 + (radius - m)^2) Therefore: radius < sqrt((radius - n)^2 + (radius - m)^2) radius^2 < (radius - n)^2 + (radius - m)^2 Therefore: if radius < n: all is good if radius < m: all is good if (radius - n)^2 + (radius - m)^2 > radius^2: all is good else: it will cut window contents I don't think metacity should try to solve the polynomial in case it decides the radius is too big.
Actually I think I switched < and > in all 3 cases :)
You *don't* think Metacity should try to solve the polynomial? 0 <= radius^2 <= min(N,M)^2 sounds like a reasonable limit to me (after having drawn a back-of-the-envelope diagram. Patryk, is there a problem with that?
Created attachment 127872 [details] Back-of-the-envelope doodle
0 <= radius^2 <= min(n,m)^2 is basically same as: 0 <= abs(radius) <= abs(min(n,m)) which in metacity can be safely reduced to: 0 <= radius <= min(n,m) Which won't allow your doodle diagram situation where radius is clearly more than both n and m. Hint: the length of the line connecting rounding circle's centre with the window's corner is of sqrt((radius - n)^2 + (radius - m)^2) length. Again these are simple tests that don't involve neither division nor square roots: if radius < n: all is good if radius < m: all is good if (radius - n)^2 + (radius - m)^2 <= radius^2: all is good else: discard the rounding (you can do the math at theme loading time to output a warning) You only get to solve the polynomial if you want to bound the radius instead of discarding it (which I think is a bad idea as people would just put 99999 in there "because it works anyway").
Created attachment 127906 [details] [review] Something like this, perhaps?
If it was me, I'd leave a little comment above the #define saying "This calculation was chosen in bug 569654", but that looks pretty good to me.
Thomas: The patch does work as far as the math goes. It also needs a second check in the preview drawing code. It also uncovers some nasty bug in the drawing code. According to both math and Inkscape (had to double check) a corner radius od 33 with all border width equal 10 should not cut away any portion of the window. In metacity it does, see attached screenshot.
Created attachment 127910 [details] Screenshot after applying the patch - shows a bug in corner drawing code