Thomas’s bug queue for the next few days:
Also: there is apparently a patch in accepted-commit-now limbo.
So, now that the compositor is in trunk and everyone is excited, this might be a good time to mention some “issues”.
Firstly, it seems that there are some weird shadow redrawing problems…these just appeared recently, so it shouldn’t be hard to find the offending commit. I think I know what it is, I just need to work out why its not working.
Next: Now that some windows have argb visuals some buggy themes may have translucent parts where there weren’t translucent parts before. Gilouche was one of these themes and Jimmac fixed it up recently. So if you start seeing through the window title bars, tell your theme author.
Also, I’ve been getting reports of terrible performance and rendering issues with xgl. It seems xgl doesn’t like any of the non-GL compositors, so I don’t know what to do about that.
Finally, on a happier note – a big thank you to whoever bought me stuff from my wishlist. Its much appreciated, even if it did take me two days before I noticed my parents had piled up my mail in a weird place while I was away…
Some recent changes to the compositor made it feel kinda slow and laggy. Not in a noticable way but more in a subconscious something feels wrong way. I tested xfwm and it seemed fine running as a compositor, and given that Metacity’s compositor has the same heritage as xfwm’s and I’ve been checking the xfwm code to see how they do things, I thought that was suggesting that there was something wrong with the Metacity one. I couldn’t see anything abnormal or strange looking at/comparing the codebase, so I turned to valgrind and oprofile. Running callgrind didn’t really show anything about the compositor code though, but as always with callgrind stuff gets lost in amongst the small functions like g_type_check_instance_is_a and the wonderfully mysterious <cycle 8>
My expectations were that some of the compositor functions would be topping the profile, given that the redrawing functions need to run anytime something changes on the desktop (modulo that they’re called from an idle handler so some redraw events are merged together) but what Oprofile said was
samples % symbol name
13908 21.8501 pos_eval_helper
10471 16.4504 pos_tokenize
7621 11.9729 meta_color_spec_render
5408 8.4962 compositor_idle_cb
3785 5.9464 fix_region
1559 2.4493 meta_draw_op_draw_with_env
1511 2.3738 free_tokens
1480 2.3251 meta_parse_position_expression
Eh? What are pos_eval_helper, pos_tokenize and meta_color_spec_render and why are they so high up the profile? Well, it turns out they’re part of the theme parsing code (Metacity has a very powerful, but complex theme format dontcha know?), but some more debugging showed that the theme expressions were being reparsed and re-evaluated every time Metacity had to draw a window frame. The impressive screenshot for this is:
Just look how many times pos_tokenize and pos_eval_helper are called when a draw op is drawn. That is one complicated function call chart.
Looking through the code I saw that draw operations were stored as their string format, like “2 * width + Bmin / height % 7”. These expressions were first tokenised (in pos_tokenize) into constituent parts (‘2’ ‘*’ ‘width’ etc) and they were then evaluated. Thing is that these strings cannot be changed at all so they can be tokenised when loading the theme rather than every time they are evaluated.
meta_color_spec_render is a similar problem. Colours can be defined as a basic colour, a GTK theme colour, a shade of a colour, or from two colours blended together. With shaded and blended colours they were generated every time Metacity had to use that colour, which is an obvious waste of time as the colour specs cannot be changed after they are created and so the colours should just be generated whenever the spec is created and then it becomes a simple function to get the correct colours.
All this is filed as bug #500279 and the patch attached to the bug fixes these two issues.
pos_eval_helper is the function that calculates the theme expressions from their tokenised state and it still gets called every time the theme needs redrawn, which sort of makes sense, as the variables may have changed since the last time, but it looks to me like the variables are all related to the width/height of the window, so it may be that we can make it so that the expressions are only re-evaluated when the width/height changes. I’ll have to look into it.
Anyway, with the patch from bug #500279 applied this is what the profile looks like now…
9355 32.4815 compositor_idle_cb
7129 24.7526 pos_eval_helper
1070 3.7151 meta_parse_size_expression
1020 3.5415 event_callback
604 2.0971 meta_compositor_process_event
597 2.0728 .plt
542 1.8819 win_extents
531 1.8437 pos_eval
519 1.8020 meta_frame_style_draw
495 1.7187 meta_draw_op_draw_with_env
The compositor functions are up where they should be…getting rid of that pos_eval_helper would be nice though, as evinced by this screenshot of the same function as above, but with the patch applied. The function is a lot less busy, which is a good thing, but the three sets of calls to pos_eval_helper could possibly be reduced somehow.
Anyway, the compositor seems faster, and I didn’t even touch that code this time :)
Sorry for the lack of daily “journal” posts over the past few days: I’ve been rather busy with real life, and it takes rather a while to gather up the crumbs to make a post. I’ve been writing a script off and on which will do the donkey work and let me or anyone else simply write the human part, but it’s not quite finished yet.
To be going on with, here’s the fascinating saga of negative numbers in theme files (well, more of a bug, really: Njáll Þorgeirsson does not have a starring role.) Someone complained that they couldn’t put negative numbers into theme files; Thomas closed the bug because the source comments (but not the documentation) say explicitly that negatives aren’t allowed. Besides, what good would they be? The reporter said that there was actually no reason to need negative literals because expressions which yield negative numbers can always be written instead. Certainly this does need documenting explicitly.
Havoc then pointed out that it makes little sense to prohibit “-1” but allow “0-1”. A long discussion of the semantics of the original comment followed. Nobody could remember ever writing it or quite what its subtleties were. Did it mean that unary negation wasn’t implemented because negatives weren’t allowed for some unstated reason, or did it mean that negative literals weren’t allowed because unary negation would over-complicate the parser? Perhaps it does make sense to prohibit “-1” if it means the code will be more maintainable? Maybe code could be shared with some existing expression parser? The issue has not been resolved.
In other news: congratulations to contributor Alex Turner (plexq on irc, aturner in svn) who got svn access yesterday after two very useful patches.
Thomas is aware that patch review is a little behind again (see above under “rather busy with real life”) and will try to get it back on track by the end of the week.
There is a GConf key called /apps/metacity/general/raise_on_click. It does not, as is widely assumed, cause windows to be raised when they’re clicked. You should never turn it off. Doing so will make your user experience confusing in subtle ways.
The short description of the key in the current schema says that it controls whether raising should be a side-effect of other user interactions. In other words, this key would have been better named “raise on click and also on a shedload of other random things like resizing”. The longer explanation runs:
Setting this option to false can lead to buggy behavior, so users are strongly discouraged from changing it from the default of true.
Many actions (e.g. clicking in the client area, moving or resizing the window) normally raise the window as a side-effect. Set this option to false to decouple raising from other user actions. Even when this option is false, windows can still be raised by an alt-left-click anywhere on the window, a normal click on the window decorations, or by special messages from pagers, such as activation requests from tasklist applets.
This option is currently disabled in click-to-focus mode.
Note that the list of ways to raise windows when raise_on_click is false does not include programmatic requests from applications to raise windows; such requests will be ignored regardless of the reason for the request. If you are an application developer and have a user complaining that your application does not work with this setting disabled, tell them it is their fault for breaking their window manager and that they need to change this option back to true or live with the bug they requested. See also bug 445447, comment 6.
(See also GNOME bug 326156.)
We still regularly receive bugs complaining that turning off raise_on_click breaks things. This is expected behaviour:
We have to keep the key for backwards compatibility, but please, everyone, just leave this key the heck alone and turned on. Thanks. :)
Metacity 2.20.1 is out, with some regression fixes and various other improvements.
Thanks to Alex Turner, Jens Granseuer, Owen Taylor, Federico Mena Quintero, and Elijah Newren for improvements in this release.
Translations
Djihed Afifi (ar), Metin Amiroff (az), Alexander Shopov (bg), Jordi Mallach (ca), David Lodge (en_GB), Jorge González (es), Iñaki Larrañaga Murgoitio (eu), Alastair McKinstry (ga), Ankit Patel (gu), Rajesh Ranjan (hi), “Last-Translator: auto\n” (hr), Changwoo Ryu (ko), Raivis Dejus (lv), Wouter Bolsterlee (nl), Gora Mohanty (or), ASB (pa), wadim dziedzic (pl), Og Maciel (pt_BR), Duarte Loreto (pt), Peter Tuhársky (sk), Maxim Dziumanenko (uk), Funda Wang (zh_CN)
Some helpful Ubunteros came and fixed Thomas’s laptop, which meant that Benjamin’s patch got reviewed (good work there), and also that the question was raised about what api.[ch] are good for.
Most of the checkin activity today was from Iain Holmes with his new compositor rewrite (or, as he has called the branch, the Bling-Tastic Bucket o’Bling). I am hugely excited about this; I hope Iain will write about his experiences here later.
On Launchpad, a discussion has been going on about whether it should be possible to set the width of window borders in general rather than its being down to the theme. Having too-narrow window borders makes resizing windows difficult, but some people don’t like the way it looks. This briefly spilled over into GNOME Bugzilla. It would seem to be a good plan to ask people to make default themes have large borders.
Alex Turner identified a place where the code could be made clearer and more easily optimised by the compiler, which seems promising.
A few months back, Matt Harley wrote a post about why he’s running Metacity rather than Compiz, which should be interesting reading for the future if we want to know why people choose Metacity rather than any other way of managing their desktop. Gentle reader, if you do, why do you use Metacity?
Photo: The Vintry, St Albans. Photo by Gary Houston, public domain.
