From kde-core-devel Wed Nov 29 16:52:51 2006 From: Matthew Woehlke Date: Wed, 29 Nov 2006 16:52:51 +0000 To: kde-core-devel Subject: Re: RFC: Making focus information visible for panes Message-Id: X-MARC-Message: https://marc.info/?l=kde-core-devel&m=116487127223187 David Faure wrote: > On Wed Nov 29 2006, Maksim Orlovich wrote: >>> Styles use the palette given to them... So I disagree. Color is the >>> palette's job, not the style's. >> Qt disagrees. QStyle::polish(QPalette). > > Ok the style has a hook into the palette. So? I think a widget adjusting a palette is still OK, isn't it? I would assert that it's asking for trouble. If you read the documentation for QStyle::polish[1], there are a number of things you can do in polish() that you would break by artificially changing the palette at another point: 1a. polish() is a good time for a style to cache palette information internally, if it needs/wants to (for instance, to cache colors that are derived from the palette). 1b. For a style like Keramik* that uses pixmaps for shading effects, polish() is a good place to pre-create them. This would mean that altering the palette later would work incorrectly, if it worked at all. 2. A style is permitted to alter the palette in polish(). If you then alter the palette at some other point, you have BROKEN the style. (* I haven't actually looked at Keramik, so it might not do this, but it's an example of a style where a developer *might* choose this technique to achieve an effect like Keramik.) Now, you *could* solve all this by re-calling polish every time you alter the palette, but that is a MAJOR performance hit, because you force a style to clear and refresh any cached information is has /for every focus change/. I can absolutely guarantee that a LOT of styles (mine included) would suffer from you doing this. All that said, there is still another major problem: you don't have one QStyle instance per widget, you have one per *container* (and I did /verify/ that), and, except in cases (e.g. kuiviewer) where you intentionally create more than one container, I think that generally translates into one instance /per application/. IOW, changing the palette changes it for ALL widgets, so you would have to be REALLY, really careful to only draw the control with keyboard focus with the altered palette. (WCTS) I'm not sure that's even possible, in which case this entire discussion is moot, because doing this /outside/ the style just plain won't work. Conversely, doing The Right Thing *in* a QStyle/KStyle is relatively simple; check if the widget you are drawing has keyboard focus, and alter your drawing accordingly. Plus this lets the style decide what that means. (Maybe instead of using the highlight color, it wants to use some other distinctive palette color, or perhaps a color the user chooses. Maybe the user just wants the plain-ol' focus rect.) If I'm wrong I trust someone will correct me, but it still looks like letting the style handler it is cleaner, safer, more efficient and more flexible. ALL that said (just to be clear) I still think *doing* this is a good idea. Let's just do it in the right place. :-) Now, one thing that MAY need to change in libs (outside of styles) is making it easy for a style to know if the widget it's being asked to draw should be drawn with the focus hint. :-) [1 http://doc.trolltech.com/3.3/qstyle.html#polish] -- Matthew Emacs is a nice OS - but it lacks a good text editor. That's why I am using Vim. -- Anonymous