Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2016-01-14 Thread Martin Gräßlin 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/
---

(Updated Jan. 14, 2016, 2:59 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks, kwin and Albert Astals Cid.


Bugs: 354811
https://bugs.kde.org/show_bug.cgi?id=354811


Repository: kwindowsystem


Description
---

We observed that with Compiz as window manager various applications
crash if they use QGuiApplication instead of QApplication. The reason
for this is that on Compiz the mapViewport code paths are used and
that has a widgets dependency.

This change should ensure that applications do at least not crash
in this condition.

BUG: 354811


Diffs
-

  src/platforms/xcb/kwindowsystem.cpp 9d287043c24894ca3c29c439c7939b139da055e8 

Diff: https://git.reviewboard.kde.org/r/126397/diff/


Testing
---

User in referenced bug report tested it, works.


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Martin Gräßlin 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/
---

(Updated Dec. 17, 2015, 10:21 a.m.)


Review request for KDE Frameworks, kwin and Albert Astals Cid.


Changes
---

Testing: works


Bugs: 354811
https://bugs.kde.org/show_bug.cgi?id=354811


Repository: kwindowsystem


Description
---

We observed that with Compiz as window manager various applications
crash if they use QGuiApplication instead of QApplication. The reason
for this is that on Compiz the mapViewport code paths are used and
that has a widgets dependency.

This change should ensure that applications do at least not crash
in this condition.

BUG: 354811


Diffs
-

  src/platforms/xcb/kwindowsystem.cpp 9d287043c24894ca3c29c439c7939b139da055e8 

Diff: https://git.reviewboard.kde.org/r/126397/diff/


Testing (updated)
---

User in referenced bug report tested it, works.


Thanks,

Martin Gräßlin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Martin Gräßlin 


> On Dec. 17, 2015, 1:55 p.m., Thomas Lübking wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
> > displayWidth/displayHeight in kwinglobals.h, right?
> > 
> > We could just "borrow" that code and kick the qApp dep then?
> 
> Thomas Lübking wrote:
> Ok, further check - there's another, unrelated, trap when trying to 
> access the root winId, we'd better fetch that from QX11Info

> Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
> displayWidth/displayHeight in kwinglobals.h, right?

I'm not sure what QDesktopWidget really does. If it's just the root window 
size: yes sure.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89648
---


On Dec. 17, 2015, 10:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 10:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Thomas Lübking 


> On Dec. 17, 2015, 12:55 p.m., Thomas Lübking wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
> > displayWidth/displayHeight in kwinglobals.h, right?
> > 
> > We could just "borrow" that code and kick the qApp dep then?

Ok, further check - there's another, unrelated, trap when trying to access the 
root winId, we'd better fetch that from QX11Info


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89648
---


On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 9:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Thomas Lübking 


> On Dec. 17, 2015, 12:55 p.m., Thomas Lübking wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
> > displayWidth/displayHeight in kwinglobals.h, right?
> > 
> > We could just "borrow" that code and kick the qApp dep then?
> 
> Thomas Lübking wrote:
> Ok, further check - there's another, unrelated, trap when trying to 
> access the root winId, we'd better fetch that from QX11Info
> 
> Martin Gräßlin wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, 
> ie. displayWidth/displayHeight in kwinglobals.h, right?
> 
> I'm not sure what QDesktopWidget really does. If it's just the root 
> window size: yes sure.

Checked it - the geometry is the bounding rect of all QScreens.

Blast, we'll have to use that as well since we've geometry() uses and 
unfortunately cannot just assume that this starts as 0,0 :-(


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89648
---


On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 9:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Thomas Lübking 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89648
---


Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
displayWidth/displayHeight in kwinglobals.h, right?

We could just "borrow" that code and kick the qApp dep then?

- Thomas Lübking


On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 9:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Alex Richardson 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89663
---



src/platforms/xcb/kwindowsystem.cpp (line 355)


qobject_cast?


- Alex Richardson


On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 9:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport

2015-12-17 Thread Thomas Lübking 


> On Dec. 17, 2015, 12:55 p.m., Thomas Lübking wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, ie. 
> > displayWidth/displayHeight in kwinglobals.h, right?
> > 
> > We could just "borrow" that code and kick the qApp dep then?
> 
> Thomas Lübking wrote:
> Ok, further check - there's another, unrelated, trap when trying to 
> access the root winId, we'd better fetch that from QX11Info
> 
> Martin Gräßlin wrote:
> > Looks like this boils down to multiple qApp->desktop()->size() calls, 
> ie. displayWidth/displayHeight in kwinglobals.h, right?
> 
> I'm not sure what QDesktopWidget really does. If it's just the root 
> window size: yes sure.
> 
> Thomas Lübking wrote:
> Checked it - the geometry is the bounding rect of all QScreens.
> 
> Blast, we'll have to use that as well since we've geometry() uses and 
> unfortunately cannot just assume that this starts as 0,0 :-(

https://git.reviewboard.kde.org/r/126403/


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126397/#review89648
---


On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126397/
> ---
> 
> (Updated Dec. 17, 2015, 9:21 a.m.)
> 
> 
> Review request for KDE Frameworks, kwin and Albert Astals Cid.
> 
> 
> Bugs: 354811
> https://bugs.kde.org/show_bug.cgi?id=354811
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> We observed that with Compiz as window manager various applications
> crash if they use QGuiApplication instead of QApplication. The reason
> for this is that on Compiz the mapViewport code paths are used and
> that has a widgets dependency.
> 
> This change should ensure that applications do at least not crash
> in this condition.
> 
> BUG: 354811
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/126397/diff/
> 
> 
> Testing
> ---
> 
> User in referenced bug report tested it, works.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel