D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. Need to add dependency kf5kdelibs4support-devel in CMakeLists.txt Without this dependency, the assembly crashes with an error. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: yurchor, victorr,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. @victorr @yurchor see D23783 REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: yurchor, victorr, #vdg, GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

yurchor added a comment. In D22468#526517 , @romangg wrote: > In D22468#526465 , @ngraham wrote: > > > Like so: > > `xi18n("@info", "the first 100 charactersthe next 100 charactersand so on");`

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. F7323570: now_qml.patch F7323571: Screenshot_20190906_111728.jpeg REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc:

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#526465 , @ngraham wrote: > Like so: > `xi18n("@info", "the first 100 charactersthe next 100 charactersand so on");` I meant the 100 chars line limit of the code source file. REPOSITORY R104

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. In D22468#526448 , @romangg wrote: > In D22468#526447 , @victorr wrote: > > > I collected another package with a new patch.F7322491: qml.patch

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. I don’t understand what package is missing in CMakeLists.txt I get an error during assembly DEBUG: Warning: Could not locate service type file kservicetypes5/kcmodule.desktop, tried ("/builddir/.local/share", "/usr/local/share", "/usr/share") and

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#526447 , @victorr wrote: > I collected another package with a new patch.F7322491: qml.patch > > F7322492: Screenshot_20190906_002723.jpeg

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. I collected another package with a new patch.F7322491: qml.patch F7322492: Screenshot_20190906_002723.jpeg REPOSITORY R104 KScreen REVISION DETAIL

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#526445 , @victorr wrote: > The CMakeLists.txt file lacks the packages needed for the build. > Installed many additional kf5 and plasa packages for assembly, > since I was too lazy to figure out which

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. The CMakeLists.txt file lacks the packages needed for the build. Installed many additional kf5 and plasa packages for assembly, since I was too lazy to figure out which package is missing. REPOSITORY R104 KScreen REVISION DETAIL

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#526341 , @victorr wrote: > If you apply the attached file, the lines are added to the localization file, and the translation works.F7322054: messages2.patch > >

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. If you apply the attached file, the lines are added to the localization file, and the translation works.F7322054: messages2.patch F7322055: skreen3.jpeg Attached a picture

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#526323 , @victorr wrote: > Are you talking about this?F7321902: messages.patch I thought there was also markup missing in the QML files. REPOSITORY R104 KScreen

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. Are you talking about this?F7321902: messages.patch REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: victorr, #vdg, GB_2, gvarsanyi, davidedmundson, ngraham,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. Since you found already the root issue want to write a quick patch to fix it? REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: victorr, #vdg, GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. File /kscreen-5.16.4/kcm/kcm.cpp contains name «KAboutData *about = new KAboutData(QStringLiteral("kcm_kscreen"),» After renaming, the translation is displayed. Attached a picture 2.F7321822: skreen2.jpeg REPOSITORY

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. Incorrect filename "kcm_displayconfiguration" in the /kscreen/kcm/Messages.sh file. Translation is not displayed. Attached a picture 1. F7321805: screen1.jpeg REPOSITORY R104 KScreen REVISION DETAIL

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

victorr added a comment. Not all lines entered the localization file. Text in patch +text: i18n("New global scale applied. " + + "Change will come into effect after restart.") Line «Change will come into effect after restart.» is ignored

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R104:6309d33f092e: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami (authored by romangg). CHANGED

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 65426. romangg added a comment. - Add scale debug output CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=65421=65426 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468 AFFECTED FILES CMakeLists.txt

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 65421. romangg added a comment. Rebase on D23732 . CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=65370=65421 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468 AFFECTED FILES

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added inline comments. INLINE COMMENTS > davidedmundson wrote in OutputPanel.qml:85 > These need to be exclusive buttons. Currently you can select N at once. Thanks, the issue wasn't them being not exclusive buttons (this is automatically with ButtonGroup above), but that the

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 65370. romangg added a comment. Fix and simplify rotation buttons. REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=65025=65370 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468 AFFECTED

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#525405 , @davidedmundson wrote: > > Talking of which, there were some updated plans with regards to the UI for X scaling in the meeting that everyone in this thread was at. > > I don't want to have made a

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

