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

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.

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 https://phabricator.kde.o

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

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

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 st

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

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 REV

D8796: Support dynamic output enabling/disabling from KScreen

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, lesl

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

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

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 unconditio

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

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 W

D7475: Make EffectsHandlerImpl::announceSupportProperty work without X11

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, lesl

D7475: Make EffectsHandlerImpl::announceSupportProperty work without X11

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 the

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

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

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 longer

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

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, ma

D7323: Expose Cursor position to DeclarativeScripting

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 https://p

D6186: Implement software cursor in OpenGL backend

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

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

D6186: Implement software cursor in OpenGL backend

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

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 c

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

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 fo

D5114: support for high dpi in aurorae

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 wors

D5731: Fix regression for timestamp handling for Xwayland windows

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 as

D5726: Fix regression for timestamp handling for Xwayland windows

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, #plas

D5704: Improve the x11 timestamp handling

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 / 2

D5704: Improve the x11 timestamp handling

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

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, spst

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

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, hardeni

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

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 handle

D5589: [helper] Terminate xclipboardsyncer if kwin_wayland goes down

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) and/or

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

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

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 REVISI

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

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, #plasma

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

luebking added inline comments. INLINE COMMENTS > client.cpp:2086 > +m_edgeGeometryTrackingConnection = connect(this, > &Client::geometryChanged, this, [this, border](){ > +hideClient(true); > +ScreenEdges::self()->reserve(this, border); and if the ge

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

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. Afai

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

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

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: #kwi

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

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 https://phab

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

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 eightb

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

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 etc.

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

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

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

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 in

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

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 some

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

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 E

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

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 been

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

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 a

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

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 Ctrl+

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

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 unsoe

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

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 installed to only o

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

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 https://phabricator.

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

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 PREFEREN

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

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 PREFERE

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

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

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 TopLevel::inputGeometry(

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

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 dif

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

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

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

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

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 https://phabricator.kde.org/settings/panel/emailpref

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

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 https://phabricator.kde.org/settings/p

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

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 *demand* some sort of soft API (slot

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

luebking added a comment. Fixwise good, APIwise, maybe add AbstractClient::placeIn(QRect &area) { 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

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 f

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

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 com

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

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 https://phabricator.kde.org/set

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

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

> 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

> 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 th

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

> 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 wi

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

> 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

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

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 t

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

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

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

> 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

> 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

> 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

> 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

> 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

> 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

> 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

> 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

> 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

To reply, visit: > 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 > >

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

console.log("hour: " + (i*10 + 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

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

> 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

> 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

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 r

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

it first 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: &g

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

/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 wrote: > > --

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

> 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::r

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

> 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 126980: Scale blurbehind and backgroundcontrast besides translating

()); 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. - Tho

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

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

> 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,

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

> 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 unre

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

> 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::r

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

> 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 126968: Morphingpopups effect, to animate tooltips

(line 143) <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

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

> 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? > > > >

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

ear interpolation 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., Mar

  1   2   3   4   >