After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 569654 - Metacity enforces silly restrictions or border-radius
Metacity enforces silly restrictions or border-radius
Status: RESOLVED OBSOLETE
Product: metacity
Classification: Other
Component: themes
2.24.x
Other All
: Normal minor
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2009-01-29 11:42 UTC by Screwtape
Modified: 2020-11-06 20:09 UTC
See Also:
GNOME target: ---
GNOME version: 2.23/2.24


Attachments
Test theme for this bug report (1.28 KB, application/x-gzip)
2009-01-29 11:49 UTC, Screwtape
  Details
Screenshot demonstrating the fun one can have with ludicrous border-radii (665.33 KB, image/png)
2009-01-29 11:52 UTC, Screwtape
  Details
Back-of-the-envelope doodle (5.67 KB, image/svg+xml)
2009-02-03 20:50 UTC, Thomas Thurman
  Details
Something like this, perhaps? (2.41 KB, patch)
2009-02-04 06:21 UTC, Thomas Thurman
none Details | Review
Screenshot after applying the patch - shows a bug in corner drawing code (73.18 KB, image/png)
2009-02-04 08:26 UTC, Patryk Zawadzki
  Details

Description Screwtape 2009-01-29 11:42:18 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:
Comment 1 Screwtape 2009-01-29 11:49:02 UTC
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.
Comment 2 Screwtape 2009-01-29 11:52:13 UTC
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.
Comment 3 Screwtape 2009-01-29 14:08:09 UTC
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
Comment 4 Patryk Zawadzki 2009-02-03 10:11:12 UTC
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.
Comment 5 Patryk Zawadzki 2009-02-03 10:15:33 UTC
Actually I think I switched < and > in all 3 cases :)
Comment 6 Thomas Thurman 2009-02-03 20:36:10 UTC
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?
Comment 7 Thomas Thurman 2009-02-03 20:50:17 UTC
Created attachment 127872 [details]
Back-of-the-envelope doodle
Comment 8 Patryk Zawadzki 2009-02-03 21:03:05 UTC
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").
Comment 9 Thomas Thurman 2009-02-04 06:21:42 UTC
Created attachment 127906 [details] [review]
Something like this, perhaps?
Comment 10 Screwtape 2009-02-04 08:15:46 UTC
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.
Comment 11 Patryk Zawadzki 2009-02-04 08:24:57 UTC
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.
Comment 12 Patryk Zawadzki 2009-02-04 08:26:15 UTC
Created attachment 127910 [details]
Screenshot after applying the patch - shows a bug in corner drawing code
Comment 13 André Klapper 2020-11-06 20:09:37 UTC
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years.

If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/metacity/-/issues/

Thank you for reporting this issue and we are sorry it could not be fixed.