Re: c++11 and workspace
On 08/17/2017 10:09 PM, Thiago Macieira wrote: On Thursday, 17 August 2017 08:52:21 PDT Martin Flöser wrote: Am 2017-08-17 16:48, schrieb Hugo Pereira Da Costa: Hi, Quick question on the status of c++11 features that I can include in breeze. Are std::function allowed ? In workspace you can/should use C++11, KWin/master even requires C++14. Confirm if you mean the core language or the standard library. As far as I am concerned, I was referring mostly to core language.
c++11 and workspace
Hi, Quick question on the status of c++11 features that I can include in breeze. Are std::function allowed ? Thanks in advance, Hugo
Re: Review Request 127866: Oxygen: Fix QCache usage
> On May 30, 2016, 11:34 a.m., Hugo Pereira Da Costa wrote: > > Ship It! > > Hugo Pereira Da Costa wrote: > err. Wait ... > There are rendering issues here once the patch is applied. > See http://wstaw.org/m/2016/05/30/plasma-desktopY12228.png > (left is "before", right is "after"). > So something seems wrong with the background gradient. > I'll investigate a bit ... > > Hugo Pereira Da Costa wrote: > Hi again, > So, thinking more about it, and actually answering the questions raised > in the review: > > - do we track public API for this part of Oxygen? Does anything in a > different library or application link to this? > No we don't this is supposed to be an "internal" (as in private) library, > used only by oxygen style and decoration. No ABI/API guarantee. > > - QColor: can we change the return type. I would say yes, (from reference > to value), and return to using QCache > - Tilesets: I would be inclined to changing the return type here too, > using values, and assuming the copy constructor does not cost much, based on > the implicit-shareness nature of pixmaps, and keep a built-in QCache here > (which should be more efficient than the (otherwise nice) FIFO. > > Now I understand that this is a lot of going back and forth, and a job I > should rather do myself, as the maintainer of the code. > So I would propose to "postpone" this RR for now, and for me to locally > - take the QPixmap change from this patch > - implement something similar for QColor and TileSet > - test. > Then if I manage to do that in a reasonable time (e.g. this week), drop > the review and commit my change instead (with proper credits where due). > Otherwise (because of me being too busy with other stuff), just commit this > review (once the problem mentionned above is fixed, though I could not > investigate yet). > > What do you think ? > > (also: I need to sanitize this run-time changing of the cache use and > max-cost) > > Michael Pyne wrote: > Hugo, > > That all sounds fair. And if it's all too much difficulty, then we might > be able to lean on the hinted-at-but-not-quite-documented QCache behavior > that ::insert() only fails if the item being inserted has a cost higher than > maxCost. In that case I could ignore the Coverity issues (since Coverity > can't statically prove that items are always < maxCost) and then drop the RR > (and perhaps ask Qt to document more stringently that guarantee for > QCache::insert). > > Still though, I think the QColor return type change would make sense even > independent of this RR. > > Hugo Pereira Da Costa wrote: > Hi again Michael, > > So, I have a working implementation here that > - keeps using QCache wherever possible, passing QColor and TileSet as > values rather than as refs (in a way that is similar to the change you did > for QPixmaps) > - uses your FIFOCache for the implementation of Oxygen::Cache, which is a > "cache of caches", and thus cannot possibly use values. > > Seems to work (though I would like to test a bit more), and should fix > all the issues with Coverty. > Since I have blatently copied your code for the home-made FiFOCache, i > have added you as a copyright holder for oxygenhelper.h > is that ok with you ? Should i put your name in other places too ? > > Best, > > Hugo > > Michael Pyne wrote: > > Since I have blatently copied your code for the home-made FiFOCache, i > have added you as a copyright holder for oxygenhelper.h > is that ok with you ? Should i put your name in other places too ? > > That's perfectly fine. You don't need to mention me elsewhere, as long as > this RR is mentioned in the commit log I think that would cover any needed > attribution of work. > > Albert Astals Cid wrote: > What's the status of this, was commited? Needs further work? Should be > discarded? yes.Code was committed,review should be discarded - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 --- On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 22, 2016, 4:
Re: Review Request 127866: Oxygen: Fix QCache usage
> On May 30, 2016, 11:34 a.m., Hugo Pereira Da Costa wrote: > > Ship It! > > Hugo Pereira Da Costa wrote: > err. Wait ... > There are rendering issues here once the patch is applied. > See http://wstaw.org/m/2016/05/30/plasma-desktopY12228.png > (left is "before", right is "after"). > So something seems wrong with the background gradient. > I'll investigate a bit ... > > Hugo Pereira Da Costa wrote: > Hi again, > So, thinking more about it, and actually answering the questions raised > in the review: > > - do we track public API for this part of Oxygen? Does anything in a > different library or application link to this? > No we don't this is supposed to be an "internal" (as in private) library, > used only by oxygen style and decoration. No ABI/API guarantee. > > - QColor: can we change the return type. I would say yes, (from reference > to value), and return to using QCache > - Tilesets: I would be inclined to changing the return type here too, > using values, and assuming the copy constructor does not cost much, based on > the implicit-shareness nature of pixmaps, and keep a built-in QCache here > (which should be more efficient than the (otherwise nice) FIFO. > > Now I understand that this is a lot of going back and forth, and a job I > should rather do myself, as the maintainer of the code. > So I would propose to "postpone" this RR for now, and for me to locally > - take the QPixmap change from this patch > - implement something similar for QColor and TileSet > - test. > Then if I manage to do that in a reasonable time (e.g. this week), drop > the review and commit my change instead (with proper credits where due). > Otherwise (because of me being too busy with other stuff), just commit this > review (once the problem mentionned above is fixed, though I could not > investigate yet). > > What do you think ? > > (also: I need to sanitize this run-time changing of the cache use and > max-cost) > > Michael Pyne wrote: > Hugo, > > That all sounds fair. And if it's all too much difficulty, then we might > be able to lean on the hinted-at-but-not-quite-documented QCache behavior > that ::insert() only fails if the item being inserted has a cost higher than > maxCost. In that case I could ignore the Coverity issues (since Coverity > can't statically prove that items are always < maxCost) and then drop the RR > (and perhaps ask Qt to document more stringently that guarantee for > QCache::insert). > > Still though, I think the QColor return type change would make sense even > independent of this RR. Hi again Michael, So, I have a working implementation here that - keeps using QCache wherever possible, passing QColor and TileSet as values rather than as refs (in a way that is similar to the change you did for QPixmaps) - uses your FIFOCache for the implementation of Oxygen::Cache, which is a "cache of caches", and thus cannot possibly use values. Seems to work (though I would like to test a bit more), and should fix all the issues with Coverty. Since I have blatently copied your code for the home-made FiFOCache, i have added you as a copyright holder for oxygenhelper.h is that ok with you ? Should i put your name in other places too ? Best, Hugo - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 --- On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 22, 2016, 4:20 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > --- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead
Re: Review Request 127866: Oxygen: Fix QCache usage
> On May 30, 2016, 11:34 a.m., Hugo Pereira Da Costa wrote: > > Ship It! > > Hugo Pereira Da Costa wrote: > err. Wait ... > There are rendering issues here once the patch is applied. > See http://wstaw.org/m/2016/05/30/plasma-desktopY12228.png > (left is "before", right is "after"). > So something seems wrong with the background gradient. > I'll investigate a bit ... Hi again, So, thinking more about it, and actually answering the questions raised in the review: - do we track public API for this part of Oxygen? Does anything in a different library or application link to this? No we don't this is supposed to be an "internal" (as in private) library, used only by oxygen style and decoration. No ABI/API guarantee. - QColor: can we change the return type. I would say yes, (from reference to value), and return to using QCache - Tilesets: I would be inclined to changing the return type here too, using values, and assuming the copy constructor does not cost much, based on the implicit-shareness nature of pixmaps, and keep a built-in QCache here (which should be more efficient than the (otherwise nice) FIFO. Now I understand that this is a lot of going back and forth, and a job I should rather do myself, as the maintainer of the code. So I would propose to "postpone" this RR for now, and for me to locally - take the QPixmap change from this patch - implement something similar for QColor and TileSet - test. Then if I manage to do that in a reasonable time (e.g. this week), drop the review and commit my change instead (with proper credits where due). Otherwise (because of me being too busy with other stuff), just commit this review (once the problem mentionned above is fixed, though I could not investigate yet). What do you think ? (also: I need to sanitize this run-time changing of the cache use and max-cost) - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 --- On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 22, 2016, 4:20 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > --- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead of pointers, so we simply make a > separate copy to be cached when we do insert. For QColor we return references > to values so we *must* return pointers, and those have to be owned by a > QCache to avoid memleaks. So I added a helper function to loop until the > cache accepts the new entry. TileSets are a similar concern, except those > have manual loops since I was uncertain about whether TileSet's copy > constructor was the best idea or not. > > This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, > 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug > 219055 (which doesn't actually appear to be a dupe of a different bug to > me...). > > > Diffs > - > > kdecoration/oxygendecohelper.cpp aa75eca > kstyle/oxygenstyle.cpp e428606 > kstyle/oxygenstylehelper.h 9510a60 > kstyle/oxygenstylehelper.cpp 612ba37 > liboxygen/oxygenhelper.h a6453a0 > liboxygen/oxygenhelper.cpp 4843604 > liboxygen/oxygenshadowcache.cpp 907e586 > > Diff: https://git.reviewboard.kde.org/r/127866/diff/ > > > Testing > --- > > Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. > Used the GUI Benchmark feature to automatically cycle through all the listed > features -- no crashes or obvious rendering errors. > > > Thanks, > > Michael Pyne > >
Re: Review Request 127866: Oxygen: Fix QCache usage
> On May 30, 2016, 11:34 a.m., Hugo Pereira Da Costa wrote: > > Ship It! err. Wait ... There are rendering issues here once the patch is applied. See http://wstaw.org/m/2016/05/30/plasma-desktopY12228.png (left is "before", right is "after"). So something seems wrong with the background gradient. I'll investigate a bit ... - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 --- On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 22, 2016, 4:20 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > --- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead of pointers, so we simply make a > separate copy to be cached when we do insert. For QColor we return references > to values so we *must* return pointers, and those have to be owned by a > QCache to avoid memleaks. So I added a helper function to loop until the > cache accepts the new entry. TileSets are a similar concern, except those > have manual loops since I was uncertain about whether TileSet's copy > constructor was the best idea or not. > > This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, > 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug > 219055 (which doesn't actually appear to be a dupe of a different bug to > me...). > > > Diffs > - > > kdecoration/oxygendecohelper.cpp aa75eca > kstyle/oxygenstyle.cpp e428606 > kstyle/oxygenstylehelper.h 9510a60 > kstyle/oxygenstylehelper.cpp 612ba37 > liboxygen/oxygenhelper.h a6453a0 > liboxygen/oxygenhelper.cpp 4843604 > liboxygen/oxygenshadowcache.cpp 907e586 > > Diff: https://git.reviewboard.kde.org/r/127866/diff/ > > > Testing > --- > > Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. > Used the GUI Benchmark feature to automatically cycle through all the listed > features -- no crashes or obvious rendering errors. > > > Thanks, > > Michael Pyne > >
Re: Review Request 127866: Oxygen: Fix QCache usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review96029 --- Ship it! Ship It! - Hugo Pereira Da Costa On May 22, 2016, 4:20 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 22, 2016, 4:20 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > --- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead of pointers, so we simply make a > separate copy to be cached when we do insert. For QColor we return references > to values so we *must* return pointers, and those have to be owned by a > QCache to avoid memleaks. So I added a helper function to loop until the > cache accepts the new entry. TileSets are a similar concern, except those > have manual loops since I was uncertain about whether TileSet's copy > constructor was the best idea or not. > > This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, > 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug > 219055 (which doesn't actually appear to be a dupe of a different bug to > me...). > > > Diffs > - > > kdecoration/oxygendecohelper.cpp aa75eca > kstyle/oxygenstyle.cpp e428606 > kstyle/oxygenstylehelper.h 9510a60 > kstyle/oxygenstylehelper.cpp 612ba37 > liboxygen/oxygenhelper.h a6453a0 > liboxygen/oxygenhelper.cpp 4843604 > liboxygen/oxygenshadowcache.cpp 907e586 > > Diff: https://git.reviewboard.kde.org/r/127866/diff/ > > > Testing > --- > > Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. > Used the GUI Benchmark feature to automatically cycle through all the listed > features -- no crashes or obvious rendering errors. > > > Thanks, > > Michael Pyne > >
Re: Review Request 127866: Oxygen: Fix QCache usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127866/#review95480 --- To be honest, I am quite puzzle by this whole thing. Now, every insertion in the cache requires at least two searches in there and (in many case) at least one copy constructor being called. This is quite expansive ... (even though this happens only if the object is not found in the cache). Also: not sure I understand what issue we are trying to fix and how: why is it that if the object inserted in the cache is immediately deleted, just retrying an indefinite amount of time will "fix" the issue. Are we not just transforming a crash into a freeze (infinite loop) ? The Qt documentation is very vague about cases where the object is deleted immediately, and the only case it mentions is: " In particular, if cost is greater than maxCost(), the object will be deleted immediately." Well, in such cases (that should not appear here), the infinite loop will not help. Right ? Since we have no idea on how "predictible" the other deletion cases are, I don't think the fix is a good fix. Does that mean that we should change the code in order to use references rather than pointer everywhere ? (as you did in the first patch on this topic) ? Or get rid of using QCache (because this absence of guarantee at the insertion stage is too much of a pain to handle) ? Or just commit and wait for bug reports about freezes ? (but with a happy coverty) ? - Hugo Pereira Da Costa On May 8, 2016, 5:03 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127866/ > --- > > (Updated May 8, 2016, 5:03 a.m.) > > > Review request for kde-workspace and Hugo Pereira Da Costa. > > > Repository: oxygen > > > Description > --- > > This should mostly complete the QCache fixes I kicked off in a previous RR, > 127837. Hugo noted there were many other similar usages, and boy he wasn't > kidding! ;) The long story short is that these usages can theoretically cause > use-after-free behavior (which can lead to crashes and even undefined > behavior if the compiler ever gets smart). > > *NOTE* It is -much- easier to review if you download the diff to your git > repository for oxygen and then run "git diff -b" to ignore whitespace > changes, particularly for the QPixmap changes. > > For QPixmaps we return values instead of pointers, so we simply make a > separate copy to be cached when we do insert. For QColor we return references > to values so we *must* return pointers, and those have to be owned by a > QCache to avoid memleaks. So I added a helper function to loop until the > cache accepts the new entry. TileSets are a similar concern, except those > have manual loops since I was uncertain about whether TileSet's copy > constructor was the best idea or not. > > This fixes a ton of Coverity issues (59717 - 259733, 259739, 259742 - 259752, > 1336154, 1336155) and might be associated with Qt bug 38142 and KDE bug > 219055 (which doesn't actually appear to be a dupe of a different bug to > me...). > > > Diffs > - > > kstyle/oxygenstylehelper.cpp 612ba37 > liboxygen/oxygenhelper.h a6453a0 > liboxygen/oxygenhelper.cpp 4843604 > liboxygen/oxygenshadowcache.cpp 907e586 > > Diff: https://git.reviewboard.kde.org/r/127866/diff/ > > > Testing > --- > > Compiled without warnings, installed and ran `oxygen-demo5 -style oxygen`. > Used the GUI Benchmark feature to automatically cycle through all the listed > features -- no crashes or obvious rendering errors. > > > Thanks, > > Michael Pyne > >
Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing
le. > > > > > => the platform integration plugin (KDE part in all Qt applications) > needs to read that setting and expose it to the styles. > Alternatively the styles could read the setting from kdeglobals directly, > but that requires them to link kconfig (to do it correctly, just grabbing it > from the users kdeglobals ini isn't that much of a problem ;-), ie. rules out > Qt-only styles (and somehow contrasts w/ the KF5 approach to things) > > > > > Ideally™ there would be an official "Qt::AA_PushButtonTextOnly" (rename > me) flag for the QPA to set and QPushButtons to pick. > > René J.V. Bertin wrote: > Hear hear ... > > I have raised that final question in the Qt bug report I filed on this > (https://bugreports.qt.io/browse/QTBUG-49887) > > Maybe the best approach is to ensure that for now ShowIconsOnButtons is > handled the best way possible in the main current KDE styles (Breeze, Oxygen, > QtCurve) and then wait to see what position Qt adopts before trying to > implement something better (= less CPU intensive)? > > Thomas Lübking wrote: > What "hear hear" - I hardly changed my opinion, just figured that the > hint is ignored all over KDE (client) code on migration from KDialogButtonBox > to QDialogButtonBox and in other places and thus considered we > -unfortunately- will best try to "fix" that (broken client code) in > QPushButton. I had simply underestimated that this is not a bug in > KDialogButtonBox only, but broken all over the place. > > Please nota bene that QPushButton is /not/ broken and does /not/ need to > be fixed. One would request it to catch and cover broken client code. They > may very well just respond "why not simply fix your shit?" > > > Maybe the best approach is to ensure that for now ShowIconsOnButtons is > handled [...] see what position Qt adopts > > In that case I'd propose to PoC a working infrastructure (QPA reads > setting and sets a dynamic property on the QApplication instance, best before > the style is constructed, so that finally the style(s) adopt that) and then > ask "can we please make this a canonical and documented application > attribute?" Hi, Jumping in. I have played with honouring ShowIconsOnButtons in Breeze style directly (on a local branch) Now: it is easy to get the icon not being shown, but the buttons are still "large", as if they would contain the icon. The reason is that, as already pointed by René, QPushButton::SizeHint already include the size of the icon to be drawn in the size it calculates and passes to QStyle::sizeFromContents. So that you would need to either "undo" the size calculated by QPushButton in the sizeHint method (which sounds quite fragile I think), or indeed, patch QPushButton in some way ... Comments welcome (especially if I missed something) - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126308/#review89737 --- On Dec. 11, 2015, 4:26 p.m., René J.V. Bertin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126308/ > --- > > (Updated Dec. 11, 2015, 4:26 p.m.) > > > Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo > Pereira Da Costa, and Yichao Yu. > > > Repository: kdelibs4support > > > Description > --- > > KF5 applications have long had a habit of drawing icons on buttons even when > this feature was turned off in the user's setting. This was mostly noticeable > in applications built on kdelibs4support. > > It seems that the actual culprit is in Qt's QPushButton implementation > (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work > around it in `KPushButton::paintEvent`, by removing the icon (forcing it to > the null icon) in the option instance, before handing off control to the > painter. > > > Diffs > - > > src/kdeui/kdialogbuttonbox.cpp 0f6649b > src/kdeui/kpushbutton.cpp 98534fa > > Diff: https://git.reviewboard.kde.org/r/126308/diff/ > > > Testing > --- > > On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 . > > I have not yet verified if there are other classes where this modification > would be relevant too. > > > Thanks, > > René J.V. Bertin > >
Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing
> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote: > > 1. What tells you that this is a dialog buttonbox pushbutton? > > 2. What happens if the button has no text? > > > > > > The bug is in QDialogButtonBox (or rather the K variant, > > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint > > correctly) > > > > > > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was > > interpreted by KPushButton _and_ kstyle for the hint, but the hint only > > covers Q'DialogButtonBox'es - there's simply no global rule for this like > > AA_DontShowIconsInMenus > > > > => KDialogButtonBox shall respect the hint for its buttons (there're two > > special creation routines). > > > > For the rest, the platform plugin ideally picks the kdeglobals setting and > > exports it to the application object (dyn property?) where the style can > > pick it and incorporate it into its calculations (ie. if no icons are > > wanted and there's text or arrow, omit the icon in size calculation and > > painting) > > > > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll > > face the mix we had, just that users of QPushButton were far less prone to > > pass them an icon in pre-KF5 times. > > > > Please also attach Hugo Pereira Da Costa. > > René J.V. Bertin wrote: > 1: why should one care? It is said nowhere that the setting defined as > "show icons on buttons" in `systemsettings(5)` applies only to dialogs. > Rather, the tooltip says "when this is enabled, KDE applications will show > icons alongside [sic!] some important buttons". > In other words, when "this" is *not* enabled, there should be no icons, > period. > I have found no sign in the code that the ShowIconsOnPushButtons hint is > to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed > carries the suggestion in its name but I would not be surprised if Qt really > thinks of it in a more general sense. Probably also because the notion of > what a dialog is has become a lot vaguer. > > And that also happens to be what Qt does; buttons show their icons on > Linux (and other Unix variants?) but on OS X or MS Windows displaying of > those icons is deactivated unless you use a style that enables it. In fact, > the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except > in the generic Unix theme (= it does work globally like > `AA_DontShowIconsInMenus` everywhere else). > > 2: a user who indicates s/he doesn't want to see icons will get an empty > button. That's also what can happen with QPushButton, and app developers have > to take this into consideration. Cf. toolbars (and Qt's position on the use > of "texted separators"). > I don't think I've ever come across a standard button showing only an > icon, except possibly the arrow button next to the progress indicators in > KMail and KDevelop. > > As to fixing it here: as it turns out, "here" is the main source for > annoying icons rearing their silly heads on buttons on my screen ;) and it > was also something of a challenge to understand why they kept appearing > despite the fact that all code appeared to return the value of > `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all > applications are going to stop using it anytime soon. > > I looked into fixing the issue in KDialogButtonBox but saw that it does > nothing to override the `ShowIconsOnPushButtons` setting. The only way to > respect the setting through that class (or a modern equivalent) would be to > set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another > regression: changes in this setting are supposed to be reflected by running > applications without requiring a restart (or a recreation of dialogs). If it > were just me I'd decree that buttons can have either text or an icon, but > right now we have to make do with this mixed situation. > > I don't mind making this an OS X (and MS Windows) specific modification, > of course, but on those platforms > > Thomas Lübking wrote: > > 1: why should one care? > > Because, as explained, that is what the hint says. Nothing else. > > > I have found no sign in the code that the ShowIconsOnPushButtons hint > is to be used only for dialogs > > No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* > KPushButton. > ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but > NOT vv. > >
Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing
> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote: > > 1. What tells you that this is a dialog buttonbox pushbutton? > > 2. What happens if the button has no text? > > > > > > The bug is in QDialogButtonBox (or rather the K variant, > > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint > > correctly) > > > > > > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was > > interpreted by KPushButton _and_ kstyle for the hint, but the hint only > > covers Q'DialogButtonBox'es - there's simply no global rule for this like > > AA_DontShowIconsInMenus > > > > => KDialogButtonBox shall respect the hint for its buttons (there're two > > special creation routines). > > > > For the rest, the platform plugin ideally picks the kdeglobals setting and > > exports it to the application object (dyn property?) where the style can > > pick it and incorporate it into its calculations (ie. if no icons are > > wanted and there's text or arrow, omit the icon in size calculation and > > painting) > > > > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll > > face the mix we had, just that users of QPushButton were far less prone to > > pass them an icon in pre-KF5 times. > > > > Please also attach Hugo Pereira Da Costa. > > René J.V. Bertin wrote: > 1: why should one care? It is said nowhere that the setting defined as > "show icons on buttons" in `systemsettings(5)` applies only to dialogs. > Rather, the tooltip says "when this is enabled, KDE applications will show > icons alongside [sic!] some important buttons". > In other words, when "this" is *not* enabled, there should be no icons, > period. > I have found no sign in the code that the ShowIconsOnPushButtons hint is > to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed > carries the suggestion in its name but I would not be surprised if Qt really > thinks of it in a more general sense. Probably also because the notion of > what a dialog is has become a lot vaguer. > > And that also happens to be what Qt does; buttons show their icons on > Linux (and other Unix variants?) but on OS X or MS Windows displaying of > those icons is deactivated unless you use a style that enables it. In fact, > the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except > in the generic Unix theme (= it does work globally like > `AA_DontShowIconsInMenus` everywhere else). > > 2: a user who indicates s/he doesn't want to see icons will get an empty > button. That's also what can happen with QPushButton, and app developers have > to take this into consideration. Cf. toolbars (and Qt's position on the use > of "texted separators"). > I don't think I've ever come across a standard button showing only an > icon, except possibly the arrow button next to the progress indicators in > KMail and KDevelop. > > As to fixing it here: as it turns out, "here" is the main source for > annoying icons rearing their silly heads on buttons on my screen ;) and it > was also something of a challenge to understand why they kept appearing > despite the fact that all code appeared to return the value of > `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all > applications are going to stop using it anytime soon. > > I looked into fixing the issue in KDialogButtonBox but saw that it does > nothing to override the `ShowIconsOnPushButtons` setting. The only way to > respect the setting through that class (or a modern equivalent) would be to > set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another > regression: changes in this setting are supposed to be reflected by running > applications without requiring a restart (or a recreation of dialogs). If it > were just me I'd decree that buttons can have either text or an icon, but > right now we have to make do with this mixed situation. > > I don't mind making this an OS X (and MS Windows) specific modification, > of course, but on those platforms > > Thomas Lübking wrote: > > 1: why should one care? > > Because, as explained, that is what the hint says. Nothing else. > > > I have found no sign in the code that the ShowIconsOnPushButtons hint > is to be used only for dialogs > > No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* > KPushButton. > ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but > NOT vv. > >
Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing
> On Dec. 10, 2015, 10:11 p.m., Thomas Lübking wrote: > > 1. What tells you that this is a dialog buttonbox pushbutton? > > 2. What happens if the button has no text? > > > > > > The bug is in QDialogButtonBox (or rather the K variant, > > QDialogButtonBoxPrivate::createButton() seems to incorporate the hint > > correctly) > > > > > > [KDE] has "ShowIconsOnPushButtons=false" in kdeglobals which was > > interpreted by KPushButton _and_ kstyle for the hint, but the hint only > > covers Q'DialogButtonBox'es - there's simply no global rule for this like > > AA_DontShowIconsInMenus > > > > => KDialogButtonBox shall respect the hint for its buttons (there're two > > special creation routines). > > > > For the rest, the platform plugin ideally picks the kdeglobals setting and > > exports it to the application object (dyn property?) where the style can > > pick it and incorporate it into its calculations (ie. if no icons are > > wanted and there's text or arrow, omit the icon in size calculation and > > painting) > > > > "Fixing" that in deprecated KPushButton doesn't really fix anything. We'll > > face the mix we had, just that users of QPushButton were far less prone to > > pass them an icon in pre-KF5 times. > > > > Please also attach Hugo Pereira Da Costa. > > René J.V. Bertin wrote: > 1: why should one care? It is said nowhere that the setting defined as > "show icons on buttons" in `systemsettings(5)` applies only to dialogs. > Rather, the tooltip says "when this is enabled, KDE applications will show > icons alongside [sic!] some important buttons". > In other words, when "this" is *not* enabled, there should be no icons, > period. > I have found no sign in the code that the ShowIconsOnPushButtons hint is > to be used only for dialogs; `SH_DialogButtonBox_ButtonsHaveIcons` indeed > carries the suggestion in its name but I would not be surprised if Qt really > thinks of it in a more general sense. Probably also because the notion of > what a dialog is has become a lot vaguer. > > And that also happens to be what Qt does; buttons show their icons on > Linux (and other Unix variants?) but on OS X or MS Windows displaying of > those icons is deactivated unless you use a style that enables it. In fact, > the default setting for `SH_DialogButtonBox_ButtonsHaveIcons` is false except > in the generic Unix theme (= it does work globally like > `AA_DontShowIconsInMenus` everywhere else). > > 2: a user who indicates s/he doesn't want to see icons will get an empty > button. That's also what can happen with QPushButton, and app developers have > to take this into consideration. Cf. toolbars (and Qt's position on the use > of "texted separators"). > I don't think I've ever come across a standard button showing only an > icon, except possibly the arrow button next to the progress indicators in > KMail and KDevelop. > > As to fixing it here: as it turns out, "here" is the main source for > annoying icons rearing their silly heads on buttons on my screen ;) and it > was also something of a challenge to understand why they kept appearing > despite the fact that all code appeared to return the value of > `ShowIconsOnPushButtons`. Deprecated or not, it doesn't look like all > applications are going to stop using it anytime soon. > > I looked into fixing the issue in KDialogButtonBox but saw that it does > nothing to override the `ShowIconsOnPushButtons` setting. The only way to > respect the setting through that class (or a modern equivalent) would be to > set an empty icon if `ShowIconsOnPushButtons=false`. That introduces another > regression: changes in this setting are supposed to be reflected by running > applications without requiring a restart (or a recreation of dialogs). If it > were just me I'd decree that buttons can have either text or an icon, but > right now we have to make do with this mixed situation. > > I don't mind making this an OS X (and MS Windows) specific modification, > of course, but on those platforms > > Thomas Lübking wrote: > > 1: why should one care? > > Because, as explained, that is what the hint says. Nothing else. > > > I have found no sign in the code that the ShowIconsOnPushButtons hint > is to be used only for dialogs > > No, but it's been used to feed SH_DialogButtonBox_ButtonsHaveIcons *AND* > KPushButton. > ShowIconsOnPushButtons implied SH_DialogButtonBox_ButtonsHaveIcons but > NOT vv. > >
Re: KScreenGenie moved to KDE Review
On 06/28/2015 06:40 PM, Hugo Pereira Da Costa wrote: On 06/28/2015 10:58 AM, Martin Koller wrote: On Thursday 18 June 2015 15:03:27 Boudhayan Gupta wrote: Here's an Imgur album with some more screenshots, including how Rectangle Selection works: http://imgur.com/a/1peZa I miss the copy button to be able to copy the captured image to the clipboard Two regressions that I've spotted running with latest KSG from master branch: 1/ as far as I can tell, the window is non resizable. KSnapshot is, and resizing the window ends up resizing the screenshot in it, which I find really useful. 2/ once you have selected Active Window + Take a new snapshot, the application waits for you to click somewhere, but as far as I can tell, the window on which you click does not matter, what you get in the screenshot is the window that was active *before* you click on take a new snapshot. This is very impractical: if you want to change active window you need first to select it (away from the ksg window, which is in fact the active one (hum: does that mean that the button is actually ill-named) ? , select an active window, then click back on kscreengenie (making this window the active one), then take a new snapshot. Ksnapshot behaves differently: you select take a new snapshot, you click on a window, the window becomes active and this is the one that gets a screenshot. I have not tested how it works when you use a delay. Another one: the delay spinbox is in 1/10 seconds unit. What use is a delay of 100 milliseconds ? KSnapshot uses seconds, which IMHO makes more sense Best regards, Hugo
Re: KScreenGenie moved to KDE Review
On 06/28/2015 10:58 AM, Martin Koller wrote: On Thursday 18 June 2015 15:03:27 Boudhayan Gupta wrote: Here's an Imgur album with some more screenshots, including how Rectangle Selection works: http://imgur.com/a/1peZa I miss the copy button to be able to copy the captured image to the clipboard Two regressions that I've spotted running with latest KSG from master branch: 1/ as far as I can tell, the window is non resizable. KSnapshot is, and resizing the window ends up resizing the screenshot in it, which I find really useful. 2/ once you have selected Active Window + Take a new snapshot, the application waits for you to click somewhere, but as far as I can tell, the window on which you click does not matter, what you get in the screenshot is the window that was active *before* you click on take a new snapshot. This is very impractical: if you want to change active window you need first to select it (away from the ksg window, which is in fact the active one (hum: does that mean that the button is actually ill-named) ? , select an active window, then click back on kscreengenie (making this window the active one), then take a new snapshot. Ksnapshot behaves differently: you select take a new snapshot, you click on a window, the window becomes active and this is the one that gets a screenshot. I have not tested how it works when you use a delay. Best regards, Hugo
Re: Review Request 121083: Replace manual export files with CMake's generate_export_header
On Nov. 22, 2014, 12:18 a.m., Harald Sitter wrote: This breaks the kde4 build as ECM is not being used there and generate_export_header is not available without ECM. Andrius da Costa Ribas wrote: generate_export_header isn't in ECM, but in cmake itself (http://www.cmake.org/cmake/help/v3.0/module/GenerateExportHeader.html). Does kde4 required cmake version not support it? Should I revert this commit? Harald Sitter wrote: Ah, I was not aware of that. So the solution probably is to move the include(GenerateExportheader) line outside the if-else for kf5/kde4 Albert Astals Cid wrote: Andrius: kdelibs4 has to work on cmake 2.8.9, when was that introduced? Harald Sitter wrote: If kubuntu packaging can be trusted it was there before that (e.g. was already in 2.8.7) Thomas Lübking wrote: http://www.cmake.org/cmake/help/v2.8.9/cmake.html#module:GenerateExportHeader I'm running cmake 3.1 here and still can't get oxygen to build in KDE4 mode anyway. Cmake aborts with: Unknown CMake command generate_export_header So surely something seems broken with this patch (missing module include ?) Can you either test compilation with option -DUSE_KDE4=1, and fix, or provide instructions, or revert ? Thanks ! Hugo - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121083/#review70761 --- On Nov. 21, 2014, 10:47 p.m., Andrius da Costa Ribas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121083/ --- (Updated Nov. 21, 2014, 10:47 p.m.) Review request for kde-workspace and kdewin. Repository: oxygen Description --- These files currently use ``` __attribute__((visibility(...))) ``` which doesn't work on MSVC. Diffs - liboxygen/oxygen_config_export.h 02ea29d liboxygen/oxygen_export.h b8877a0 liboxygen/CMakeLists.txt 69b7bd2 Diff: https://git.reviewboard.kde.org/r/121083/diff/ Testing --- Builds with msvc 2013 64bit Thanks, Andrius da Costa Ribas
Re: KF5 tab style issues
On 09/15/2014 10:42 PM, Michal Humpula wrote: Hi there, I've encountered strange issue with the style of kf5 kdevelop tabs. Specifically the close button takes more space then anything else. It seems to be drawn by QCommonStyle class, which is inherited by KStyle. What I'm not sure, where is the bug. Is it in the * QCommonStyle because it draws that big close button, or * KStyle because it does not override close button by itself, or * my specific style/setup combo, which I'm not sure what it is because Qt5 seems to choosing proper style magically by itself? As for the last one, I've seen it on at least one other machine (scummos), so it doesn't seems to be specific to my setup. Any hint, what should I fix, would be appreciated Cheers Michal Hi, please file a bug at https://bugs.kde.org/enter_bug.cgi?product=Breeze (component QStyle) providing application, steps to reproduce and screenshot tab close buttons in oxygen-demo5 (second page, and checking on the show tab close button options, look good here, but again there might be application specific issues ... Thanks ! Hugo
Re: Review Request 118763: Remove find_package(XCB) call as it is handled already by the top-level cmake file
On June 26, 2014, 11:49 a.m., Hugo Pereira Da Costa wrote:
Ship It!
Ping ? Should I commit this myself ?
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/#review61017
---
On June 16, 2014, 2:07 p.m., Bernd Steinhauser wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/
---
(Updated June 16, 2014, 2:07 p.m.)
Review request for kde-workspace, Plasma and Hugo Pereira Da Costa.
Repository: oxygen
Description
---
No idea if kde-workspace is still the right group, if not, please change.
find_package(XCB) is called without specifying the required components. This
leads to linking to unused dependencies in case they are installed.
Since XCB is searched for in the top level cmake file in the repository,
there is no need to search for it again. The component required there (only
base XCB) is sufficient.
Although, this should be sufficient to fix the deps problem, it makes sense
to link to XCB::XCB instead of ${XCB_LIBRARIES}, since the former is what is
actually needed.
Diffs
-
kstyle/CMakeLists.txt 165b62a
liboxygen/CMakeLists.txt 0d1dd94
Diff: https://git.reviewboard.kde.org/r/118763/diff/
Testing
---
Thanks,
Bernd Steinhauser
Re: Review Request 118763: Remove find_package(XCB) call as it is handled already by the top-level cmake file
On June 16, 2014, 6:57 a.m., Martin Gräßlin wrote:
looks good to me, +1. Please add Hugo Pereira Da Costa to the Review
Request, though.
The review request made me wonder whether we still need to find XLib in
Oxygen, though. The parts shown only use XCB, so maybe we could just go for
finding only XCB?
@Martin,
yes you are probably right. X11 should not be necessary any more.
I'll double check and commit. (have other stuff pending).
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/#review60162
---
On June 16, 2014, 2:07 p.m., Bernd Steinhauser wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/
---
(Updated June 16, 2014, 2:07 p.m.)
Review request for kde-workspace, Plasma and Hugo Pereira Da Costa.
Repository: oxygen
Description
---
No idea if kde-workspace is still the right group, if not, please change.
find_package(XCB) is called without specifying the required components. This
leads to linking to unused dependencies in case they are installed.
Since XCB is searched for in the top level cmake file in the repository,
there is no need to search for it again. The component required there (only
base XCB) is sufficient.
Although, this should be sufficient to fix the deps problem, it makes sense
to link to XCB::XCB instead of ${XCB_LIBRARIES}, since the former is what is
actually needed.
Diffs
-
kstyle/CMakeLists.txt 165b62a
liboxygen/CMakeLists.txt 0d1dd94
Diff: https://git.reviewboard.kde.org/r/118763/diff/
Testing
---
Thanks,
Bernd Steinhauser
Re: Review Request 118763: Remove find_package(XCB) call as it is handled already by the top-level cmake file
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/#review61017
---
Ship it!
Ship It!
- Hugo Pereira Da Costa
On June 16, 2014, 2:07 p.m., Bernd Steinhauser wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118763/
---
(Updated June 16, 2014, 2:07 p.m.)
Review request for kde-workspace, Plasma and Hugo Pereira Da Costa.
Repository: oxygen
Description
---
No idea if kde-workspace is still the right group, if not, please change.
find_package(XCB) is called without specifying the required components. This
leads to linking to unused dependencies in case they are installed.
Since XCB is searched for in the top level cmake file in the repository,
there is no need to search for it again. The component required there (only
base XCB) is sufficient.
Although, this should be sufficient to fix the deps problem, it makes sense
to link to XCB::XCB instead of ${XCB_LIBRARIES}, since the former is what is
actually needed.
Diffs
-
kstyle/CMakeLists.txt 165b62a
liboxygen/CMakeLists.txt 0d1dd94
Diff: https://git.reviewboard.kde.org/r/118763/diff/
Testing
---
Thanks,
Bernd Steinhauser
Re: Review Request 115964: Oxygen: avoid calls to reparseConfiguration() on startup.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115964/#review50797 --- Right now, I'm not able to compile, due to: home/hpereira/kf5/src/kde-workspace/kstyles/oxygen/oxygenstyle.cpp: In constructor ‘Oxygen::Style::Style()’: /home/hpereira/kf5/src/kde-workspace/kstyles/oxygen/oxygenstyle.cpp:193:60: error: ‘class Oxygen::StyleConfigData’ has no member named ‘sharedConfig’ _helper( new StyleHelper( StyleConfigData::self()-sharedConfig() ) ), Does this review depend on some pending changes in kconfig ? - Hugo Pereira Da Costa On Feb. 23, 2014, 12:08 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115964/ --- (Updated Feb. 23, 2014, 12:08 p.m.) Review request for kde-workspace and Hugo Pereira Da Costa. Repository: kde-workspace Description --- Oxygen: avoid calls to reparseConfiguration() on startup. strace -e open kate | grep -v NOENT | grep oxygenrc | wc -l went from 8 to 4 (and when looking for kdeglobals, from 13 to 11) There's still a lot of work to be done in that area... (within KConfigSkeleton, and probably kdeplatformtheme) Diffs - kstyles/oxygen/oxygenshadowhelper.h 066edd3292f390401691a9d6e5182e9dc6d24133 kstyles/oxygen/oxygenshadowhelper.cpp b3f3d0c876067292a72647e9fcda07cbbf4ba4b0 kstyles/oxygen/oxygenstyle.h 35323b4578eb9aa0d1efcc5901bab80fadea64f3 kstyles/oxygen/oxygenstyle.cpp f6ff24ad515003ad031c80665935e6d113b5a1e5 kstyles/oxygen/oxygenstylehelper.h 57b86c7ed540364040e87a8464299db17c460647 kstyles/oxygen/oxygenstylehelper.cpp f2af4172c5beb8b8c3bb0d75b5eb351dc710289e kwin/clients/oxygen/demo/oxygenshadowdemodialog.cpp 9e1481cab4cca2961b48c791e72d925ae4a83d05 kwin/clients/oxygen/oxygendecohelper.h 4c9a44ab54edec7bdc428f33e580efc60efdf235 kwin/clients/oxygen/oxygendecohelper.cpp e6bbcbf9df7eee3188c84cd74f0e32b7cfd162d3 kwin/clients/oxygen/oxygenfactory.h 9ed90ec1567ed537a3524b2d4a507a3471341a64 kwin/clients/oxygen/oxygenfactory.cpp 6d5b08b9160a7794f668feb2914450b602360ce6 libs/oxygen/oxygenhelper.h ee381a74320da172e834550f61ef214377870bfc libs/oxygen/oxygenhelper.cpp cd6562b411fdb1bce6d0c740a4b38dae33218324 Diff: https://git.reviewboard.kde.org/r/115964/diff/ Testing --- Thanks, David Faure
Re: Review Request 115964: Oxygen: avoid calls to reparseConfiguration() on startup.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115964/#review50816 --- Ship it! code is good and works well. Thanks ! - Hugo Pereira Da Costa On Feb. 25, 2014, 12:02 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115964/ --- (Updated Feb. 25, 2014, 12:02 p.m.) Review request for kde-workspace and Hugo Pereira Da Costa. Repository: kde-workspace Description --- Oxygen: avoid calls to reparseConfiguration() on startup. strace -e open kate | grep -v NOENT | grep oxygenrc | wc -l went from 8 to 4 (and when looking for kdeglobals, from 13 to 11) There's still a lot of work to be done in that area... (within KConfigSkeleton, and probably kdeplatformtheme) Diffs - kstyles/oxygen/oxygenshadowhelper.h 066edd3292f390401691a9d6e5182e9dc6d24133 kstyles/oxygen/oxygenshadowhelper.cpp b3f3d0c876067292a72647e9fcda07cbbf4ba4b0 kstyles/oxygen/oxygenstyle.h 35323b4578eb9aa0d1efcc5901bab80fadea64f3 kstyles/oxygen/oxygenstyle.cpp f6ff24ad515003ad031c80665935e6d113b5a1e5 kstyles/oxygen/oxygenstylehelper.h 57b86c7ed540364040e87a8464299db17c460647 kstyles/oxygen/oxygenstylehelper.cpp f2af4172c5beb8b8c3bb0d75b5eb351dc710289e kwin/clients/oxygen/demo/oxygenshadowdemodialog.cpp 9e1481cab4cca2961b48c791e72d925ae4a83d05 kwin/clients/oxygen/oxygendecohelper.h 4c9a44ab54edec7bdc428f33e580efc60efdf235 kwin/clients/oxygen/oxygendecohelper.cpp e6bbcbf9df7eee3188c84cd74f0e32b7cfd162d3 kwin/clients/oxygen/oxygenfactory.h 9ed90ec1567ed537a3524b2d4a507a3471341a64 kwin/clients/oxygen/oxygenfactory.cpp 6d5b08b9160a7794f668feb2914450b602360ce6 libs/oxygen/oxygenhelper.h ee381a74320da172e834550f61ef214377870bfc libs/oxygen/oxygenhelper.cpp cd6562b411fdb1bce6d0c740a4b38dae33218324 Diff: https://git.reviewboard.kde.org/r/115964/diff/ Testing --- Thanks, David Faure
Re: Review Request 115515: [oxygen] Check whether we are on platform X11 before calling into xcb
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115515/#review49179 --- Ship it! ship it, again :) - Hugo Pereira Da Costa On Feb. 6, 2014, 12:22 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115515/ --- (Updated Feb. 6, 2014, 12:22 p.m.) Review request for kde-workspace and Hugo Pereira Da Costa. Repository: kde-workspace Description --- [oxygen] Check whether we are on platform X11 before calling into xcb Just because we compiled with X11 present doesn't mean we run on X11. This fixes quite a lot of crashers when trying to run framework apps on Wayland. @Hugo: do you know of further files which use xcb unconditionally and which I just haven't hit yet? Diffs - kstyles/oxygen/CMakeLists.txt ca9cc1a1710187f5013482ef502c456238fd4373 kstyles/oxygen/oxygenblurhelper.cpp d774b2c45f8ec452311d34f35adf4d9bcc39693d kstyles/oxygen/oxygenshadowhelper.cpp f77093daa4f907afd55333032c6f6b618ad2f47f kstyles/oxygen/oxygenstylehelper.cpp 7e2de41822bf0205e2cec7ea82c8e3f6751a2a6b kstyles/oxygen/oxygenwindowmanager.h 7ef54d3172b0b0a92b6c33152858cb923b9d3f1e kstyles/oxygen/oxygenwindowmanager.cpp 308ce4d049b6ce4c9c2cdd67448d351747f34b19 libs/oxygen/oxygenhelper.h 8d734e6e3f3a23c0257cc400417c340ba6d48bea libs/oxygen/oxygenhelper.cpp 1fe78c24c053e74f2179de00ee9a5701997a0acb Diff: https://git.reviewboard.kde.org/r/115515/diff/ Testing --- running Kate on Wayland till it crashes (or doesn't) Thanks, Martin Gräßlin
Re: Review Request 112335: In oxygen: Use the iconSize from mainToolbar
On Jan. 7, 2014, 5:44 a.m., Àlex Fiestas wrote: Hey hugo, is this in somehow now that oxygen uses KStyle again? Hi Alex, yes this has been removed from Oxygen::pixelMetric and moved to kstyle::pixelMetric. However it has the same issue: case PM_ToolBarIconSize: return KIconLoader::global()-currentSize(KIconLoader::Toolbar); so feel free to move the fix there and commit. Hugo - Hugo --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112335/#review46952 --- On Aug. 28, 2013, 5:09 p.m., Àlex Fiestas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112335/ --- (Updated Aug. 28, 2013, 5:09 p.m.) Review request for kde-workspace and Hugo Pereira Da Costa. Repository: kde-workspace Description --- In the qplatformplugin we use information from MainToolbar (which makes sense) but in the styles we use information from Toolbar. This unify both by using MainToolbar in styles (if accepted will modify kstyle as well). Diffs - kstyles/oxygen/oxygenstyle.cpp 83aaf5c Diff: https://git.reviewboard.kde.org/r/112335/diff/ Testing --- Thanks, Àlex Fiestas
Re: Review Request 114537: Fix progressbar's busy animation with QtQuickControls
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114537/#review45932 --- Ship it! Ship It! - Hugo Pereira Da Costa On Dec. 19, 2013, 11:18 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114537/ --- (Updated Dec. 19, 2013, 11:18 a.m.) Review request for kde-workspace and Hugo Pereira Da Costa. Repository: kde-workspace Description --- This makes the animated busy state of QtQuickControls' progressbar actually move. Diffs - kstyles/oxygen/animations/oxygenbusyindicatorengine.cpp da4d67e kstyles/oxygen/oxygenstyle.cpp cf371ca Diff: http://git.reviewboard.kde.org/r/114537/diff/ Testing --- Tested QQC gallery and QWidget5 gallery, both works correctly. Thanks, Martin Klapetek
Review Request 114328: re-add customstyleelement suite to kstyle
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114328/ --- Review request for kdelibs. Repository: kdelibs Description --- re-add customstyleelement suite to kstyle Diffs - tier4/frameworkintegration/src/kstyle/kstyle.h 8a881f5 tier4/frameworkintegration/src/kstyle/kstyle.cpp 27d407e Diff: http://git.reviewboard.kde.org/r/114328/diff/ Testing --- compiles, works, fix kde-workspace build also: will be used when moving oxygen from qcommonstyle back to kstyle (right now we have a fork of some of the said methods) Thanks, Hugo Pereira Da Costa
projects page dead ?
https://projects.kde.org/projects/ gives 'bad gateway'. Is it just me ? A known (hopefully temporary) issue ? Has the page moved ? Thanks in advance, Hugo
kde-workspace git broken ?
Hello,
I think commits
9f70241d57f3ba1013b9f28650478c8bbb1233e0
137dd285bdf821fd2c8a5c17e30dc9c1a6eca87b
09ea308ab55505efe7aeaebcd4aef6292cd884e6
seriously broke kde-workspace
At least several changes in oxygen where reverted in the process.
(below is a diff of oxygenstyle.h between
008ac5efabfb99f04813e5dad29c2d0a92d13fc5 and current master), which
should definitly not be there ...
can someone more git-knowledgable check/confirm/fix ?
Hugo
diff --git a/kstyles/oxygen/oxygenstyle.h b/kstyles/oxygen/oxygenstyle.h
index afe632b..59f2ae4 100644
--- a/kstyles/oxygen/oxygenstyle.h
+++ b/kstyles/oxygen/oxygenstyle.h
@@ -24,7 +24,7 @@
// (c) 2002,2003 Maksim Orlovich mo0...@mail.rochester.edu
// based on the KDE3 HighColor Style
// Copyright (C) 2001-2002 Karol Szwed gall...@kde.org
-// (C) 2001-2002 Fredrik Höglund fred...@kde.org
+// (C) 2001-2002 Fredrik Höglund fred...@kde.org
// Drawing routines adapted from the KDE2 HCStyle,
// Copyright (C) 2000 Daniel M. Duley mos...@kde.org
// (C) 2000 Dirk Mueller muel...@kde.org
@@ -51,24 +51,18 @@
#include oxygenmetrics.h
#include oxygentileset.h
-#include QMap
-#include QAbstractScrollArea
-#include QCommonStyle
-#include QDockWidget
-#include QMdiSubWindow
-#include QStyleOption
-#include QStyleOptionSlider
-#include QStylePlugin
-#include QToolBar
-#include QToolBox
-#include QWidget
-
-#include QIcon
-
-namespace OxygenPrivate
-{
- class TabBarData;
-}
+#include QtCore/QMap
+#include QtGui/QAbstractScrollArea
+#include QtGui/QCommonStyle
+#include QtGui/QDockWidget
+#include QtGui/QMdiSubWindow
+#include QtGui/QStyleOption
+#include QtGui/QStyleOptionSlider
+#include QtGui/QToolBar
+#include QtGui/QToolBox
+#include QtGui/QWidget
+
+#include KIcon
namespace Oxygen
{
@@ -85,23 +79,6 @@ namespace Oxygen
class WidgetExplorer;
class BlurHelper;
- class StylePlugin : public QStylePlugin
- {
- Q_OBJECT
- Q_PLUGIN_METADATA(IID org.qt-project.Qt.QStyleFactoryInterface FILE
oxygen.json )
-
- public:
-
- //! constructor
- StylePlugin(QObject *parent = 0):
- QStylePlugin(parent)
- {}
-
- //! create style
- QStyle* create( const QString );
-
- };
-
//! toplevel manager
class TopLevelManager: public QObject
{
@@ -197,6 +174,7 @@ namespace Oxygen
bool eventFilterComboBoxContainer( QWidget*, QEvent* );
bool eventFilterDockWidget( QDockWidget*, QEvent* );
bool eventFilterMdiSubWindow( QMdiSubWindow*, QEvent* );
+ bool eventFilterQ3ListView( QWidget*, QEvent* );
bool eventFilterScrollBar( QWidget*, QEvent* );
bool eventFilterTabBar( QWidget*, QEvent* );
bool eventFilterToolBar( QToolBar*, QEvent* );
@@ -211,7 +189,7 @@ namespace Oxygen
//@}
- protected Q_SLOTS:
+ protected slots:
//! update oxygen configuration
void oxygenConfigurationChanged( void );
@@ -226,7 +204,10 @@ namespace Oxygen
{ return pixelMetric(PM_DefaultLayoutSpacing, option, widget); }
//! standard icons
- virtual QIcon standardIcon( StandardPixmap, const QStyleOption* = 0,
const QWidget* = 0) const;
+ virtual QIcon standardIconImplementation(
+ StandardPixmap standardIcon,
+ const QStyleOption *option,
+ const QWidget *widget) const;
protected:
@@ -300,6 +281,59 @@ namespace Oxygen
//! list of slabs
typedef QListSlabRect SlabRectList;
+ /*!
+ tabBar data class needed for
+ the rendering of tabbars when
+ one tab is being drawn
+ */
+ class TabBarData: public QObject
+ {
+
+ public:
+
+ //! constructor
+ explicit TabBarData( Style* parent ):
+ QObject( parent ),
+ _style( parent ),
+ _dirty( false )
+ {}
+
+ //! destructor
+ virtual ~TabBarData( void )
+ {}
+
+ //! assign target tabBar
+ void lock( const QWidget* widget )
+ { _tabBar = widget; }
+
+ //! true if tabbar is locked
+ bool locks( const QWidget* widget ) const
+ { return _tabBar _tabBar.data() == widget; }
+
+ //! set dirty
+ void setDirty( const bool value = true )
+ { _dirty = value; }
+
+ //! release
+ void release( void )
+ { _tabBar.clear(); }
+
+ //! draw tabBarBase
+ virtual void drawTabBarBaseControl( const QStyleOptionTab*, QPainter*,
const QWidget* );
+
+ private:
+
+ //! pointer to parent style object
+ QWeakPointerconst Style _style;
+
+ //! pointer to target tabBar
+ QWeakPointerconst QWidget _tabBar;
+
+ //! if true, will paint on next TabBarTabShapeControlCall
+ bool _dirty;
+
+ };
+
//@}
//! animations
@@ -341,6 +375,10 @@ namespace Oxygen
SplitterFactory splitterFactory( void ) const
{ return *_splitterFactory; }
+ //! tabBar data
+ TabBarData tabBarData( void ) const
+ { return *_tabBarData; }
+
//!@name subelementRect specialized functions
//@{
@@ -477,6 +515,8 @@ namespace Oxygen
bool drawPanelItemViewItemPrimitive( const QStyleOption*, QPainter*,
const QWidget* ) const;
bool drawPanelLineEditPrimitive( const QStyleOption*, QPainter*, const
QWidget* ) const;
bool drawIndicatorMenuCheckMarkPrimitive( const QStyleOption*,
QPainter*, const QWidget*
Re: kde-workspace git broken ?
On 11/11/2013 02:13 PM, Martin Gräßlin wrote: On Monday 11 November 2013 13:29:32 Hugo Pereira Da Costa wrote: Hello, I think commits 9f70241d57f3ba1013b9f28650478c8bbb1233e0 137dd285bdf821fd2c8a5c17e30dc9c1a6eca87b 09ea308ab55505efe7aeaebcd4aef6292cd884e6 seriously broke kde-workspace yes, it's broken. I already created a sysadmin request. Unfortunately a similar commit got just pushed to KDE/4.11. So at the moment both master and KDE/4.11 branch are broken. Please remember: do not push a revert of a merge commit. Is it possible to have a commit hook that would prevent such revert ? Cheers Martin
please delete kde-workspace master-xcb remote branch
Hi, could someone with proper credentials delete the kde-workspace remote branch called master-xcb, I just created it *by mistake* while pushing some changes to master ? Thanks in advance, Hugo
Re: Review Request: make folderview icons translucent if composite is enabled
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101463/#review13308 --- For the record, I have implemented a 'modified' version of Thomas patch in oxygen (without the clearMask() call that could possibly break things elsewhere), and it just works (without this current patch implemented). See: http://wstaw.org/m/2012/05/03/dnd0.png This is pushed to master. I do not think it requires backporting to KDE 4.8 (As a side note, I have also implemented alpha channel for Window tabbing dnd icons, in oxygen-decoration, to test the above. See: http://wstaw.org/m/2012/05/03/dnd1.png) I believe this review request can therefore be discarder. - Hugo Pereira Da Costa On April 19, 2012, 11:29 p.m., Mathias Stephan Panzenböck wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101463/ --- (Updated April 19, 2012, 11:29 p.m.) Review request for KDE Base Apps. Description --- This patch makes dragged folderview icons translucent if composite is enabled. It is a kinda hack that uses an event filter to find Qt's D'n'D window, clears any mask on it and sets the Qt::WA_TranslucentBackground attribute. I use it day to day and it works fine. The proper place to fix this would be in Qt, but they wrongfully marked the bug report as invalid, because they think X11 does not support translucent windows: http://bugreports.qt.nokia.com/browse/QTBUG-8519 This addresses bug 256475. http://bugs.kde.org/show_bug.cgi?id=256475 Diffs - plasma/applets/folderview/iconview.h e648ff0 plasma/applets/folderview/iconview.cpp 3186b18 Diff: http://git.reviewboard.kde.org/r/101463/diff/ Testing --- Thanks, Mathias Stephan Panzenböck
Re: Review Request: make folderview icons translucent if composite is enabled
On May 29, 2011, 1:05 p.m., Thomas Lübking wrote:
Not sure wheher it's really worth it (though using ARGB over XShape might
actually bring better performance) but I assume the style (oxygen) can deal
this more efficiently (via polishment) and also globally (not only for the
folderview plasmoid but _all_ Qt icon drags)
Gonna try and send Hugo a patch if it works.
Thomas Lübking wrote:
Yes, is. As trivial as
if (widget-testAttribute(Qt::WA_X11NetWmWindowTypeDND)
FX::compositingActive()) // uses KWindowSystem, FX is bespin
{
widget-setAttribute(Qt::WA_TranslucentBackground);
widget-clearMask();
}
Mathias Stephan Panzenböck wrote:
If you have *a lot* of desktop icons and drag them all, the current code
(using a mask and no ARGB window) makes kwin so slow, that the automatic
desktop effects deactivation kicks in. So I think just for that it is worth
to apply this (or a similar) patch. Also I think using these masked icons are
really ugly. If no ARGB window can be used it would be better to use drag
icons like dolphin does.
I agree with Thomas it would be much less of a hack if it goes in the style.
Will add in there.
(PS: I see other places where I'd love ARGB in DND, namely when moving title
windows around for window tabbing).
- Hugo
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101463/#review3574
---
On April 19, 2012, 11:29 p.m., Mathias Stephan Panzenböck wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101463/
---
(Updated April 19, 2012, 11:29 p.m.)
Review request for KDE Base Apps.
Description
---
This patch makes dragged folderview icons translucent if composite is
enabled. It is a kinda hack that uses an event filter to find Qt's D'n'D
window, clears any mask on it and sets the Qt::WA_TranslucentBackground
attribute. I use it day to day and it works fine.
The proper place to fix this would be in Qt, but they wrongfully marked the
bug report as invalid, because they think X11 does not support translucent
windows:
http://bugreports.qt.nokia.com/browse/QTBUG-8519
This addresses bug 256475.
http://bugs.kde.org/show_bug.cgi?id=256475
Diffs
-
plasma/applets/folderview/iconview.h e648ff0
plasma/applets/folderview/iconview.cpp 3186b18
Diff: http://git.reviewboard.kde.org/r/101463/diff/
Testing
---
Thanks,
Mathias Stephan Panzenböck
Re: Using KColorScheme in style only (Was: fix frameworks-kactions compile error)
On 04/19/2012 11:27 AM, Stephen Kelly wrote:
Hi, can anyone who knows about styles/KColorScheme comment on this?
Thanks,
Stephen Kelly wrote:
Kevin Ottens wrote:
On Wednesday 11 April 2012 23:34:18 Stephen Kelly wrote:
[...]
At the center of the questions of what to do with KUrlLabel and
KCapacityBar are the question of what to do about their KColorScheme
dependency. Their use of KColorScheme seems to me like something that
should be handled by the style instead (otherwise, for example,
QProgressBar wouldn't look consistent with a KCapacityBar with its
backgrounds and fill etc). Oxygen may already even handle a KCapacityBar
(I'm not certain):
yes. Oxygen does overwrite the default rendering for kcapacitybar.
kde-workspace/kstyles/oxygen{master}$ git grep CapacityBar
oxygenstyle.cpp:CE_CapacityBar( newControlElement(
CE_CapacityBar ) )
oxygenstyle.cpp:if( element == CE_CapacityBar )
oxygenstyle.cpp:fcn =Style::drawCapacityBarControl;
oxygenstyle.cpp:bool Style::drawCapacityBarControl( const
QStyleOption* option, QPainter* painter, const QWidget* widget ) const
oxygenstyle.h:virtual bool drawCapacityBarControl( const
QStyleOption*, QPainter*, const QWidget* ) const;
oxygenstyle.h:QStyle::ControlElement CE_CapacityBar;
KColorScheme is very different from QPalette. It seems to have 4
dimensions that QPalette doesn't have. I'm all for configuration, but I
think it should have an affect in the style, not in the widgets
themselves. Any thoughts?
Would it be possible to remove the use of KColorScheme from these
widgets in general?
In my oppinion, yes.
Oxygen does use kcolorscheme extensively (that's actually one of the
reason why it can't easily be isolated as a Qt only widget style)
I believe it's ok if default rendering of custom kdeui widgets uses Qt
only (QPalette, QStyle)
One can always overwrite the default rendering to benefit from whatever
KDE features you want.
I'm unfortunately completely ignorant regarding KColorScheme... I agree
it should probably not leak into our widgets and be used by the styles
only though.
Hugo, any ideas/comment on this? The background is here:
http://thread.gmane.org/gmane.comp.kde.devel.frameworks/473/focus=485
Thanks,
Re: Binary compatiblity for liboxygenstyle.so
Adding Ralf in CC, since he is experiencing the same issues. -- This starts to get off-topic, but I see the same with zsh, as in I started zsh from bash and run the above tests, with : ending it works, without it doesn't. There's either some weird statement in your local or global shell profile or Qt (4.8/git, vanilla/distro?) is broken on interpreting that env. print_env.sh -- #!/bin/sh env -- chmod +x print_env.sh QT_PLUGIN_PATH=/opt/kde4/lib64/kde4/plugins ./print_env | grep QT_PLUGIN_PATH QT_PLUGIN_PATH=/opt/kde4/lib64/kde4/plugins: ./print_env | grep QT_PLUGIN_PATH If the correct value ends up there, i'd bet for a Qt bug (the other dir could be just a failsafe thing, ie if the configured plugin dir doesn't exist, Qt might lookup likely places in it's own path. Good news is that you can continue using bash - iff you still want to, that is ;-) Cheers, Thomas
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/12 19:38, Andras Mantia wrote: On Friday, February 24, 2012 05:48:36 PM Sune Vuorela wrote: On 2012-02-24, Hugo Pereira Da Costah...@oxygen-icons.org wrote: I understand that. The point I was trying to make, is that you would still get the old pluggin, admittingly without crashing, but which would nonetheless be not correct. Whattabout just linking liboxygenstyle static into the oxygen style? Problem solved. No more abi to manage. Alternatively, set the SOVERSION of liboxygenstyle to the version of KDE Workspace. As you have no external dependencies outside kde-workspace this should not give any problems for anyone. I tend to think proper soversion bumping would be a good solution. In both cases it does not fix (sorry for repeating myself) the true issue, which is that your plugin_path is not consistent with your LD_LIBRARY_PATH. In fact, thinking more about it, I would be inclined to leave the code as it is now, because such BIC crashes make it fairly easy to spot the inconsistency, which otherwise can have much more pernicious effects. For instance I have had many bug reports about this or that new option in the configuration ... does not work (and it actually does work, except you are using the old pluggin), or worse: this or that optimization of the code (to fix slow pixmap allocation on NVIDIA to name one, as reported in a bug), that has no effect at all in my case (says the reporter), again because of loading the wrong pluggin. These two examples are much more complicated to spot and fix than the crash cases, and would occur more often if the BIC crash cases becomes fixed, by any of the solutions proposed above. Also note that nowhere in KDE you can find an About Oxygen button that would tell you which so version you are using anyway ... Cheers, Hugo Andras
Re: Binary compatiblity for liboxygenstyle.so
Hi Andras, no there is no guarantee for liboxygenstyle.so.4, and I don't plan to guarantee it. It is all internal to kde-workspace, and has no public api. the fact that oxygen gets broken for kde installed from source due to incorrect plugin path is an issue with Qt installantion, and/or with local KDE install (experts should confirm), that must be fixed upstream, disregarding of the above. In fact the same issue can lead (and has led) to crashes elsewhere, notably in the decorations. Therefore I don't think it justifies ensuring the the BIC you ask, which would generate extra -and IMHO unnecessary- complexity, if not overhead. Sorry, Hugo Hi, is there any BC guarantee for liboxygenstyle.so.4? If not, i think there should be... It is not the first time that you cannot run application installed by your distribution under a self-compiled KDE master, because BIC issues in lliboxygenstyle.so. The apps from there load the oxygen.so plugin from the system directories, but as the link against the master's liboxygenstyle.so dynamically, so if that changes in a BIC way, the apps crash and don't start. Eg: qtcreator: symbol lookup error: /usr/lib64/kde4/plugins/styles/oxygen.so: undefined symbol: _ZN6Oxygen7TileSetC1ERK7QPixmap Right now the problem was most probably was the following commit: http://commits.kde.org/kde-workspace/04490c8a827347ed41b9b1bee0539cea750ddf50 I know this does not affect regular releases in distributions, but it is very bad for those working/testing KDE master. Andras
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/2012 11:21 AM, Hugo Pereira Da Costa wrote: Hi Andras, no there is no guarantee for liboxygenstyle.so.4, and I don't plan to guarantee it. It is all internal to kde-workspace, and has no public api. the fact that oxygen gets broken for kde installed from source due to incorrect plugin path is an issue with Qt installantion, and/or with local KDE install (experts should confirm), that must be fixed upstream, disregarding of the above. In fact the same issue can lead (and has led) to crashes elsewhere, notably in the decorations. Therefore I don't think it justifies ensuring the the BIC you ask, which would generate extra -and IMHO unnecessary- complexity, if not overhead. If I remember correctly, the recipy for getting the right pluggin path (and thus avoid the crashes -always-), is to edit $HOME/.config/Trolltech.conf, and remove (or fix) the libraryPath section in [qt]. Sorry, Hugo Hi, is there any BC guarantee for liboxygenstyle.so.4? If not, i think there should be... It is not the first time that you cannot run application installed by your distribution under a self-compiled KDE master, because BIC issues in lliboxygenstyle.so. The apps from there load the oxygen.so plugin from the system directories, but as the link against the master's liboxygenstyle.so dynamically, so if that changes in a BIC way, the apps crash and don't start. Eg: qtcreator: symbol lookup error: /usr/lib64/kde4/plugins/styles/oxygen.so: undefined symbol: _ZN6Oxygen7TileSetC1ERK7QPixmap Right now the problem was most probably was the following commit: http://commits.kde.org/kde-workspace/04490c8a827347ed41b9b1bee0539cea750ddf50 I know this does not affect regular releases in distributions, but it is very bad for those working/testing KDE master. Andras
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/2012 11:25 AM, Hugo Pereira Da Costa wrote: On 02/24/2012 11:21 AM, Hugo Pereira Da Costa wrote: Hi Andras, no there is no guarantee for liboxygenstyle.so.4, and I don't plan to guarantee it. It is all internal to kde-workspace, and has no public api. the fact that oxygen gets broken for kde installed from source due to incorrect plugin path is an issue with Qt installantion, and/or with local KDE install (experts should confirm), that must be fixed upstream, disregarding of the above. In fact the same issue can lead (and has led) to crashes elsewhere, notably in the decorations. Therefore I don't think it justifies ensuring the the BIC you ask, which would generate extra -and IMHO unnecessary- complexity, if not overhead. oh and forget the bit about decorations, its actually not true (or unrelated). If I remember correctly, the recipy for getting the right pluggin path (and thus avoid the crashes -always-), is to edit $HOME/.config/Trolltech.conf, and remove (or fix) the libraryPath section in [qt]. Sorry, Hugo Hi, is there any BC guarantee for liboxygenstyle.so.4? If not, i think there should be... It is not the first time that you cannot run application installed by your distribution under a self-compiled KDE master, because BIC issues in lliboxygenstyle.so. The apps from there load the oxygen.so plugin from the system directories, but as the link against the master's liboxygenstyle.so dynamically, so if that changes in a BIC way, the apps crash and don't start. Eg: qtcreator: symbol lookup error: /usr/lib64/kde4/plugins/styles/oxygen.so: undefined symbol: _ZN6Oxygen7TileSetC1ERK7QPixmap Right now the problem was most probably was the following commit: http://commits.kde.org/kde-workspace/04490c8a827347ed41b9b1bee0539cea750ddf50 I know this does not affect regular releases in distributions, but it is very bad for those working/testing KDE master. Andras
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/2012 11:43 AM, Pino Toscano wrote: Alle venerdì 24 febbraio 2012, Hugo Pereira Da Costa ha scritto: no there is no guarantee for liboxygenstyle.so.4, and I don't plan to guarantee it. It is all internal to kde-workspace, and has no public api. Then please correctly bump its SONAME, instead of using the same SONAME (liboxygenstyle.so.4) for API-/ABI- incompatible new versions of it. Hi Pino, I think the real issue here is: despite the fact that you have a local kde build, the pluggins that gets loaded by Qt when running a KDE or Qt application are not the ones you compiled. An old one is used instead, that is inconsistent with the libraries it links to. The new, compiled one, should get loaded. No change of so name will fix that. Correct ? Hugo
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/2012 11:51 AM, Thiago Macieira wrote: On sexta-feira, 24 de fevereiro de 2012 11.49.44, Hugo Pereira Da Costa wrote: despite the fact that you have a local kde build, the pluggins that gets loaded by Qt when running a KDE or Qt application are not the ones you compiled. An old one is used instead, that is inconsistent with the libraries it links to. The new, compiled one, should get loaded. No change of so name will fix that. Correct ? Correct. Make sure that your new KDE's installation is found by the Qt plugin system, either by setting QT_PLUGIN_PATH or by editing Trolltech.conf. Side note: since this is actually a recurring issue, I guess it should be documented on techbase (or elswhere more appropriate; suggestions welcome). I'm ready to do so, but might need help, proof-reading from expert. Thiago ? Cheers, Hugo
Re: Binary compatiblity for liboxygenstyle.so
On 02/24/2012 11:56 AM, Andras Mantia wrote: On Friday, February 24, 2012 11:51:51 AM Thiago Macieira wrote: Correct. Make sure that your new KDE's installation is found by the Qt plugin system, either by setting QT_PLUGIN_PATH or by editing Trolltech.conf. I have this: printenv QT_PLUGIN_PATH /opt/qt4/plugins:/opt/kde4/lib64/kde4/plugins:/encrypted/home/andris/.kde4/lib64/kde4/plugins/:/opt/kde4/lib64/kde4/plugins/ cat ~/.config/Trolltech.conf [qt] 4.7\libraryPath=/opt/kde4/lib64/kde4/plugins 4.8\libraryPath=/opt/kde4/lib64/kde4/plugins qtcreator QPainter::begin: Paint device returned engine == 0, type: 2 QPainter::setCompositionMode: Painter not active qtcreator: symbol lookup error: /usr/lib64/kde4/plugins/styles/oxygen.so: undefined symbol: _ZN6Oxygen7TileSetC1ERK7QPixmap are other applications also crashing (like, e.g. plasma workspace, or kwin) ? (It should) if not it might be a QtCreator specific issue. No matter what I do, it still loads from /usr/lib64. Andras
Re: Review Request: #include fixx11h.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103638/#review9843 --- As far as Oxygen is concerned, feel free to ship it. - Hugo Pereira Da Costa On Jan. 15, 2012, 12:53 p.m., Erik Sigra wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103638/ --- (Updated Jan. 15, 2012, 12:53 p.m.) Review request for KDE Base Apps and Hugo Pereira Da Costa. Description --- Add #include fixx11h.h in some places where it was missing. This improves buildability. Diffs - kcminit/main.cpp 68bfb05 kcontrol/access/kaccess.h c364b25 kcontrol/access/kcmaccess.cpp 64a6803 kcontrol/bell/bell.cpp 3d9395f kcontrol/fonts/fonts.cpp 61886d4 kcontrol/input/kapplymousetheme.cpp c9bc511 kcontrol/input/main.cpp 0f9f33a kcontrol/input/mouse.cpp cebb174 kcontrol/input/xcursor/legacytheme.cpp 28d7f2a kcontrol/input/xcursor/previewwidget.cpp aff149b kcontrol/input/xcursor/thememodel.cpp d69dde0 kcontrol/input/xcursor/themepage.cpp 0f678ed kcontrol/input/xcursor/xcursortheme.cpp 2fc7504 kcontrol/keyboard/kcmmisc.cpp 382f026 kcontrol/keyboard/keyboard_hardware.cpp 9f9c026 kcontrol/keyboard/xinput_helper.cpp 2971d20 kcontrol/kfontinst/lib/FcEngine.cpp 8b74fce kcontrol/krdb/krdb.cpp 92d84e9 kcontrol/randr/krandrapp.cpp af53671 kcontrol/randr/module/randrpolltest.cpp 30d689e kcontrol/style/kcmstyle.cpp b8f46be kdm/kcm/background/bgrender.cpp 17cf33d kdm/kfrontend/krootimage.cpp e4ddc85 khotkeys/libkhotkeysprivate/actions/keyboard_input_action.cpp b3b1ec3 khotkeys/libkhotkeysprivate/triggers/gestures.cpp e70c074 khotkeys/libkhotkeysprivate/windows_handler.cpp eb02374 kinfocenter/Modules/base/os_base.h 9c903ea kinfocenter/Modules/opengl/opengl.cpp 7df2b17 klipper/klipper.cpp aafe616 krunner/krunnerdialog.cpp 007887f krunner/lock/lockprocess.cc 65c7f1d krunner/screensaver/xautolock.cpp 7124215 krunner/startupid.cpp a436183 kscreensaver/libkscreensaver/kscreensaver.cpp 55bcbea ksmserver/fadeeffect.h 8d45ebc ksmserver/fadeeffect.cpp 8b94834 ksmserver/kcheckrunning.cpp f0648fc ksmserver/legacy.cpp 62a4672 ksmserver/logouteffect.cpp a2fc060 ksplash/ksplashqml/SplashApp.h 9b63c4e ksplash/ksplashqml/main.cpp d59bff8 ksplash/simple/main.cpp 0e730bf kstyles/oxygen/oxygenshadowhelper.cpp 28cc651 kstyles/oxygen/oxygenstylehelper.cpp 24710cd ksystraycmd/main.cpp 9a72389 kwin/atoms.h 95e1bde kwin/clients/b2/b2client.cpp 6b52996 kwin/clients/oxygen/oxygenclient.cpp cd94eb4 kwin/clients/oxygen/oxygensizegrip.cpp 221ee74 kwin/effects/resize/resize.cpp 84bdd7f kwin/effects/showfps/showfps.cpp 5161401 kwin/effects/showpaint/showpaint.cpp f689b1c kwin/kcmkwin/kwindecoration/preview.cpp a3d4256 kwin/kcmkwin/kwinoptions/mouse.cpp 51a80f9 kwin/kcmkwin/kwinoptions/windows.cpp 30c94c0 kwin/killer/killer.cpp d37a654 kwin/killwindow.cpp 57e15e5 kwin/libkwineffects/kwinglobals.cpp 575813e kwin/opengltest/opengltest.cpp eda7b51 kwin/outline.cpp 6ef499c kwin/overlaywindow.h 14d2d58 kwin/tabbox/tabboxhandler.cpp e91ea71 kwin/tools/decobenchmark/preview.cpp 429cbd2 kwin/tools/test_gravity.cpp 4ddb136 kwin/utils.cpp a9abc5d kwin/workspace.h 40562d4 libs/kephal/service/xrandr12/randrdisplay.h 3a6392c libs/kworkspace/kdisplaymanager.cpp 28fabfc plasma/desktop/shell/plasmaapp.cpp 7abd8fc plasma/generic/applets/systemtray/protocols/fdo/fdoselectionmanager.cpp 4257202 plasma/generic/applets/systemtray/protocols/fdo/x11embedcontainer.cpp 1826512 plasma/generic/dataengines/mouse/cursornotificationhandler.h 7b4d3eb plasma/netbook/shell/plasmaapp.cpp 22c54b2 powerdevil/daemon/actions/dpms/powerdevildpmsaction.cpp a16bf7e powerdevil/daemon/backends/upower/xrandrbrightness.h 875c667 Diff: http://git.reviewboard.kde.org/r/103638/diff/diff Testing --- Thanks, Erik Sigra
Re: Review Request: fix the rect passed to kcapacitybar paint method
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103669/ --- (Updated Jan. 12, 2012, 7:06 p.m.) Review request for Dolphin, kdelibs and Solid. Changes --- Changed rect() into contentsRect(), as advised by Thomas Description --- fixes the rect passed to paint method so that it matches the widget rect and not the event (clip) rect, since the former is what's expected by widget styles. This addresses bug 290879. http://bugs.kde.org/show_bug.cgi?id=290879 Diffs (updated) - kdeui/widgets/kcapacitybar.cpp 6e63c3f Diff: http://git.reviewboard.kde.org/r/103669/diff/diff Testing --- Fixes bug above. No regression with other styles. Thanks, Hugo Pereira Da Costa
Re: hard-dep for Qt 4.8
Personally, I think making Qt4.8 mandatory for KDE4.8 without strong reasons to do so raises the barrier a bit too high for casual contributors willing to fix bugs. I personally find Qt harder to compile than kde and am quite reluctant to update, just to be able to still fix bugs in oxygen. Hugo On Thursday 12 January 2012, Marco Martin wrote: On Thursday 12 January 2012, Martin Gräßlin wrote: Our distributions will ship 4.8 together with 4.8 and this combination might just be untested by the maintainers. So I strongly suggest that we make Qt 4.8 a dependency for master just to get the code tested and fixed before distros use it. So unless there is a good reason why not to increase, I would like to see master depend on Qt 4.8. yep, that is the most valid reason imo, distribution will have 4.8, no matter what That's no valid reason. Distributions shipped kdepim 4.4 together with Qt 4.7. Still we never made Qt 4.7 a hard requirement for kdepim 4.4. If you want developers to switch to Qt 4.8 to get the KDE code better tested with this version of Qt then simply ask developers to do so. I see no good reason to force developers to do so. Regards, Ingo
Re: hard-dep for Qt 4.8
On 01/13/2012 12:12 AM, Hugo Pereira Da Costa wrote: Personally, I think making Qt4.8 mandatory for KDE4.8 without strong reasons to do so raises the barrier a bit too high for casual contributors willing to fix bugs. I personally find Qt harder to compile than kde and am quite reluctant to update, just to be able to still fix bugs in oxygen. or saying it differently: so far I've been quite happy with the fact that I could develop in kde while still relying on the Qt installation provided by my (one year old) linux distro, and not having to compile it myself ... Hugo On Thursday 12 January 2012, Marco Martin wrote: On Thursday 12 January 2012, Martin Gräßlin wrote: Our distributions will ship 4.8 together with 4.8 and this combination might just be untested by the maintainers. So I strongly suggest that we make Qt 4.8 a dependency for master just to get the code tested and fixed before distros use it. So unless there is a good reason why not to increase, I would like to see master depend on Qt 4.8. yep, that is the most valid reason imo, distribution will have 4.8, no matter what That's no valid reason. Distributions shipped kdepim 4.4 together with Qt 4.7. Still we never made Qt 4.7 a hard requirement for kdepim 4.4. If you want developers to switch to Qt 4.8 to get the KDE code better tested with this version of Qt then simply ask developers to do so. I see no good reason to force developers to do so. Regards, Ingo
Re: Review Request: fix the rect passed to kcapacitybar paint method
On Jan. 10, 2012, 6:08 p.m., Thomas Lübking wrote: kdeui/widgets/kcapacitybar.cpp, line 374 http://git.reviewboard.kde.org/r/103669/diff/2/?file=46380#file46380line374 rather ..., contentsRect()); Otherwise looks fine (i guess setCllipRegion() is overhead) Concerning setClipRegion() overhead, no clue. Concerning passing contentsRect() instead of rect(), well, QStyleOption::initFrom uses widget-rect() by default. Now, other widgets implementation might use contentsRect() when initializing their options. Testing, both work (I guess cause the margin is zero :)). - Hugo --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103669/#review9716 --- On Jan. 10, 2012, 5:06 p.m., Hugo Pereira Da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103669/ --- (Updated Jan. 10, 2012, 5:06 p.m.) Review request for kdelibs. Description --- fixes the rect passed to paint method so that it matches the widget rect and not the event (clip) rect, since the former is what's expected by widget styles. This addresses bug 290879. http://bugs.kde.org/show_bug.cgi?id=290879 Diffs - kdeui/widgets/kcapacitybar.cpp 6e63c3f Diff: http://git.reviewboard.kde.org/r/103669/diff/diff Testing --- Fixes bug above. No regression with other styles. Thanks, Hugo Pereira Da Costa
Re: Building kde-workspace with latest checkout
Adding kde-core-devel, which is better suited for these kind of emails.
Seems to me that trunk is actually not up to date for kdelibs, and that
accessor to
resizeMethodHint is missing from plasma/wallpaper.h / .cpp
See patch attached. (can someone fix/commit this ?)
At least it made things compile again for me.
Hugo
Hi guys,
I keep getting this error while trying to build kde-workspace module:
[ 57%] Building CXX object
plasma/generic/wallpapers/image/CMakeFiles/plasma_wallpaper_image.dir/backgroundlistmodel.o
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:
In member function ‘virtual void Image::save(KConfigGroup)’:
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:130:66:
error: ‘resizeMethodHint’ was not declared in this scope
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:
In member function ‘virtual QWidget*
Image::createConfigurationInterface(QWidget*)’:
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:152:51:
error: ‘resizeMethodHint’ was not declared in this scope
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:241:34:
error: ‘resizeMethodHint’ was not declared in this scope
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:251:60:
error: ‘resizeMethodHint’ was not declared in this scope
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:
In member function ‘void Image::positioningChanged(int)’:
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:630:47:
error: ‘resizeMethodHint’ was not declared in this scope
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:
In member function ‘void Image::renderWallpaper(const QString)’:
/home/ggorosito/kde/kdesrc/kde/kde-workspace/plasma/generic/wallpapers/image/image.cpp:783:44:
error: ‘resizeMethodHint’ was not declared in this scope
make[2]: ***
[plasma/generic/wallpapers/image/CMakeFiles/plasma_wallpaper_image.dir/image.o]
Error 1
Full log: http://pastebin.com/UnE2m3Am
Today I talked with Aaron about this, he told me to make sure that
kdelibs is up to date and is setted up to branch KDE/4.7.. and it is:
ggorosito@glaptop:~/kde/kdesrc/kde/kdelibs$ git config --get-all
remote.origin.url
http://anongit.kde.org/kdelibs.git
ggorosito@glaptop:~/kde/kdesrc/kde/kdelibs$ git branch
* KDE/4.7
master
Any ideas!?
###
# Gonzalo Gorosito
# Programador sysadmin
#
# http://www.tutorialesdebian.com - Tutoriales para debianeros,
scripts, info, notícias y mucho mas.
# http://www.ggorosito.com.ar - Website personal
###
Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe
diff --git a/plasma/wallpaper.cpp b/plasma/wallpaper.cpp
index 97ec7e8..16cf3a7 100644
--- a/plasma/wallpaper.cpp
+++ b/plasma/wallpaper.cpp
@@ -421,6 +421,9 @@ void Wallpaper::setResizeMethodHint(Wallpaper::ResizeMethod
resizeMethod)
emit renderHintsChanged();
}
+Wallpaper::ResizeMethod Wallpaper::resizeMethodHint( void ) const
+{ return d-lastResizeMethod; }
+
void Wallpaper::setTargetSizeHint(const QSizeF targetSize)
{
d-targetSize = targetSize;
diff --git a/plasma/wallpaper.h b/plasma/wallpaper.h
index e1e9891..3086517 100644
--- a/plasma/wallpaper.h
+++ b/plasma/wallpaper.h
@@ -416,12 +416,17 @@ class PLASMA_EXPORT Wallpaper : public QObject
/**
* This method is invoked by setUrls(KUrl::List)
* Can be Overriden by Plugins which want to support setting Image URLs
- * Will be changed to virtual method in libplasma2/KDE5
+ * Will be changed to virtual method in libplasma2/KDE5
* @since 4.7
*/
void addUrls(const KUrl::List urls);
protected:
+
+
+Wallpaper::ResizeMethod resizeMethodHint( void ) const;
+
+
/**
* This constructor is to be used with the plugin loading systems
* found in KPluginInfo and KService. The argument list is expected
Expert advice needed on problems with oxygen + alpha channel
Hello, I'm facing a bug with oxygen and KToolBar when compositing is active that I need expert advice on. The issue: when you detach (unlock and move out of the window) a KToolbar from a main window, oxygen gives it the translucent background flag, and uses it to draw nice beveled corner on the detached toolbar. The same *was* done for Dockwidgets, and it *used to* work in both cases. Sadly enough it does not work anymore (try it with e.g. toolbars in dolphin). You get a zillion of errors message like: X Error: BadMatch (invalid parameter attributes) 8 Major opcode: 62 (X_CopyArea) Resource id: 0x22027d8 and the toolbar contents is not drawn anymore. Nor does it work for Dockwidgets in which there is a KPart (okular or kate), as was reported in bug https://bugs.kde.org/show_bug.cgi?id=273848 To fix the bug, I found a workaround that unfortunately looks nice only when using KWin. I could do the same for KToolBars, but what annoys me is - it does work without the workaround for other QDockWidgets (with no kpart in there) - it does work without the workaround for QToolBar (try, e.g. in QGit, or Quassel) - it looks ugly when using kde applications in another DE. I believe it is somehow related to XmlGui, and would rather have this fixed there, than propagating ugly workarounds. However, I have no clue where to look, how to start. Advice ? Thanks in advance, Hugo
Re: Review Request: Fix logic with clear-button animation in klineedit (notably at first try) and address bug 268898.
On July 26, 2011, 10:46 p.m., Aleix Pol Gonzalez wrote: kdeui/widgets/klineedit.cpp, line 358 http://git.reviewboard.kde.org/r/102095/diff/1/?file=30032#file30032line358 Wouldn't it be better to put it this way? Just saying... d-clearButton-animateVisible(d-wideEnoughForClear text.length() 0); Nicolas Alvarez wrote: I think the original is clearer, to be honest. Hugo Pereira Da Costa wrote: I agree with Nicolas. I have nothing against putting boolean test inside the method call, *in principle*, but I believe it's convenient only if the boolean condition is short enough to write. Other than that, anyone willing to go for a ship it ? It was reported on bug 268898 that the patch actually works, and that no regression has been found so far (which is also my own experience) - Hugo --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102095/#review5129 --- On July 26, 2011, 9:54 p.m., Hugo Pereira Da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102095/ --- (Updated July 26, 2011, 9:54 p.m.) Review request for kdelibs. Summary --- Details: - fixes the somewhat incorrect logic in KLineEditButton::animateVisible - simplifies KLineEdit::updateClearButtonIcon consequently. This addresses bug 268898. http://bugs.kde.org/show_bug.cgi?id=268898 Diffs - kdeui/widgets/klineedit.cpp 8f1c8a4 kdeui/widgets/klineedit_p.h 95016bd Diff: http://git.reviewboard.kde.org/r/102095/diff Testing --- tested with klineedittest found in kdelibs/kdeui/tests, this with and without the patch attached to comment #1 of bug 268898, used to actually trigger the mentionned bug. Also tested with other klineEdit implementation such as Dolphin's location bar. Thanks, Hugo
Re: KMainWindow unit test crash, caused by recent Oxygen commit
I could reproduce the issue with both kxmlgui_unittest and
kmainwindow_unittest
Oxygen was the curlpit indeed. (there was a real flaw in the code,
thanks to Thomas for pointing it out).
Should be fixed with
http://commits.kde.org/kde-workspace/68dbf302f2cb7bb011bdbd52e8dda04902a1dae5
Sorry for the trouble,
Hugo
On 06/27/2011 04:46 PM, Frank Reininghaus wrote:
Hi,
I'm seeing a failure in kmainwindow_unittest:
http://my.cdash.org/testDetails.php?test=6516885build=203056
The test crashes with the backtrace below. Sometimes, also kxmlgui_unittest
crashes with that bt, but that's not 100% reproducible.
It seems that the failure is due to commit 2bc71422 in kde-workspace:
https://projects.kde.org/projects/kde/kdebase/kde-
workspace/repository/revisions/2bc71422e031879d21b1b65fe774d23f5dfd
I don't know KMainWindow and KXmlGui well, so I can't say if they do anything
wrong here, if the tests need to be fixed, or if Oxygen really is the culprit.
Best regards,
Frank
P.S.: The backtrace:
Program received signal SIGSEGV, Segmentation fault.
0x76664cea in QObject::installEventFilter (this=0x7fffcc80,
obj=0x6af770) at kernel/qobject.cpp:2061
2061if (d-threadData != obj-d_func()-threadData) {
(gdb) bt
#0 0x76664cea in QObject::installEventFilter (this=0x7fffcc80,
obj=0x6af770) at kernel/qobject.cpp:2061
#1 0x7fffef61045a in Oxygen::SplitterFactory::registerWidget
(this=0x696ce0, widget=0x7fffcc80)
at /home/kde-devel/kde/src/KDE/kde-
workspace/kstyles/oxygen/oxygensplitterproxy.cpp:59
#2 0x7fffef6124d6 in Oxygen::Style::polish (this=0x665b60,
widget=0x7fffcc80) at /home/kde-devel/kde/src/KDE/kde-
workspace/kstyles/oxygen/oxygenstyle.cpp:211
#3 0x75550fed in QWidget::event (this=0x7fffcc80,
event=0x7fffcb00) at kernel/qwidget.cpp:8364
#4 0x75a2b87e in QMainWindow::event (this=0x7fffcc80,
event=0x7fffcb00) at widgets/qmainwindow.cpp:1480
#5 0x779da24f in KMainWindow::event (this=0x7fffcc80,
ev=0x7fffcb00) at /home/kde-
devel/kde/src/KDE/kdelibs/kdeui/widgets/kmainwindow.cpp:1100
#6 0x754ea03e in QApplicationPrivate::notify_helper (this=0x62c9f0,
receiver=0x7fffcc80, e=0x7fffcb00) at kernel/qapplication.cpp:4477
#7 0x754e9d3e in QApplication::notify (this=0x7fffdab0,
receiver=0x7fffcc80, e=0x7fffcb00) at kernel/qapplication.cpp:4442
#8 0x7664aa33 in QCoreApplication::notifyInternal
(this=0x7fffdab0, receiver=0x7fffcc80, event=0x7fffcb00) at
kernel/qcoreapplication.cpp:787
#9 0x7664e765 in QCoreApplication::sendEvent
(receiver=0x7fffcc80, event=0x7fffcb00) at
../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:215
#10 0x75552a4c in QWidget::ensurePolished (this=0x7fffcc80) at
kernel/qwidget.cpp:9615
#11 0x7554f3df in QWidget::setVisible (this=0x7fffcc80,
visible=true) at kernel/qwidget.cpp:7630
#12 0x004087c2 in QWidget::show (this=0x7fffcc80) at /home/kde-
devel/qt/include/QtGui/../../src/gui/kernel/qwidget.h:487
#13 0x0040743a in KMainWindow_UnitTest::testNameWithSpecialChars
(this=0x7fffdaa0) at /home/kde-
devel/kde/src/KDE/kdelibs/kdeui/tests/kmainwindow_unittest.cpp:81
#14 0x004064c9 in KMainWindow_UnitTest::qt_metacall
(this=0x7fffdaa0, _c=QMetaObject::InvokeMetaMethod, _id=2,
_a=0x7fffce30)
at /home/kde-
devel/kde/build/KDE/kdelibs/kdeui/tests/kmainwindow_unittest.moc:86
#15 0x76652ba7 in QMetaObject::metacall (object=0x7fffdaa0,
cl=QMetaObject::InvokeMetaMethod, idx=6, argv=0x7fffce30) at
kernel/qmetaobject.cpp:237
#16 0x76655836 in QMetaMethod::invoke (this=0x7fffd310,
object=0x7fffdaa0, connectionType=Qt::DirectConnection, returnValue=...,
val0=..., val1=..., val2=...,
val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at
kernel/qmetaobject.cpp:1597
#17 0x76654ade in QMetaObject::invokeMethod (obj=0x7fffdaa0,
member=0x7242b0 testNameWithSpecialChars, type=Qt::DirectConnection,
ret=..., val0=..., val1=..., val2=
..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...)
at kernel/qmetaobject.cpp:1151
#18 0x77439544 in QMetaObject::invokeMethod (obj=0x7fffdaa0,
member=0x7242b0 testNameWithSpecialChars, type=Qt::DirectConnection,
val0=..., val1=..., val2=..., val3=
..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at
../../include/QtCore/../../src/corelib/kernel/qobjectdefs.h:410
#19 0x774372ec in QTest::qInvokeTestMethodDataEntry (slot=0x7242b0
testNameWithSpecialChars) at qtestcase.cpp:1277
#20 0x77437879 in QTest::qInvokeTestMethod (slotName=0x40a1b8
testNameWithSpecialChars(), data=0x0) at qtestcase.cpp:1385
#21 0x77437f15 in QTest::qInvokeTestMethods
(testObject=0x7fffdaa0) at qtestcase.cpp:1540
#22 0x774383a4 in QTest::qExec (testObject=0x7fffdaa0, argc=1,
Re: Help on .desktop files for oxygen
Some follow-up. I coded locally the necessary changes to have oxygen-settings included inside systemsettings, even when through changing the various pages into KCModules, and ... well ... it pretty much defeats the purpose (see screenshot at: http://simplest-image-hosting.net/jpg-0-plasma-desktopye3595) The result is that the advanced configuration is now more visible than the basic ones (located in Application Appearance and Workspace Appearance, for widget and decoration style), which is pretty much the opposite of what oxygen-settings was trying to achieve ... So unless someone has a better idea (or disagrees with the above), I guess I'll leave things unchanged (oxygen-settings as a non-advertised standalone application) for kde 4.7. Oppinions ? Comments ? Ideas ? (somehow I wish system-settings still had its advanced tab). Hugo Le 04/28/2011 04:08 PM, Hugo Pereira Da Costa a écrit : Le 04/28/2011 02:52 PM, Harald Sitter a écrit : On Thursday 28 April 2011 11:54:13 Hugo Pereira Da Costa wrote: Having all the configuration featurs of oxygen-settings in systemsettings is simply not an option (it has been discussed at length among oxygen devs). It does not have to be in the existing KCMs ;) (i.e. see what Ben wrote). All I am saying is, not having a settings UI appear in SystemSettings but in the menu is rather confusing from a user POV. Icon probably should be start-here-oxygen. mmm. I'm confused. There is no start-here-oxygen.png icon in themes, whereas there is an oxygen.png icon (well, in oxygen theme). But maybe I should rather ship the icon (and install) with the application ? Oh my bad, I was thinking of start-here.png, which is not suitable. Basically I think there are 2 options here: a) ship your own icon b) use oxygen.png Latter is not very advisable iff you choose to have the app listed in the menu, as the implementation of the menu (think gnome-menu) is responsible for icon lookup, which of course then fails if the used icon set is not oxygen. For a KCM/SystemSettings entry that would be a none-issue as IIRC KIconLoader always tries to look for icons in oxygen as second to last option. So, if your desktop file is only used within a KApplication (such as SystemSettings) you can use any icon from the oxygen set knowing that it will always be displayed, if your desktop file can be used by non-KApps you will need to ship an icon for the application and install it to the hicolor theme. Also see [1]. General note about the Settings category: I think in a default setup only SystemSettings is listed in Settings (which will not make it show up in the menu at all), adding another application entry to the Settings category will make it show up in the menu... a menu with 2 entries (of which one is already listed by default in kickoff favorites *and* the kickoff computer tab) seems like bad default appearance to me *shrug* Fine with me (and I agree about your concern). Any better Categories suggestion ? (or alternatively, where do I find the list of available Categories) ? [3] contains a list of all registred cateogires. [2] is the main spec on desktop files, it points to other relevant specs as needed (which includes [1] and [3] of course ;)). I do still believe that integrating the app to show up within systemsettings rather than the menu would be the way to go though :) Yep. You, and Berto and Ben got me convinced. I'll do that. Thanks a bunch for the feedback and advice. [1] http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec- latest.html#install_icons [2] http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec- latest.html [3] http://standards.freedesktop.org/menu-spec/menu-spec-1.0.html#category- registry
Re: QPixmap::handle(): Pixmap is not an X11 class pixmap
Le 05/02/2011 04:33 PM, Christoph Feck a écrit : On Monday 02 May 2011 16:15:42 Wolfgang Rohdewald wrote: since updating to kubuntu 11.04 I am getting this message while KApplication is created, but only if I use raster or opengl as graphics system: kpat --graphicssystem raster QPixmap::handle(): Pixmap is not an X11 class pixmap this seems to happen with all KDE programs. Any idea? kdelibs 4.6.2 qt 4.7.2 What's the widget style you are using ? I remember a recent discussion in #qt-labs about this, and I think it has already been fixed in Qt 4.8 branch. The issue, if I understood the discussion correctly, is that even while you are setting a non X11 backend, it is still possible for applications to create an X11 native QPixmap via the fromX11Pixmap() function, but the checks in Qt forgot that case. Those checks were added to make sure applications don't try to access the underlying X11 pixmap so that changing the default backend to raster (or opengl) is possible for a future version of Qt. In other words, you likely don't need to worry about that warning, unless your application breaks because of it. On the other hand, I am highly interested why kpat tries to use X11 pixmaps at all. Adding kde-games-devel. Christoph Feck (kdepepo)
Re: Review Request: Add KMessageWidget, an alternative to KMessageBox
On May 1, 2011, 10:14 a.m., Hugo Pereira Da Costa wrote: Hello Aurelien, Though I have nothing to say against the current design of your kmessagebox widget, something I miss in your implementation is the possibility for a widget style to override/replace your rendering. I can indeed imagine some styles (not oxygen though) for which your design might not integrate that well, notably skulpture for which all widgets have square corners, that won't match your nice rounded one. There is a mechanism to register custom style primitives that widget styles may or may not support, and for your custom widget to test whether the style supports the new primitive or not, allowing it to fallback to its own rendering, when not supported. I think it would be nice for your widget (and other 'custom' widgets that you'd plan to implement) to use this mechanism, for better future integration. kdeui/kdelibs/kcapacitybar implements this mechanism (the widget appears in the statusbar of dolphin to give the available space on a given device), and oxygen (as well as bespin) actually uses it to render the capacity bar as a regular progress bar, instead of the default implementation (which you can see by using any style but oxygen, or bespin, e.g. plastique). Tell me that you think, and also if needed I can help implementing if you agree with the above but are short of time to work on it. Cheers, Hugo Aaron J. Seigo wrote: note that this widget exposes a bug in oxygen style where when fading between two strings in a QLabel, the background is not updated. so when the text changes as well as the background color, the old background color remains. this is easily seen if you run kmessagewidgetdemo from kdeexamples/kmessagewidget/ and switch between the Information and Positive messages. Hugo Pereira Da Costa wrote: Thanks Aaron. Its now fixed (in a temporary way) in trunk. Note: I've been planing to re-implement the label text transition for a looong time now, in a much more robust way, to avoid these kind of issues -that occur elsewhere in kde- ... But have not found time so far :( Aurélien Gâteau wrote: @Hugo: Interesting, I didn't know about this mechanism. I will be quite busy for the upcoming two weeks so if you feel like implementing it, feel free to (btw: I don't get the nice capacity bar in Dolphin anymore, just a progress bar, is it normal?) @Aaron: I was able to reproduce this bug for a while when the content widget was hidden away by moving it (0, content-height()). Strangely enough changing the code to move it to (0, -content-height()) fixed it for me. Aurélien: btw: I don't get the nice capacity bar in Dolphin anymore, just a progress bar, is it normal? Yes its normal. Its precisely what the styling mechanism allowed us to do. We (that is: Nuno, I, and some others) felt that the default implementation provided by Dolphin, although nice, had some polishing issues, and did not integrate well with Oxygen, so we re-implemented it in oxygen, and after some more discussion with Nuno, we figured that rather than implementing a new UI element with different rendering for just this purpose, we'd better just reuse the progress bar element, which which achieves just the same goal. - Hugo --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/#review3002 --- On April 30, 2011, 1:10 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/ --- (Updated April 30, 2011, 1:10 p.m.) Review request for kdelibs. Summary --- KMessageWidget is a new widget which can be considered as a less intrusive alternative for KMessageBox. It was designed during KDE UX sprint 2011 ( http://community.kde.org/Sprints/UX2011/KMessageWidget ). The class documentation should make it clear how and when it can be used. This widget could be used by: - Browsers to implement their remember password widgets (Konqueror, Rekonq...) - Forms to provide feedback on validation errors - Bluedevil KCM to replace its custom No Bluetooth adapter have been found message widget - Nepomuk/Strigi KCM to indicate status of their services - Gwenview to replace its custom save bar - ... I still have a few additions in mind for the API but it is already usable and since we are freezing I think it can be merged in master in its current state. I assume it will still be possible to extend the API a bit before kdelibs 4.7 freezes for good. I also wrote an example program in the kdeexample repository ( https
Re: Review Request: Add KMessageWidget, an alternative to KMessageBox
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/#review3003 --- Also, I wanted to add that Gtk already has a widget similar to your idea, called GtkInfoBar, and which is rendered by oxygen-gtk like in the screenshot below: http://simplest-image-hosting.net/jpg-0-plasma-desktopou9644 Now I must say it is not widely used in gtk land. - Hugo On April 30, 2011, 1:10 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/ --- (Updated April 30, 2011, 1:10 p.m.) Review request for kdelibs. Summary --- KMessageWidget is a new widget which can be considered as a less intrusive alternative for KMessageBox. It was designed during KDE UX sprint 2011 ( http://community.kde.org/Sprints/UX2011/KMessageWidget ). The class documentation should make it clear how and when it can be used. This widget could be used by: - Browsers to implement their remember password widgets (Konqueror, Rekonq...) - Forms to provide feedback on validation errors - Bluedevil KCM to replace its custom No Bluetooth adapter have been found message widget - Nepomuk/Strigi KCM to indicate status of their services - Gwenview to replace its custom save bar - ... I still have a few additions in mind for the API but it is already usable and since we are freezing I think it can be merged in master in its current state. I assume it will still be possible to extend the API a bit before kdelibs 4.7 freezes for good. I also wrote an example program in the kdeexample repository ( https://projects.kde.org/projects/kde/kdeexamples/repository/show?rev=agateau%2Fkmessagewidget ), though I am wondering whether it shouldn't be moved in kdeui/tests as it is more a manual test program than an example. Diffs - kdeui/CMakeLists.txt d1873d154f4dde92c29f2e6dab1be70d49ddb55e kdeui/widgets/kmessagewidget.h PRE-CREATION kdeui/widgets/kmessagewidget.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/101249/diff Testing --- Screenshots --- Montage from kmessagewidgetdemo http://git.reviewboard.kde.org/r/101249/s/141/ Thanks, Aurélien
Re: Review Request: Add KMessageWidget, an alternative to KMessageBox
On May 1, 2011, 10:14 a.m., Hugo Pereira Da Costa wrote: Hello Aurelien, Though I have nothing to say against the current design of your kmessagebox widget, something I miss in your implementation is the possibility for a widget style to override/replace your rendering. I can indeed imagine some styles (not oxygen though) for which your design might not integrate that well, notably skulpture for which all widgets have square corners, that won't match your nice rounded one. There is a mechanism to register custom style primitives that widget styles may or may not support, and for your custom widget to test whether the style supports the new primitive or not, allowing it to fallback to its own rendering, when not supported. I think it would be nice for your widget (and other 'custom' widgets that you'd plan to implement) to use this mechanism, for better future integration. kdeui/kdelibs/kcapacitybar implements this mechanism (the widget appears in the statusbar of dolphin to give the available space on a given device), and oxygen (as well as bespin) actually uses it to render the capacity bar as a regular progress bar, instead of the default implementation (which you can see by using any style but oxygen, or bespin, e.g. plastique). Tell me that you think, and also if needed I can help implementing if you agree with the above but are short of time to work on it. Cheers, Hugo Aaron J. Seigo wrote: note that this widget exposes a bug in oxygen style where when fading between two strings in a QLabel, the background is not updated. so when the text changes as well as the background color, the old background color remains. this is easily seen if you run kmessagewidgetdemo from kdeexamples/kmessagewidget/ and switch between the Information and Positive messages. Thanks Aaron. Its now fixed (in a temporary way) in trunk. Note: I've been planing to re-implement the label text transition for a looong time now, in a much more robust way, to avoid these kind of issues -that occur elsewhere in kde- ... But have not found time so far :( - Hugo --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/#review3002 --- On April 30, 2011, 1:10 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101249/ --- (Updated April 30, 2011, 1:10 p.m.) Review request for kdelibs. Summary --- KMessageWidget is a new widget which can be considered as a less intrusive alternative for KMessageBox. It was designed during KDE UX sprint 2011 ( http://community.kde.org/Sprints/UX2011/KMessageWidget ). The class documentation should make it clear how and when it can be used. This widget could be used by: - Browsers to implement their remember password widgets (Konqueror, Rekonq...) - Forms to provide feedback on validation errors - Bluedevil KCM to replace its custom No Bluetooth adapter have been found message widget - Nepomuk/Strigi KCM to indicate status of their services - Gwenview to replace its custom save bar - ... I still have a few additions in mind for the API but it is already usable and since we are freezing I think it can be merged in master in its current state. I assume it will still be possible to extend the API a bit before kdelibs 4.7 freezes for good. I also wrote an example program in the kdeexample repository ( https://projects.kde.org/projects/kde/kdeexamples/repository/show?rev=agateau%2Fkmessagewidget ), though I am wondering whether it shouldn't be moved in kdeui/tests as it is more a manual test program than an example. Diffs - kdeui/CMakeLists.txt d1873d154f4dde92c29f2e6dab1be70d49ddb55e kdeui/widgets/kmessagewidget.h PRE-CREATION kdeui/widgets/kmessagewidget.cpp PRE-CREATION Diff: http://git.reviewboard.kde.org/r/101249/diff Testing --- Screenshots --- Montage from kmessagewidgetdemo http://git.reviewboard.kde.org/r/101249/s/141/ Thanks, Aurélien
Help on .desktop files for oxygen
Hi, For a couple of releases, oxygen widget and decoration style comes with an advanced configuration tool, called oxygen-settings [1], and accessible (so far) only from terminal (or krunner). For the record: there are already 'basic' configuration tools available at the usual places in system-settings for the decoration and the style with a much more limited set of configuration options, the idea being not to expose newcommers and casual users to too many options of which they simply do not care (such as the duration of the animations). On the other hand, another advantage that I see for oxygen-settings is that it regroups the widget style and decoration options at the same place. For 4.7 I'd like to make this advanced tool available via kde' launcher menu. Good idea ? Bad idea ? Objections ? Also I guess need a .desktop file for that. Never wrote such a thing before, so some other expert pair of eyes would help. The content I came up with by looking at various other desktop files is attached. Advice welcome, Hugo [1] Screenshot: http://simplest-image-hosting.net/jpg-0-plasma-desktopat4300 [Desktop Entry] Type=Application Exec=oxygen-settings -caption %c %i Icon=oxygen GenericName=Oxygen Advanced Configuration Terminal=false Name=Oxygen Advanced Configuration Categories=Qt;KDE;Settings; X-KDE-ServiceTypes=DBUS/InstantMessenger X-DBUS-StartupType=Unique X-DBUS-ServiceName=org.kde.konversation
Re: Help on .desktop files for oxygen
Harald, David, Thanks for the feedback. Obviously too much careless copy/paste on my side. Harald: Having all the configuration featurs of oxygen-settings in systemsettings is simply not an option (it has been discussed at length among oxygen devs). So the available solutions are either: 1/ no advanced options GUI at all, and we instruct power users on how to modify oxygenrc manually at will. This is what we started with, and Nuno has a blog post about it. 2/ keep oxygen-settings to edit oxygenrc for you, but don't make it generally available, instructing people that it is here and can be run via command line on a per-user, per-bug, per-whatever basis. This is the current solution (and I've read complains about it on various forums). 3/ make it available via .desktop file, which is what I suggest now. Also, we're naturally ready to discuss the possibility of moving some (one or two, no more) of the advanced options to the basic options. More detailed questions follow. X-KDE-ServiceTypes=DBUS/InstantMessenger X-DBUS-StartupType=Unique X-DBUS-ServiceName=org.kde.konversation are probably not supposed to be there ;) True. I guess now you know which desktop file I use to make mine ;) GenericName and Name ought not be the same... Generic could be something like Advanced Widget and Window Decoration Settings. Clear. Thanks ! Icon probably should be start-here-oxygen. mmm. I'm confused. There is no start-here-oxygen.png icon in themes, whereas there is an oxygen.png icon (well, in oxygen theme). But maybe I should rather ship the icon (and install) with the application ? General note about the Settings category: I think in a default setup only SystemSettings is listed in Settings (which will not make it show up in the menu at all), adding another application entry to the Settings category will make it show up in the menu... a menu with 2 entries (of which one is already listed by default in kickoff favorites *and* the kickoff computer tab) seems like bad default appearance to me *shrug* Fine with me (and I agree about your concern). Any better Categories suggestion ? (or alternatively, where do I find the list of available Categories) ? Even a KDE;Application;Misc would suit me. I want the application to appear in the menu, but do not care about how visible it is (and it should definitely not be as visible as system-settings). Hugo
Re: OpenPrinting Summit - Print Dialog and Colour Management
On Wednesday 16 March 2011 19:55:48 todd rme wrote: On Wed, Mar 16, 2011 at 1:44 PM, John Layt johnl...@googlemail.com wrote: Hi, I'll be attending the OpenPrinting Summit [1] to discuss how to complete the Common Printing Dialog [2] and integrate it into KDE and Qt. I'm looking for any feedback people may have about the CPD, and any questions you want me to ask while I'm there. The CPD is a common print dialog implementation in Qt and Gtk that gets called via DBus. The dialog includes a preview image, more user-friendly options, better driver integration, and settings management. Most programs will simply print their entire document to PDF and pass the file to the CPD and not have any more involvement, but there is a callback mode for longer multi-page documents to use. We will obviously need new API to wrap the new workflow and functionality, which I think should be a stand-alone Qt-based library until such time as Qt can be convinced to integrate it natively. This library will also need to provide fallback functionality to use the native Qt print dialog for when the CPD is not present, such as on Windows and OSX. Otherwise apps will need to code for two different print paths depending on the platform which is not desirable. You can have a play with the dialog in its current rough and ugly state at [3]. OpenPrinting have a GSoC project to finish the dialog this year which I've promised to help advertise. It would be great if we could find a good Qt gui hacker to make it really shine. Also on the agenda is integrated end-to-end Colour Management possibly using colord [4], something I know absolutly nothing about, so any feedback or suggestions people have on that will be very welcome. For starters the dependencies for colord are glib and policykit. The OpenPrinting Summit is part of the Linux Foundation Collaboration Summit immediately following Camp KDE, so if you're attending and want to be involved in either of these areas please let me know, backup on the more technical aspects would be welcome. Cheers! John. Are there screenshots of the most recent version available? Just made these: http://simplest-image-hosting.net/png-0-cpd Left: Qt Right: gtk Hugo -Todd
Re: OpenPrinting Summit - Print Dialog and Colour Management
On Wednesday 16 March 2011 19:55:48 todd rme wrote: On Wed, Mar 16, 2011 at 1:44 PM, John Layt johnl...@googlemail.com wrote: Hi, I'll be attending the OpenPrinting Summit [1] to discuss how to complete the Common Printing Dialog [2] and integrate it into KDE and Qt. I'm looking for any feedback people may have about the CPD, and any questions you want me to ask while I'm there. The CPD is a common print dialog implementation in Qt and Gtk that gets called via DBus. The dialog includes a preview image, more user-friendly options, better driver integration, and settings management. Most programs will simply print their entire document to PDF and pass the file to the CPD and not have any more involvement, but there is a callback mode for longer multi-page documents to use. We will obviously need new API to wrap the new workflow and functionality, which I think should be a stand-alone Qt-based library until such time as Qt can be convinced to integrate it natively. This library will also need to provide fallback functionality to use the native Qt print dialog for when the CPD is not present, such as on Windows and OSX. Otherwise apps will need to code for two different print paths depending on the platform which is not desirable. You can have a play with the dialog in its current rough and ugly state at [3]. OpenPrinting have a GSoC project to finish the dialog this year which I've promised to help advertise. It would be great if we could find a good Qt gui hacker to make it really shine. Also on the agenda is integrated end-to-end Colour Management possibly using colord [4], something I know absolutly nothing about, so any feedback or suggestions people have on that will be very welcome. For starters the dependencies for colord are glib and policykit. The OpenPrinting Summit is part of the Linux Foundation Collaboration Summit immediately following Camp KDE, so if you're attending and want to be involved in either of these areas please let me know, backup on the more technical aspects would be welcome. Cheers! John. Are there screenshots of the most recent version available? Just made these: http://simplest-image-hosting.net/png-0-cpd Left: Qt Right: gtk Hugo -Todd
Re: Merge or Cherry-Pick?
On Thursday 03 February 2011 13:04:18 Johannes Sixt wrote: Am 2/3/2011 12:15, schrieb Hugo Pereira Da Costa: So I git cloned KDE/4.6 into some local branch (git checkout KDE/4.6; git checkout -b toolbuttons), then fix, then test. Now I want to merge to the KDE/4.6 branch; thats easy. Then I want to merge to master (or to some local branch cloned from master and then from there to master). As already announced in this thread, this results in tons of conflicts (notably many .desktop files). Before anybody begins to work in this way, someone with sufficient knowlege must introduce the first real merge of the 4.6 branch into the master branch. The conflicts must be resolved; or it is possible to punt by using -s ours. As long as this merge did not happen, anyone who wants to use the merge workflow is at a loss, unfortunately. I do: git merge -X ours toolbuttons Is that the correct action to take ? No. Perhaps you meant 'git merge -s ours toolbuttons', No, I really meant git merge -X ours toolbuttons which I believe, is equivalent to git merge -s recursive -X ours toolbuttons and which, according to the documentation (and confirmed by my test), would pick my changes if there is no conflict, and keep the master branch source, in case of conflict. Which, in my case, is exactly what I need (since there is no conflict with oxygenstyle.cpp). Anyway, I agree with you, I'm not that confident with the merge part itself, and will follow your advice, by just waiting. My understanding is also that people should refrain from cherry-picking before this original merge happens, cause that would only make it more painful. Correct ? but even that would be wrong because it would ignore your changes that you made in the 4.6 branch - but this defeats the purpose of the merge. Or should I give up and cherry-pick ? (I'd really like not to). My recommendation: Keep the fix in 4.6 only for the moment. Just wait until the initial merge has happened - and lets hope (HOPE BIG TIME) that the person doing that merge knows what s/he is doing! Hannes
RESOLVED vs CLOSED
Hello all, sorry for this newbye question. I was pointed out that RESOLVED bugs on bugs.kde.org can be further tagged as CLOSED. ... which I did not know. Right now, there are 432 bug reports related to oxygen which are RESOLVED, but not CLOSED. What exactly is the policy on tagging them as CLOSED, with respect to RESOLVED ? (so far I've been using the second pretty much in place of the first) Should I go through all of them and CLOSE them ? Or just the ones that are only FIXED, or INVALID, or UPSTREAM ? Thx, Hugo
Re: system-settings color change vs Qt (non-kde) apps
On Tuesday, January 04, 2011 09:05:13 pm Albert Astals Cid wrote: A Dimarts, 4 de gener de 2011, Hugo Pereira Da Costa va escriure: Hello all, Here, when I change the color scheme in kde's system-settings, the change is propagated properly to KDE apps, but not to Qt (non-kde) applications (like, for instance QGit, and other applications of mine). I believe this is due to differences between QApplication and KApplication (correct ?) Experimenting locally, I found out that if in oxygen's style I connect to kglobalsettings changes, and explicitely call KGlobal::config()-reparseConfiguration(); there, then the above gets fixed. I believe however that this is not quite acceptable, since obviously reparseConfiguration would be called twice for kde apps (correct ?). So. Does anyone have advice/insight on what we can do about it ? e.g.: - leave the situation as it is now ? (meaning that whenever you change color scheme you have to restart every Qt app you have, to get the right color changes, which I believe is unfortunate. - hack oxygen (in a way similar/better than the above, if possible) - leave that upstream to Qt (?) I'd say this should be added to kdebase/workspace/qguiplatformplugin_kde Sounds promising, I'll have a look and will post review request if I come up with something. Thanks for the info ! Hugo Albert Thanks in advance, Hugo