davidedmundson added inline comments. INLINE COMMENTS > OutputPanel.qml:85 > + > + RotationButton { > + value: 0 These need to be exclusive buttons. Currently you can select N at once. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To:

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

davidedmundson added a comment. > Talking of which, there were some updated plans with regards to the UI for X scaling in the meeting that everyone in this thread was at. > I don't want to have made a plan in a meeting, and then discard it one week later. I think it's easier to leave that

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 65025. romangg added a comment. - Change visibility and reposition on enablement change REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=63234=65025 BRANCH qmlize REVISION DETAIL

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. Never mind, that was apparently a just-corrected typo (https://community.kde.org/index.php?title=Schedules%2FPlasma_5=revision=85944=85406), and the dependency will be 5.62. Given that, if there's not enough time to get the frameworks changes in, I would be in

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. I'm quite interested getting the rewritten UI and Wayland fractional scaling in ASAP (VDG can always tweak and polish the UI once it's landed). However according to https://community.kde.org/Schedules/Plasma_5, the Frameworks dependency version for Plasma 5.17

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. I just looked at the release schedule of frameworks and it will be difficult to get the required output duplication changes in KWayland 5.62 (tagging date 7th September), which Plasma 5.17 will depend on. Question is if we want to merge even without output

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. FYI: I'm currently working on output duplication (and some preparations for that in KWin/KWayland) and will wait until this works fine before pushing this rewrite. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham accepted this revision as: VDG. ngraham added a comment. (provided the UI tweaks can be made later) REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: #vdg, GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham resigned from this revision. ngraham added a comment. Okay, sure. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin, #vdg Cc: #vdg, GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos, knambiar, broulik, filipf,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#508314 , @ngraham wrote: > In D22468#508290 , @romangg wrote: > > > I currently concentrate on libkscreen backend work and output duplication and have based other

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. In D22468#508290 , @romangg wrote: > I currently concentrate on libkscreen backend work and output duplication and have based other patches on this rewrite here. So I only want to touch this diff again in case of

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#507819 , @ngraham wrote: > Thanks Roman! This is really excellent overall. A big improvement over the current one . Nevertheless I think we can make it even better and I have some UI review comments: > >

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Thanks Roman! This is really excellent overall. A big improvement over the current one . Nevertheless I think we can make it even better and I have some UI review comments:

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#507797 , @ngraham wrote: > Pretty sweet! > > 1. Any chance you'd be interested in implementing the combined scaling/font DPI design we discussed at the sprint, as detailed in D22468#496149

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. Pretty sweet! 1. Any chance you'd be interested in implementing the combined scaling/font DPI design we discussed at the sprint, as detailed in D22468#496149 ? 2. How does one test this? After building kscreen and

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 63234. romangg added a comment. - Add back global scale writing legacy code REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=63232=63234 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#506807 , @GB_2 wrote: > The `icons` folder can be deleted since it's not used anymore I think. True, thanks! REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 63232. romangg added a comment. - Remove icons folder REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=63231=63232 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468 AFFECTED FILES

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 63231. romangg added a comment. Rebase on master. REPOSITORY R104 KScreen CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22468?vs=61910=63231 BRANCH qmlize REVISION DETAIL https://phabricator.kde.org/D22468 AFFECTED FILES

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

GB_2 added a comment. The `icons` folder can be deleted since it's not used anymore I think. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin Cc: GB_2, gvarsanyi, davidedmundson, ngraham, mvourlakos, knambiar, broulik, filipf,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added inline comments. INLINE COMMENTS > broulik wrote in OutputPanel.qml:41 > Try `Kirigami.FormData.checkable: true` to make the checkbox to the left > instead of having a lonely tiny checkbox floating there Then there are two checkboxes. Looked at Night Color KCM, which does it by

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg updated this revision to Diff 61910. romangg marked 23 inline comments as done. romangg added a comment. - Message on backend error - Use switch statements in OutputModel - Initialize roleNames from super class - i18n labels - Checkboxes to the left - Set onClicked -

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. In D22468#496130 , @davidedmundson wrote: > > Screen size and DPI combinations are so variable that I think it makes sense to support a higher level of precision. > > Talking of which, there were some updated plans

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

davidedmundson added a comment. > Ha, you're totally right. Amazing! Is this something to fix in Plasma frameworks? The problem relates to use of Handlers instead of old style MouseArea. I haven't finished investigating, but I have a bad feeling QQuickWindow/QQuickWidget is the

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

gvarsanyi added a comment. In D22468#496052 , @ngraham wrote: > I'm not sold on the proposal to use 1.25x increments, personally. For my personal use case, I like using 1.1x (13" 1080p screen) and I wouldn't want to lose that. Screen size and

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

davidedmundson added a comment. > Screen size and DPI combinations are so variable that I think it makes sense to support a higher level of precision. Talking of which, there were some updated plans with regards to the UI for X scaling in the meeting that everyone in this thread was at.

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

mvourlakos added a comment. In D22468#496123 , @ngraham wrote: > I guess I was thinking that it should be aligned to the slider itself, like this: > > Scale: |---o---| > 1x 1.5x2x >

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

filipf added a comment. There is an example in the Fuzzy Clock config: F7029166: image.png In D22468#496052 , @ngraham wrote: > I'm not sold on the proposal to use 1.25x increments, personally. For

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. I guess I was thinking that it should be aligned to the slider itself, like this: Scale: |---o---| 1x 1.5x2x REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

mvourlakos added a comment. In D22468#496052 , @ngraham wrote: > For the scale slider, I'd put the numbers underneath, and have threenumbers; one on each end and one in the middle. So if the maximum value was 2x, it would look like this: >

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

ngraham added a comment. For the scale slider, I'd put the numbers underneath, and have threenumbers; one on each end and one in the middle. So if the maximum value was 2x, it would look like this: |---o---| 1x 1.5x2x I'm not sold on the

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

gvarsanyi added inline comments. INLINE COMMENTS > OutputPanel.qml:96 > +to: 3 > +stepSize: 0.2 > +live: true `stepSize` settings confuse me a bit. For perOutput it's 0.2, for global, it's 0.1. I recommend setting both to 0.25 increments. Both

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

broulik added a comment. In D22468#495897 , @romangg wrote: > Ha, you're totally right. Amazing! Is this something to fix in Plasma frameworks? Breeze kstyle. I bet the issue is that handlers don't accept the events and/or process them

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#495791 , @davidedmundson wrote: > > On Wayland everything works fine. Might be a Qt bug in input handlers directly and not the code here. Other ideas? > > It'll be our code. Try running with --style=fusion

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

davidedmundson added a comment. > On Wayland everything works fine. Might be a Qt bug in input handlers directly and not the code here. Other ideas? It'll be our code. Try running with --style=fusion and see if the problem magically goes away. If so, it'll be us not accepting the

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. On X11 I had problems moving the outputs in the overview. For some reason the click was not caught by the ui but KWin tries to move the whole window instead. On Wayland everything works fine. Might be a Qt bug in input handlers directly and not the code here. Other

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

filipf added a comment. In D22468#495727 , @filipf wrote: > Yeah I may have improvised that with the Night Color KCM  > > The HIG way (and looking at other places) is to put the

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added inline comments. INLINE COMMENTS > broulik wrote in OutputPanel.qml:41 > `i18n` and everywhere else > i18n and everywhere else I saw in other KCMs the Kirigami label being set without the i18n macro and I hoped Kirigami would do this internally. But apparently not, so I will add

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

filipf added a comment. Yeah I may have improvised that with the Night Color KCM  The HIG way (and looking at other places) is to put the value to the right of the slider. In D22468#495721

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

mvourlakos added a subscriber: ngraham. mvourlakos added a comment. In D22468#495723 , @romangg wrote: > In D22468#495719 , @mvourlakos wrote: > > > This looks really nice, > > > > one question,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#495719 , @mvourlakos wrote: > This looks really nice, > > one question, having current values under the Slider does it follow a HIG rule or is it a proposition? I copied this style from Night Color

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#495679 , @filipf wrote: > Looks better, in particular I like the approach with buttons for orientation because it saves the second or two of having to remember what's clockwise or counter-clockwise. > >

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

mvourlakos added a comment. This looks really nice, one question, having current values under the Slider does it follow a HIG rule or is it a proposition? REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin Cc: mvourlakos,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg added a comment. In D22468#495713 , @knambiar wrote: > The `Scale` slider UI position is different in X11 and Wayland. Is this intentional? Yea, on Wayland we can set the scale for each output individually. On X11 there is only

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

knambiar added a comment. The `Scale` slider UI position is different in X11 and Wayland. Is this intentional? REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D22468 To: romangg, #plasma, #kwin Cc: knambiar, broulik, filipf, plasma-devel, LeGast00n, jraleigh,

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

broulik added a comment. Pretty cool! INLINE COMMENTS > kcm.cpp:48 > +qmlRegisterType(); > +qmlRegisterType("org.kde.kscreen", 1, 0, > "KScreenOutput"); > + Given this is for the KCM only, I would suggest to name the import `org.kde.private.kcm.kscreen` > output_model.cpp:45 > +

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

filipf added a comment. Looks better, in particular I like the approach with buttons for orientation because it saves the second or two of having to remember what's clockwise or counter-clockwise. Only some minor things I noticed in the screenshots: - can the margins on the view

D22468: Rewrite KScreen KCM as ConfigModule with outputs model and Kirigami

romangg created this revision. romangg added reviewers: Plasma, KWin. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. romangg requested review of this revision. REVISION SUMMARY The KScreen KCM was still partly Qt Widgets based. Also it did not use a model for the