D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

2018-01-14 Thread Thomas Lübking
luebking added a comment. Restricted Application edited projects, added Plasma; removed KWin. I'm not actually smiling ... Anyway, instead of using three vectors it's strongly suggest to use one vector (or rather array) of a struct. a) you can access the inner values by names b) it's

D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

2018-01-14 Thread Thomas Lübking
luebking added a comment. Restricted Application edited projects, added KWin; removed Plasma. Why was that a problem? (Or how can I see the previous patch in this messy review tool which draws an entire core when typing...) REPOSITORY R108 KWin REVISION DETAIL

D9879: [effects/blur] Disable texture cache on Wayland

2018-01-14 Thread Thomas Lübking
luebking added a comment. Restricted Application edited projects, added KWin; removed Plasma. On a general note, UI-wise: if there's no way to ever enable an item it should not be shown anyway. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D9879 To: graesslin, #kwin,

D9699: Keep fullscreen windows in active layer based on transients not the group

2018-01-06 Thread Thomas Lübking
luebking added a comment. Restricted Application edited projects, added KWin; removed Plasma. The original implementation based the fullscreen status on the stack position of the window (ie. whenever a window would rise above the plain stack position of the FS window, it would loose the FS

D9608: [KScreen Effect] Fade opacity only for transparent windows

2018-01-02 Thread Thomas Lübking
luebking added a comment. Restricted Application edited projects, added KWin; removed Plasma. Should the lock screen not be guarded by a black layer in the compositor? As of the effect, maybe this should rather fade in a black layer than fade out everything else? REPOSITORY R108 KWin

D8796: Support dynamic output enabling/disabling from KScreen

2017-11-13 Thread Thomas Lübking
luebking added a comment. Ignore the request as "probably temporary" and just freeze rendering? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff,

D8145: Update pointer position whenever a window gets (un)minimized

2017-10-05 Thread Thomas Lübking
luebking added a comment. Why should it? The stack remains the same, just one window changes its visibility. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8145 To: graesslin, #kwin, #plasma Cc: luebking, davidedmundson, plasma-devel, kwin, bwowk, ZrenBot, progwolff,

D7521: [WIP] make use of foreign protocol

2017-08-25 Thread Thomas Lübking
luebking added a comment. There's a major pitfall to inter-process modality: In case of a persistent transient (used by several clients), the leader may loose the focus access "forever" (because the modal window remains) Thus and in general, transients should certainly not be

D7398: Don't create QWhatsThis when user presses showContextHelp button

2017-08-25 Thread Thomas Lübking
luebking added a comment. Probably to handle internal WhatsThis online help (ie. you click the maximize button of a deco and get the information that this is a maximize button) but I don't know either. Apparently even Lubos was already unsure what this was about. I'm not sure whether

D7475: Make EffectsHandlerImpl::announceSupportProperty work without X11

2017-08-23 Thread Thomas Lübking
luebking added a comment. I take there's no such support announcement on native wayland at all? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D7475 To: graesslin, #kwin, #plasma, davidedmundson Cc: luebking, davidedmundson, plasma-devel, kwin, ZrenBot, progwolff,

D7475: Make EffectsHandlerImpl::announceSupportProperty work without X11

2017-08-23 Thread Thomas Lübking
luebking added a comment. Pretty uninformed consideration: When dealing with X11/wayland hybrids, won't one end up with two property indexes which are hard -if not impossible- to align (because one will be assigned by kwin internally and the other by the casual xwayland server)? Might

D7096: Only send active window changes to X11 root window if the X11 window changed

2017-08-20 Thread Thomas Lübking
luebking added a comment. Unless things changed for the worse, clients (through this function) still send a client message (on _NET_ACTIVE_WINDOW) - the difference between the regular call and the force version is the source indication being "2" ("i'm a pager/taskbar and know what i'm doing

D7096: Only send active window changes to X11 root window if the X11 window changed

2017-08-20 Thread Thomas Lübking
luebking added a comment. There's no technical advantage in a local static, only a design one. You prevent it from future mis-use beyond its limited purpose, because m_activeWindow isn't actually a "property" of RootInfo. If you however at some point need several instances, that's no

D7096: Only send active window changes to X11 root window if the X11 window changed

2017-08-20 Thread Thomas Lübking
luebking added a comment. David has some point though - m_activeWindow *can* get out of sync (server error, mal... stupid client - and will be temporarily due to the async setup) and must not be used directly to query the active window. Since there should be only one root per process,

D7323: Expose Cursor position to DeclarativeScripting

2017-08-15 Thread Thomas Lübking
luebking added a comment. 2¢ - you don't have to expose the cursor position for those effects, just a signal that the cursor position changed and the ability to "do some" at the "current" cursor position (which is then resolved by the core) REPOSITORY R108 KWin REVISION DETAIL

D6186: Implement software cursor in OpenGL backend

2017-06-19 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > Kanedias wrote in scene_opengl.cpp:713 > How come! There's a condition on !m_cursorTexture for that, no? Or is > Cursor::cursorChanged itself called on every paint? Sorry, I missed that the "if (!m_cursorTexture) {" condition still covers these

D6186: Implement software cursor in OpenGL backend

2017-06-18 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > scene_opengl.cpp:713 > +// handle shape update on case cursor image changed > +connect(Cursor::self(), ::cursorChanged, this, > updateCursorTexture); > +} You still update the cursor texture with every paint() - there should

D6186: Implement software cursor in OpenGL backend

2017-06-18 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > Kanedias wrote in scene_opengl.cpp:734 > You mean, to check whether BLEND was enabled prior to calling this func? You might have to - and in addition possibly reset the actual blend function from before. Please wait for Martins comment on this,

D6186: Implement software cursor in OpenGL backend

2017-06-18 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > scene_opengl.cpp:699 > +// don't paint if no image for cursor is set > +const QImage img = kwinApp()->platform()->softwareCursor(); > +if (img.isNull()) { The entire head should probably be in some init function, not in every paint

D6141: make shadows work for windows 100%width or height

2017-06-09 Thread Thomas Lübking
luebking added a comment. I think what David has in mind is a shadow constellation like oo || with the lower edges deliberately being omitted - this will cause glitchy rendering (because the bottom edge will be stretched over the edges) - thus he's looking

D5114: support for high dpi in aurorae

2017-06-04 Thread Thomas Lübking
luebking added a comment. Just saw this because of a bug report. Why was this patch approved at all? This line: scaleFactor = (qreal)dpi / (qreal)96; is totally nuts. dpi is already qreal, so 96 is implicitly casted and 96.0f or so would have done. Overmore and far

D5731: Fix regression for timestamp handling for Xwayland windows

2017-05-06 Thread Thomas Lübking
luebking accepted this revision. luebking added a comment. This revision is now accepted and ready to land. Matter of semantics only (if you would want to use the XCB_CURRENT_TIME symbol wrt backend abstraction matters and readability - the invalidity isn't implicitly explained, maybe read

D5726: Fix regression for timestamp handling for Xwayland windows

2017-05-06 Thread Thomas Lübking
luebking added a comment. 0L is XCB_CURRENT_TIME iow "kinda invalid", so it's sane to block that in setX11Time() Also: a conditional early return? In a four line function? Really? ;-P REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5726 To: graesslin, #kwin,

D5704: Improve the x11 timestamp handling

2017-05-04 Thread Thomas Lübking
luebking added a comment. In https://phabricator.kde.org/D5704#106951, @cfeck wrote: > I may be totally unaware about what the changes do, but from reading the comments on the bug report, I had expected to see something like > > if (timestamp > m_X11Time || int(timestamp + INT_MAX /

D5704: Improve the x11 timestamp handling

2017-05-03 Thread Thomas Lübking
luebking added a comment. > Another problem related to timestamp handling is KWin getting broken by wrong timestamps sent by applications. A prominent example is clusterssh Should have scrolled down the mail ;-) REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5704

D5704: Improve the x11 timestamp handling

2017-05-03 Thread Thomas Lübking
luebking added a comment. This should also mitigate the "idiotic client confused X11 time with epoch" condition (iirc some ssh per script?), yesno? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5704 To: graesslin, #kwin, #plasma Cc: luebking, plasma-devel, kwin,

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

2017-04-26 Thread Thomas Lübking
luebking added a comment. According to the bug kwin_wayland receives a SIGINT, not a SIGSEGV? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5589 To: graesslin, #kwin, #plasma Cc: luebking, plasma-devel, kwin, spstarr, progwolff, Zren, lesliezhai, ali-mohamed,

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

2017-04-26 Thread Thomas Lübking
luebking added a comment. This boils down to the question why the process is still lingering around. If the only parent/child link is actually the socket, then it's more likely to zombie around on a bad socket. In this case you can fire as many signals as you want - they'll never be

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

2017-04-26 Thread Thomas Lübking
luebking added a comment. If the child survives the parent that normally means it's forked at some point and the fork will clear the flag - anyway: Kai says the process knocks out gdb - is it a zombie process (indicated by "D", cannot be stopped or killed, let alone being terminated)

D5249: [RFC] New effect plugin - projector (keystone) correction

2017-03-29 Thread Thomas Lübking
luebking added a comment. ftr, not sure about --transform correctnes, but metamodes ViewportIn/ViewportOut work flawless on at least the nvidia blob. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5249 To: nowicki, #plasma, graesslin Cc: luebking, kwin, plasma-devel,

D5245: Desaturate non-responsive windows

2017-03-29 Thread Thomas Lübking
luebking added a comment. In https://phabricator.kde.org/D5245#98917, @graesslin wrote: > This is something we do not know. We do not know what color scheme the window uses. Given that a desaturation is probably the best we can do. Lower brightness? REPOSITORY R108 KWin

D5232: Add override to methods that override methods on their parent class

2017-03-29 Thread Thomas Lübking
luebking added a comment. In case you'd rather not want to introduce a git history wall: -Wno-inconsistent-missing-override -Wno-inconsistent-missing-destructor-override -Wno-initializer-overrides REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5232 To: apol,

[Differential] [Changed Subscribers] D4718: support for auto-hidden windows to resize

2017-02-22 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > client.cpp:2086 > +m_edgeGeometryTrackingConnection = connect(this, > ::geometryChanged, this, [this, border](){ > +hideClient(true); > +ScreenEdges::self()->reserve(this, border); and if the geometry

[Differential] [Commented On] D4220: Add a basic SNI for keyboard layout

2017-01-20 Thread Thomas Lübking
luebking added a comment. In https://phabricator.kde.org/D4220#78878, @graesslin wrote: > > This change is about the opposite: supporting *changing* the layout. Not about notifying the layout change, that is something we already support for at least a year. Yes.

[Differential] [Commented On] D4220: Add a basic SNI for keyboard layout

2017-01-20 Thread Thomas Lübking
luebking added a comment. "On Wayland that kded has no real access to the layouts and cannot properly implement switching. Given that it's better to integrate the SNI directly in KWin." I'll raise a fundamental question: is wayland prone to end up being PID 0: systemd PID 1: kwin

[Differential] [Accepted] D4113: Correct inital loading of BorderActivate

2017-01-17 Thread Thomas Lübking
luebking accepted this revision. luebking added a reviewer: luebking. REPOSITORY R108 KWin BRANCH master REVISION DETAIL https://phabricator.kde.org/D4113 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: davidedmundson, #plasma, mart, luebking Cc:

[Differential] [Commented On] D4113: Correct inital loading of BorderActivate

2017-01-13 Thread Thomas Lübking
luebking added a comment. The entire parsing is totally not safe against JoeReddiot murking around in the config file, I wonder what happens if we pass "-1" and what is " " cast as... One should probably use "parseInt(border, 10);"? REPOSITORY R108 KWin REVISION DETAIL

[Differential] [Changed Subscribers] D3617: [Touchpad KCM] New KWin Wayland version

2016-12-25 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > subdiff wrote in kwinwaylandbackend.cpp:84 > Are you sure? The foreach keyword is still listed in the official Qt docu. > What's the best alternative? A normal for-loop? With upcounting integer or > iterator? "might deprecate it" - magic

[Differential] [Commented On] D3472: API for window tabbing

2016-11-25 Thread Thomas Lübking
luebking added a comment. On a general note: this terribly looks like doubling all (or a lot of) the core logics reg. tabs - which we had in initial KDE 4 tabbing and which made tabbing *incredibly* buggy. I'd suggest to forward the cores tab logics and not keep local states, counters

[Differential] [Commented On] D3132: [platformx/x11] Add a freeze protection against OpenGL

2016-10-24 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in composite.cpp:748 > this misses the > > if (m_scene->compositingType() & OpenGLCompositing) also indention. > x11_platform.cpp:206 > +// Deliberately continue with PreFrame > +case OpenGLSafePoint::PreFrame: >

[Differential] [Commented On] D3132: [platformx/x11] Add a freeze protection against OpenGL

2016-10-23 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > antlarr wrote in x11_platform.cpp:215 > yeah, but then it wouldn't be a synchronous call and it would lose all its > meaning, isn't it? :) No, why? The purpose is to push the freeze timer forward as long as the GUI thread isn't blocked. If the

[Differential] [Commented On] D3132: [platformx/x11] Add a freeze protection against OpenGL

2016-10-23 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > antlarr wrote in x11_platform.cpp:215 > The timer is in another thread, so it can't be stopped/restarted from this > thread. You should be able to call it as slot (whether QMetaObject::invokeMethod() works, I've never tried) > antlarr wrote

[Differential] [Changed Subscribers] D3132: [platformx/x11] Add a freeze protection against OpenGL

2016-10-23 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > composite.h:242 > bool m_composeAtSwapCompletion; > -bool m_firstFrame = true; > +int m_frameCount = 0; > +int m_maxFramesTestedForSafety = 30; m_testedFrames or similar, "Frame Count" is too generic. Alternatively, just use

[Differential] [Commented On] D2931: Destroy DebugConsole on hide of QWindow

2016-10-16 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in debug_console.cpp:547 > It might be a bug in KWin's QPA, so I wouldn't blame Qt here (yet). Wouldn't that prevent the QWindow signal just as well? REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2931

[Differential] [Commented On] D3037: Support docks which take input

2016-10-16 Thread Thomas Lübking
luebking added a comment. In https://phabricator.kde.org/D3037#56298, @graesslin wrote: > > I don't see any real world cases where that would be needed. Embedded runners/search widgets (pretty famous item on KDE3 times, no idea whether it's relevant) "You have

[Differential] [Commented On] D3037: Support docks which take input

2016-10-12 Thread Thomas Lübking
luebking added a comment. E... this does not allow the dock to control input, ie. the dock can or can not take focus but you want focus if eg. clicking into a lineedit while do certainly not want it when clicking a button that will activate a window (FSP trouble) With the stateful

[Differential] [Changed Subscribers] D2945: Workaround xkbcommon behavior concerning consumed modifiers

2016-10-06 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > keyboard_input.cpp:420 > +// handle this is through new API in xkbcommon which doesn't exist yet > +if (m_consumedModifiers & ~m_modifiers) { > +return mods; Possible pitfalls: What if eg. Ctrl+Alt+Foo creates a keysym, but

[Differential] [Commented On] D2953: [tabbox] Intercept QWheelEvents on QQuickWindow for scrolling

2016-10-06 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in tabboxhandler.cpp:621 > dangerous. If there is a scrolling in both horizontal and vertical (could > happen on touchpad) it could result in no scrolling if one is negative and > the other is positive. One might call that

[Differential] [Changed Subscribers] D2953: [tabbox] Intercept QWheelEvents on QQuickWindow for scrolling

2016-10-06 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > tabboxhandler.cpp:618 > +if (watched == d->window()) { > +if (e->type() == QEvent::Wheel) { > +QWheelEvent *event = static_cast(e); test type first - you'll get many paint events etc. but the filter is

[Differential] [Commented On] D2931: Destroy DebugConsole on hide of QWindow

2016-10-04 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in debug_console.cpp:547 > closeEvent is also not invoked. iow QWidget is bitrot. Maybe better use QML ... :-( REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2931 EMAIL PREFERENCES

[Differential] [Commented On] D2931: Destroy DebugConsole on hide of QWindow

2016-10-04 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > debug_console.cpp:546 > +[this] (bool visible) { > +if (visible) { > +// ignore if (!visible) deleteLater(); ... ;-P REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2931 EMAIL

[Differential] [Commented On] D2931: Destroy DebugConsole on hide of QWindow

2016-10-04 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in debug_console.cpp:547 > I tried implementing the hideEvent, but for whatever reason it didn't get > triggered. What about closeEvent? REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2931 EMAIL

[Differential] [Changed Subscribers] D2931: Destroy DebugConsole on hide of QWindow

2016-10-04 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > sebas wrote in debug_console.cpp:547 > return? Why not reimplment the hideEvent? REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2931 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To:

[Differential] [Changed Subscribers] D2787: Add support for resize only borders on Wayland

2016-09-15 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > abstract_client.cpp:1634 > +const auto margins = decoration()->resizeOnlyBorders(); > +if (!margins.isNull()) { > +return Toplevel::inputGeometry() + margins; spare this test and just return

[Differential] [Commented On] D2786: [server] Don't send key release for not pressed keys and no double key press

2016-09-15 Thread Thomas Lübking
luebking added a comment. Sounds reasonable enough, but seems to point out an issue in wayland: If clients "somehow" rely on balanced input, are there artificial releases when a client looses focus? If not, do you end up with several clients in autorepeat (client side? really? with

[Differential] [Commented On] D2584: Introduce a config option whether applications are allowed to block compositing

2016-09-13 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > sebas wrote in compositing.ui:9 > So your change made the window's minimum width more than 300px wider? That > sounds wrong... > > What I mean is to resize the form in designer to the smallest possible size > before you save it there, perhaps

[Differential] [Commented On] D2630: Support highlighting windows through EffectsHandlerImpl

2016-08-31 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > kwineffects.h:496 > + **/ > +virtual bool performFeature(Feature feature, const QVariantList > ); > + maybe only "perform" (and have the signature fill in the information), but in general a much better solution ;-) REPOSITORY rKWIN

[Differential] [Commented On] D2630: Support highlighting windows through EffectsHandlerImpl

2016-08-31 Thread Thomas Lübking
luebking added a comment. You could expose the relevant method index, but looking up the signature is more elegant (and this isn't a hot call) REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2630 EMAIL PREFERENCES

[Differential] [Accepted] D2627: Set the restore geometry after placing a ShellClient for the first time

2016-08-31 Thread Thomas Lübking
luebking accepted this revision. luebking added a reviewer: luebking. This revision is now accepted and ready to land. REPOSITORY rKWIN KWin BRANCH geometry-restore-after-placement REVISION DETAIL https://phabricator.kde.org/D2627 EMAIL PREFERENCES

[Differential] [Changed Subscribers] D2630: Support highlighting windows through EffectsHandlerImpl

2016-08-30 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > effects.cpp:1560 > +const auto typeId = qMetaTypeId>(); > +for (int i = 0; i < e->metaObject()->methodCount(); ++i) { > +auto method = e->metaObject()->method(i); Errr... wut? Any reason to not

[Differential] [Commented On] D2627: Set the restore geometry after placing a ShellClient for the first time

2016-08-30 Thread Thomas Lübking
luebking added a comment. Fixwise good, APIwise, maybe add AbstractClient::placeIn(QRect ) { Placement::self()->place(this, area); setGeometryRestore(geometry()); } to a) prevent uninformed messing around with the restore geomery b) guarantee sync behavior (when placing occurs) ?

[Differential] [Commented On] D2584: Introduce a config option whether applications are allowed to block compositing

2016-08-26 Thread Thomas Lübking
luebking added a comment. They'll unredirect the window. The property is however not mandarory: "The compositing manager MAY bypass compositing for both fullscreen and non-fullscreen windows if bypassing is requested" (the supplemental "but MUST NOT bypass if it would cause differences

[Differential] [Changed Subscribers] D2584: Introduce a config option whether applications are allowed to block compositing

2016-08-26 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > compositing.h:63 > int openGLPlatformInterface() const; > +bool isWindowsBlockingCompositing() const; > The "is" prefix is a QML mandate, right? isObeyingCompositorBlockRequests isCompositorBlockAllowed However, afaiu the biggest

[Differential] [Commented On] D2531: Warp the xcb pointer whenever pointer leaves an X11 surface

2016-08-22 Thread Thomas Lübking
luebking added a comment. Errr you should get a LeaveNotify on grabs and an EnterNotify on releases anyway (but I do not really understand the tackled problem) REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2531 EMAIL PREFERENCES

[Differential] [Commented On] D2531: Warp the xcb pointer whenever pointer leaves an X11 surface

2016-08-22 Thread Thomas Lübking
luebking added a comment. Could one map/unmap an InputOnly window instead to trigger a (differen) leave notify event? (or would unmapping the window cause a - disturbing? - enter notify event?) REPOSITORY rKWIN KWin REVISION DETAIL https://phabricator.kde.org/D2531 EMAIL PREFERENCES

Re: Review Request 128647: [DesktopView] Stop raising desktop over dialog windows

2016-08-12 Thread Thomas Lübking
> On Aug. 12, 2016, 11:31 vorm., Martin Gräßlin wrote: > > Could someone please test, whether this works? > > > > diff --git a/shell/desktopview.cpp b/shell/desktopview.cpp > > index 14a25af..38c6621 100644 > > --- a/shell/desktopview.cpp > > +++ b/shell/desktopview.cpp > >

Re: Review Request 128647: [DesktopView] Stop raising desktop over dialog windows

2016-08-12 Thread Thomas Lübking
> On Aug. 12, 2016, 11:05 vorm., Anthony Fieroni wrote: > > I can't see bug in Qt implementaion of setFlags > > https://code.woboq.org/qt5/qtbase/src/plugins/platforms/xcb/qxcbwindow.cpp.html#1130 > > xcb plugin bug? Afaik the entire thing operates on an undocumented string property to carry

Re: Review Request 128647: [DesktopView] Stop raising desktop over dialog windows

2016-08-10 Thread Thomas Lübking
gt; I see it make sense, because it change props on focusIn before click who > triggers the bug. This is correct patch to me. > > Thomas Lübking wrote: > The bug is the sheer existence of this function which is a ridiculous > workaround for Qt *silently* ignoring the window

Re: Review Request 128647: [DesktopView] Stop raising desktop over dialog windows

2016-08-10 Thread Thomas Lübking
> On Aug. 10, 2016, 1:23 nachm., David Rosca wrote: > > https://phabricator.kde.org/D2121 > > Anthony Fieroni wrote: > So why not ship it? > > Sebastian Kügler wrote: > Did you read Martin's comments on the phab request? > > Anthony Fieroni wrote: > I see it make sense, because it

[Differential] [Commented On] D2164: Allow struts on panels between screen edges if WM is KWin

2016-07-14 Thread Thomas Lübking
luebking added a comment. In https://phabricator.kde.org/D2164#40170, @graesslin wrote: > - fixed logic error with platform check See? ;-) Looks good otherwise. INLINE COMMENTS > panelview.cpp:926 > > -//Extended struts against a screen edge near to another screen

[Differential] [Commented On] D2164: Allow struts on panels between screen edges if WM is KWin

2016-07-14 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > graesslin wrote in panelview.cpp:857 > > also, how does this react when the WM is replaced? > > tricky. I think it's a corner case which could be ignored. We don't really > have a way to detect it. I would say only "experienced" users know how

[Differential] [Changed Subscribers] D2164: Allow struts on panels between screen edges if WM is KWin

2016-07-14 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > panelview.cpp:848 > > +bool PanelView::shouldNotSetStrut() const > +{ API sanity: "if (!shouldNotSetStrut())" ... double negations make people dizzy ;-) > panelview.cpp:857 > +NETRootInfo rootInfo(QX11Info::connection(), NET::Supported

[Differential] [Changed Subscribers] D1807: Request the resize of ShellSurface after Decoration updated the borders when maximizing

2016-06-09 Thread Thomas Lübking
luebking added inline comments. INLINE COMMENTS > shell_client.cpp:592 > +if (m_maximizeMode == MaximizeFull) { > +m_geomMaximizeRestore = geometry(); > +requestGeometry(workspace()->clientArea(MaximizeArea, this)); ifff geometry() includes the deco for the ShellClient, this

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-14 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-13 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-13 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-11 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-11 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-11 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-11 Thread Thomas Lübking
> On April 11, 2016, 5:45 vorm., Martin Gräßlin wrote: > > > otherwise kwindowsystem::self() is nullptr > > > > how can KWindowSystem::self() be null? And why should that have anything to > > do with KWin? KWindowSystem does not depend on a window manager runn

Re: Review Request 127631: [ksmserver] We must be sure that kwin process is ended

2016-04-11 Thread Thomas Lübking
; https://git.reviewboard.kde.org/r/127631/ > --- > > (Updated April 10, 2016, 7:46 nachm.) > > > Review request for kwin, Plasma, David Edmundson, Martin Gräßlin, and Thomas > Lübking. > > > Repository: plasma-workspace > > > Description &g

Re: Review Request 127102: Use fixed width for digital clock applet

2016-02-19 Thread Thomas Lübking
+ i) + ", minute: " + (i*10 + i) + " -> " > + str); > } > > qml: hour: 0, minute: 0 -> 00:00 > qml: hour: 11, minute: 11 -> 11:11 > qml: hour: 22, minute: 22 -> 22:22 > qml: hour: 33, minute: 33 -> 09:33 > q

Re: Review Request 127102: Use fixed width for digital clock applet

2016-02-18 Thread Thomas Lübking
> On Feb. 18, 2016, 12:05 a.m., David Edmundson wrote: > > applets/digital-clock/package/contents/ui/DigitalClock.qml, line 555 > > > > > > rather than looping, can we use FontMetric's maximumCharacterWidth > >

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-17 Thread Thomas Lübking
> On Feb. 17, 2016, 2:06 p.m., Thomas Lübking wrote: > > effects/morphingpopups/package/contents/code/morphingpopups.js, line 27 > > <https://git.reviewboard.kde.org/r/126968/diff/5/?file=98#file98line27> > > > > Hold it! ;-) > > >

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-17 Thread Thomas Lübking
n move also when fade ended. Once somebody decides to alter durations, you'll run into conflicts. - Thomas Lübking On Feb. 17, 2016, 2:03 p.m., Marco Martin wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-16 Thread Thomas Lübking
rst and start a new animation because and if that failed (retarget returning false) - Thomas Lübking On Feb. 11, 2016, 8:52 a.m., Marco Martin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > ht

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-16 Thread Thomas Lübking
/morphingpopups.js (line 67) <https://git.reviewboard.kde.org/r/126968/#comment63043> I guess that's not required - you'll either have two entries or none. - Thomas Lübking On Feb. 11, 2016, 8:52 a.m., Marco Martin

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-09 Thread Thomas Lübking
> On Feb. 2, 2016, 4:40 p.m., Thomas Lübking wrote: > > effects/morphingpopups/package/contents/code/morphingpopups.js, line 86 > > <https://git.reviewboard.kde.org/r/126968/diff/1/?file=442552#file442552line86> > > > > a) let's have AnimationEffect::ret

Re: Review Request 126980: Scale blurbehind and backgroundcontrast besides translating

2016-02-04 Thread Thomas Lübking
> On Feb. 3, 2016, 9:43 p.m., Thomas Lübking wrote: > > effects/backgroundcontrast/contrast.cpp, line 383 > > <https://git.reviewboard.kde.org/r/126980/diff/1/?file=442652#file442652line383> > > > > This looks fishy - and QTransform a tiny bit clumsy

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-03 Thread Thomas Lübking
> On Feb. 3, 2016, 12:18 a.m., Xuetian Weng wrote: > > I just wonder, does this change also fix: > > https://bugs.kde.org/show_bug.cgi?id=241557 ? > > > > Also, I have impression that if there's a big difference between the size > > of two tooltip window, it w

Re: Review Request 126980: Scale blurbehind and backgroundcontrast besides translating

2016-02-03 Thread Thomas Lübking
r.setHeight(r.height() * data.yScale()); shape |= r; } the multiplications might need to be put into qRound or qCeil to avoid glitches. This spares several qregion copies, conversions into and out of painter paths and the second shape translation. - Thomas L

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-03 Thread Thomas Lübking
tps://git.reviewboard.kde.org/r/126968/#comment62783> please move data.*Translation() as well as the forth/back translation into the QTransform to avoid superflous region operations - Thomas Lübking On Feb. 2, 2016, 8:51 p.m., Marco Martin

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-02 Thread Thomas Lübking
> On Feb. 2, 2016, 4:40 nachm., Thomas Lübking wrote: > > effects/morphingpopups/package/contents/code/morphingpopups.js, line 86 > > <https://git.reviewboard.kde.org/r/126968/diff/1/?file=442552#file442552line86> > > > > a) let's have AnimationEffect::ret

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-02 Thread Thomas Lübking
> On Feb. 3, 2016, 12:18 a.m., Xuetian Weng wrote: > > I just wonder, does this change also fix: > > https://bugs.kde.org/show_bug.cgi?id=241557 ? > > > > Also, I have impression that if there's a big difference between the size > > of two tooltip window, it will look bad. No, completely

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-02 Thread Thomas Lübking
) <https://git.reviewboard.kde.org/r/126968/#comment62749> you should not even be here then ;-) - Thomas Lübking On Feb. 2, 2016, 2:04 p.m., Marco Martin wrote: > > --- > This is an automatically generated e-mail. T

Re: Review Request 126968: Morphingpopups effect, to animate tooltips

2016-02-02 Thread Thomas Lübking
> On Feb. 2, 2016, 4:40 p.m., Thomas Lübking wrote: > > effects/morphingpopups/package/contents/code/morphingpopups.js, line 29 > > <https://git.reviewboard.kde.org/r/126968/diff/1/?file=442552#file442552line29> > > > > backtrace? > > Martin Gräßlin

Re: Review Request 126906: Simplified fullscreen blur for Application Dashboard

2016-01-28 Thread Thomas Lübking
> On Jan. 27, 2016, 5:22 p.m., Thomas Lübking wrote: > > effects/blur/blur.cpp, line 474 > > <https://git.reviewboard.kde.org/r/126906/diff/1/?file=441789#file441789line474> > > > > Did you try the behavior on a varying scene? > > > &g

Re: Review Request 126906: Simplified fullscreen blur for Application Dashboard

2016-01-27 Thread Thomas Lübking
nterpolation suffers from artifacts when things below move (I had tried an even smarter trick for general blurring, looks stunning and is incredibly fast ... as long as you deal with static contents) - Thomas Lübking On Jan. 27, 2016, 1:24 p.m., Martin G

Re: Review Request 126713: fix font preview colors

2016-01-15 Thread Thomas Lübking
t) > > David Edmundson wrote: > ...or maybe I should be the one copying this. I have a completely lying > comment about Format_RGB30 not existing. > > Thomas Lübking wrote: > The 30 and some 32 bit formats are more recent additions to Qt5 (5.4, > iirc) >

Re: Review Request 126713: fix font preview colors

2016-01-15 Thread Thomas Lübking
marked as submitted. Review request for Plasma, Martin Gräßlin and Martin Klapetek. Changes --- Submitted with commit 9977edac12d6bec56f8d9cf7f97529177ca842eb by Thomas Lübking to branch Plasma/5.5. Bugs: 336089 https://bugs.kde.org/show_bug.cgi?id=336089 Repository: plasma-desktop

  1   2   3   4   >