> On Ago. 12, 2014, 11:19 a.m., Christian Esken wrote: > > For me it looks fine, with some questions: > > 1) Is this completely decoupled from KMix? I do not see a "open mixer" to > > open the main application. > > 2) Is it supposed to be integrated in KMix > > 3) In general, the question is how to progress from here. Diego, do you > > want KMix integration, or a standalone app? Have any ideas or plans to get > > it in KDE? > > Martin Klapetek wrote: > Note that new work on QML based KMix has started, however this will be > Plasma5 only --> http://apachelog.wordpress.com/2014/08/11/volume/ > > Diego Casella wrote: > I'll recap everything, hopefully once for all.. > This applet is meant to show the availabe mixers (with the option to show > all of them, or just the Master one), and modify/mute/unmute them. That's all. > If you need to change Master channel or in general configure KMix, you > need to run the "old" KMix application. That's why, in the QML applet, I > added a button named "Mixer setup": you click it, and the "old" KMix should > appear (in my opinion QML applets have be minimal and simple, so everything > else must be done in a regular QtWidget based application). > Anyway back on the topic: I said "should" because at the moment KMix does > not appear, complaining that: > > "QDBusConnection: session D-Bus connection created before > QCoreApplication. Application may misbehave." > > This is the reason why I mentioned, in the very first description of this > review request, that KMix needs to be patched :) > As about "how to progess from here", I've mentioned some ideas in the > beginning: > > * patch Plasma QML bindings to provide mouse wheel events, so I can > intercept scroll events in the panel and update volume levels accordingly, > providing the same functionality the current KMix tray icon does; > * patch Plasma QML slider to provide mouse wheel support (don't know if > this has been already fixed in Plasma5); > * patch KMix itself. It should run somewhat "daemonized" and, when QML > applet calls "kmix" the regular, QtWidget-based application, will pop up. > Since in the sourcecode I've seen references to "kmixd", which i think it > stands for "KMix daemon", this may be simple to implement: the daemon > provides the low-level connection to the hardware, and this applet and KMix > itself connect to the daemon (via DataEngine/DBus mechanism) to communicate > with it. > > > That being said: > * I've been trying to update this applet in the last couple of years, but > * since KDE4/Plasma is now in maintenance mode (aka no new featured will > be added), those patches won't happen anymore; > * and since someone is already on the process to "reinvent" the wheel; > > I'm kinda sad/tired of how things went so far. The applet is pretty > functional (I use it on a daily basis), and it could work fine in Plasma5 > too. As far as I'm concerned, at this point I don't see any reason to keep > this review open. If Harald wants to use any portion of this code, I'm 100% > OK with that. > > Christian Esken wrote: > Diego, I am not sure what went so wrong here. I am defintely also not > happy how things went on. What makes me most sad is, that you have a working > QML applet that just needs really minor integration work. All these minimal > issues like "scrollwheel not supported" (1) should really not stop us from > giving the users the option. If they must have scrollwheel or media buttons, > then they can use classic KMix dock. If they want a nicer integrated applet, > they can use your applet (2). It can be a simple option in KMix's > configuration dialog (Dock: Classic, Plasma, Off) instaed of (Dock: On, Off). > > Sigh :-| > > > Whatever your decision is, I can understand you Diego. If you want to > integrate it would make me happy (see some technical tips below). If you do > not want it or you cannot (essential missing plasma features that you do not > want to add yourselves), then it is possibly better to close this review > request. > > --- Technical notes ----------- > > About daemonizing KMix: That exists since forever: Simply disable "dock > into systray" and close window, and KMix will vanish forever. The "KMix will > show its GUI at every login" behavior/issue is gone since the support of > hotplugging. To show (I cannot remember that you ever mentioned problems with > that before) just do a DBUS call "show" on the Window (a standard for all Qt > windows). If you want to try that, please head along. If > > Side note (as you mentioned it): There is also a real daemon (technically > a kded module), but KMixD daemon has no GUI, and proviees just a Mixer DBUS > interface. > > ---- > (1) Some ramblings about QML/Plasma: Things seem to progress sometimes > slowly or not at all in QML/Plasma. Though much has happened Plasma/QML seems > to still lack features. Initially there were no sliders at all in Plasma, and > I knew it would be a very rocky way to get all the features in, like to add > missing stuff like "mousewheel over slider". Then I made a clear decision to > keep my foot out of QML completely. > > (2) I actually still do not understand if integration can now be done or > not. There was this mysterious comment "it would be pretty trivial to add a > check in the systemtray [...]" that sounds like somebody needs to take > responsibilty and add that. > > Aaron J. Seigo wrote: > <disclaimer>I am have not been involved with Plasma for almost a year, > but I keep getting CC'd on the comments here. ;)</disclaimer> > > Firstly, Christian is the maintainer of KMix. It is ultimately his > decision if this goes into kmix or not. Plasma developers can and should > provide feedback because that is where the experience and expertise around > QML in a Plasma workspace exists, but it is ultimately Christian's decision. > If the Plasma team has some wishes in how KMix goes forward, they should > coordinate those through Christian. This is basic common sense and courtesy > for KDE's culture of development. So if Christian is happy with it, he should > be able to push this into the kmix repository, end of story. > > Wheel events: they are well supported by QML2. There is *no reason* that > they are not made available here. The MouseArea item gets wheel events; if > anything what is called for is a "acceptWheelEvents" property on the iconic > version of applets and hook those up to some signals for applets to get > access to. For all the discussion about it, someone could have instead put > such a patch in place. > > Christian: as for your ramblings, they are unecessary and innacurate. I > get that you would have liked to see this go faster and as the maintainer of > kmix I totally understand that. But adding complaints about another project > doesn't move anything further, it just increases the chances of people > deciding not to work with each other. To address the content of your > concerns: there have been sliders in Plasma pre- and post-QML for just about > forever. There may have been short period of time when the QML widgets were > first being made that there wasn't a QML slider, but that certainly wasn't a > long period. Also, adding mousewheel over slider is a trivial patch if it > isn't already there. (I haven't tested the QML2 slider for this at the time > of writing, so it may already be there.) > > "There was this mysterious comment "it would be pretty trivial to add a > check in the systemtray [...]" that sounds like somebody needs to take > responsibilty and add that." > > Let me take the "mystery" out of it: in Plasma 4 it had not yet been > decided how precisely to handle this. It was a "nice to have" feature and > many other things were still on the "must be done" list. In Plasma 5 there is > now an implementation: > http://vizzzion.org/blog/2014/02/dbus-activated-systemtray-plasmoids/ > > So KMix should just need to have a DBus service it registers and the kmix > applet should magically appear if it exists. What isn't there, at least > afaict, is a way for kmix to know if the widget has appeared (and if not, > show its old KSystemTrayIcon based UI). This could be done with another dbus > service on the applet side that kmix watches for, but even then ... the more > fundamental question for kmix looms: > > Is KMix meant to be a tightly integrated part of a Plasma workspace, or > is it intended to be used in any "random" desktop environment? > > Personally, I think there is a lot to be gained both in terms of user > experience and simplicity in development for kmix to be Plasma-focused, and I > don't think there are nearly as many users of KMix outside of Plasma as there > are that use KMix with Plasma (I would expect it to be a "rounding error" > type number). But that's your call, Christian. > > If KMix does focus on Plasma, then a QML plasmoid with DBus activation as > described in Sebas' blog entry above is all that is needed and this could go > into kmix tomorrow at Christian's descrection. > > Diego Casella wrote: > @Christian and @Aaron: thanks for your replies :) > > This morning I've modified a little the sourcecode in order to call > "qdbus org.kde.kmix /kmix/KMixWindow org.qtproject.Qt.QWidget.show", disabled > the "old" KMix tray icon, and it works. It's kinda clumsy, but it works :) > Regular KMix widget gets displayed when the corresponding button is > clicked, and it won't appear at login anymore. These are already great news > for me :) > > Now, I think we have some options here: > * ship this applet _as is_ in KDE4, it is functioning afterall; > * port and/extend/improve this applet to get the most out of > KDE5/Plasma/QML2; > > I'm ok with both of these, now you guys tell me what do you think.
Could it have a standalone, maybe extragear release for KDE4? then focus can be shifted to the plasma5 port - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112208/#review64369 ----------------------------------------------------------- On Mag. 4, 2014, 9:46 p.m., Diego Casella wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/112208/ > ----------------------------------------------------------- > > (Updated Mag. 4, 2014, 9:46 p.m.) > > > Review request for Plasma, Aaron J. Seigo, Christian Esken, Marco Martin, and > Igor Poboiko. > > > Repository: kmix > > > Description > ------- > > KMix qml applet. > As you can see from the screenshot, the applet is pretty much functional: you > can display all the controls available, change its orientation, and decide to > whether show all of them or just the Master Control, and refresh its status > when new controls are added/removed/updated (such as Amarok current playing > track). See screenshots below :) > Differences from the old kmix tray: > * no media player controls ( I never investigated how to get them, but > honestly opening the audio applet to change/skip/pause audio track makes > little sense to me ... if anyone wants this feature back, don't be shy and > step in); > * the button used to select which Mixers are visible has been changed to open > Phonon kcm page: since visible mixers are already configurable from KMix app, > having a button to show KMix *and* a button to modify Mixers visibilty made > little sense here too, so I preferred to give more visibility to Phonon kcm; > > Known issues: > * there is still no way to get notified of mouse wheel events over the > popupIcon, so it is not possible to scroll over to increase/decrease the > master control volume; > * no scroll events over the sliders too; > * if you want to use the applet you most likely will disable KMix tray icon > but, if you do so, KMix will show its GUI at every login and you have to > close it manually. This requires KMix to be patched. Furthermore, if you > click "KMix Setup" button, KMix window will not restored anymore: this needs > to be pathed as well. > * resize doesn't work properly. > > > Diffs > ----- > > plasma/kmix-applet-qml/contents/ui/HorizontalControl.qml 7e87c8e > plasma/kmix-applet-qml/contents/ui/HorizontalMixerListDelegate.qml 26f2968 > plasma/kmix-applet-qml/contents/ui/MixersList.qml 66bda73 > plasma/kmix-applet-qml/contents/ui/VerticalControl.qml 1702be7 > plasma/kmix-applet-qml/contents/ui/VerticalMixerListDelegate.qml bab9ac6 > plasma/kmix-applet-qml/contents/ui/kmixapplet.qml eecb91c > > Diff: https://git.reviewboard.kde.org/r/112208/diff/ > > > Testing > ------- > > Tested against master and works fine. > > > File Attachments > ---------------- > > Default look > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet.png > Menu Actions > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet1.png > Applet Config Options > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet2.png > Vertical Control > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/22/kmix_applet3.png > ToolButton label and Config page after updates > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/24/kmix_applet5.png > Control Icon and Label left aligned > > https://git.reviewboard.kde.org/media/uploaded/files/2013/08/27/kmix_applet6.png > Kmix, horizontal view > > https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/9d6b0ca4-5a75-45cc-ab8e-61b13d4079e6__kmix_horizontal_new.png > Kmix applet, vertical view > > https://git.reviewboard.kde.org/media/uploaded/files/2014/05/04/7efb308a-c306-47c2-883f-64d1f32db5b5__kmix_vertical_new.png > > > Thanks, > > Diego Casella > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
