GNOME Bugzilla – Bug 631091
st-theme-node: Support non-uniform border-radii
Last modified: 2010-10-04 22:38:47 UTC
See patch.
Created attachment 171489 [details] [review] st-theme-node: Support non-uniform border-radii Non-uniform border-radii are already supported when using a gradient background, this patch adds support for solid colors as well. The currently applied technique of using corner textures and filling the remaining area with rectangles is extended, so that each corner is padded with rectangles to the size of the largest corner.
Created attachment 171495 [details] [review] st-theme-node: Support non-uniform border-radii Looks like we currently leak the corner texture, so add unref.
Review of attachment 171495 [details] [review]: Except for one comment, looks good to me. (Long term, I'm not sure this is tenable - I think we'll have to break down and spend the time to do something like https://bugzilla.gnome.org/show_bug.cgi?id=607500#c32 at some point.) I would like to see some tests of this in tests/interactive - it appears however, that tests/interactive is broken at the moment because of the use of global.set_property_mutable. I think it would be OK to make environment.js:_blockMethod() do: if (global === undefined) // test environment return; and similar for the other reference to global in there. (separate patch) ::: src/st/st-theme-node-drawing.c @@ +803,3 @@ + /* We have painted all corners as if they had a radius of max_border_radius; + * now we add padding to those corners with a smaller radius, which allows + * us to treat corners as uniform further on. 'We have painted all corners as if they had a radius of max_border_radius;' - confused by that. It seems that you painted the corners with their natural size ofmax_width_radius[<corner>]
Created attachment 171513 [details] [review] st-theme-node: Support non-uniform border-radii Updated comment - I'll have a look at writing a test later on.
Created attachment 171622 [details] [review] tests: Fix environment Environment.init() uses Shell.Global, which is not accessible outside the mutter process; allowing to run the function when window.global is undefined fixes the environment for tests.
Created attachment 171623 [details] [review] st-theme-node: Support non-uniform border-radii Added test case.
Review of attachment 171622 [details] [review]: ::: js/ui/environment.js @@ +85,3 @@ + if (global === undefined) // test environment + return; Early return here is error-prone for future additions. Obsoleted by my other comment, but otherwise I'd suggest moving all the blocking stuff into a function _blockMethods() ::: tests/testcommon/ui.js @@ +10,2 @@ function init() { + window.global = undefined; Don't like this - it's pretending that referencing an undefined global works like I thought worked, but it doesn't. I think my new preference for an approach is in the UI setup code: window.global = { set_property_mutable : function() {} };
Review of attachment 171623 [details] [review]: ::: tests/interactive/border-radius.js @@ +25,3 @@ + + 'background: white; ' + + 'border-radius: 0px 5px 10px 15px;' + + 'padding: 5px;' }), { x_fill: false }); Doens't particularly matter here, but if these are actually all identical here, would be a lot easier to read if it was: add_test_case("0px 5px 10px 15px"); Or something like that
Created attachment 171725 [details] [review] tests: Fix environment Updated according to suggestion in IRC.
Review of attachment 171725 [details] [review]: Fine
Created attachment 171726 [details] [review] st-theme-node: Support non-uniform border-radii (In reply to comment #8) > would be a lot easier to read if it was: > > add_test_case("0px 5px 10px 15px"); > > Or something like that Done.
Comment on attachment 171725 [details] [review] tests: Fix environment Attachment 171725 [details] pushed as b33ebb4 - tests: Fix environment
Review of attachment 171726 [details] [review]: Looks good.
Attachment 171726 [details] pushed as 3d2d396 - st-theme-node: Support non-uniform border-radii