Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Martin Gräßlin]

> On Sept. 16, 2015, 6:23 p.m., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c

rather orthogonal to the change. I'm not sure whether a huge switch statemement 
will make it better. What might be more interesting is to get something like 
KWin's Xcb::Atom in to also only fetch the atoms which are actually used. 
Though the Atom class is of course considerable larger than just an xcb_atom_t.


- Martin


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


On Sept. 16, 2015, 3:42 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 3:42 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring that we don't needless copy the atoms into
> each class.
> 
> CHANGELOG: Allow interacting with multiple X servers in the NETWM classes.
> 
> [autotests] NETWM tests get a new X server per test
> 
> Making use of new feature of allowing multiple X connections:
> start X server in init() instead of initTestCase().
> 
> 
> Diffs
> -
> 
>   autotests/netrootinfotestwm.cpp e918873a5281f6ecbcc1769de3dadcf8f6222f6a 
>   autotests/netwininfotestclient.cpp a5b10376b943ea914a9521b5c07f9ef13a65d2f1 
>   autotests/netwininfotestwm.cpp a98e12fd690b0250337c7588e1525af1d0dda38c 
>   src/platforms/xcb/netwm.cpp d99a925ad2b99d5e60ab1dafcb01400b4a6a4c93 
>   src/platforms/xcb/netwm_p.h 917a86ed5b6c83f36e73bbc346360b943d457243 
> 
> Diff: https://git.reviewboard.kde.org/r/125259/diff/
> 
> 
> Testing
> ---
> 
> see adjusted unit tests. Also tried it in kwin_wayland
> 
> 
> 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 125259: Support multiple X servers in the NETWM classes

[Thomas Lübking]

> On Sept. 16, 2015, 4:23 nachm., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.

The switch in the demo code is just a usage demo - I'd promise red overweight 
;-)
Constantly using atom(Atom a) would of course allow a sparse array that mostly 
contains of "0" and is only filled on demand.

The reason why I thought it might be a good idea *now* is because this already 
requires a HUGE change that will block git blame.


- Thomas


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


On Sept. 16, 2015, 1:42 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 1:42 nachm.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring that we don't needless copy the atoms into
> each class.
> 
> CHANGELOG: Allow interacting with multiple X servers in the NETWM classes.
> 
> [autotests] NETWM tests get a new X server per test
> 
> Making use of new feature of allowing multiple X connections:
> start X server in init() instead of initTestCase().
> 
> 
> Diffs
> -
> 
>   autotests/netrootinfotestwm.cpp e918873a5281f6ecbcc1769de3dadcf8f6222f6a 
>   autotests/netwininfotestclient.cpp a5b10376b943ea914a9521b5c07f9ef13a65d2f1 
>   autotests/netwininfotestwm.cpp a98e12fd690b0250337c7588e1525af1d0dda38c 
>   src/platforms/xcb/netwm.cpp d99a925ad2b99d5e60ab1dafcb01400b4a6a4c93 
>   src/platforms/xcb/netwm_p.h 917a86ed5b6c83f36e73bbc346360b943d457243 
> 
> Diff: https://git.reviewboard.kde.org/r/125259/diff/
> 
> 
> Testing
> ---
> 
> see adjusted unit tests. Also tried it in kwin_wayland
> 
> 
> 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 125270: KBuildSycoca: use qCWarning rather than fprintf(stderr, ...) or qWarning

[David Faure]

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

(Updated Sept. 17, 2015, 6:36 a.m.)


Review request for KDE Frameworks and Albert Astals Cid.


Changes
---

Ported to qCWarning, including all qWarning calls


Summary (updated)
-

KBuildSycoca: use qCWarning rather than fprintf(stderr, ...) or qWarning


Repository: kservice


Description (updated)
---

REVIEW: 125270


Diffs (updated)
-

  src/sycoca/kbuildservicefactory.cpp bebb76947672d0e2c100c7149c642406c30355f1 
  src/sycoca/kbuildservicegroupfactory.cpp 
6ac0fc491f42bb117954ba8488b2847c8954e6b3 
  src/sycoca/kbuildservicetypefactory.cpp 
2610c8d99beb12e02070365b078deb4455a6472a 
  src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
  src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
  src/sycoca/ksycocadict.cpp e90969c9acb7ef94ae8da2098400c7207e0aeb67 
  src/sycoca/ksycocafactory.cpp bc55fb54f0836f492ad04519ca1c9f93e9af3880 
  src/sycoca/vfolder_menu.cpp b17ef2d371a2804ebe6029a0b7abfbebe4298d50 

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


Testing
---


Thanks,

David Faure

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

Re: Review Request 125271: KSycoca: rebuild ksycoca in process rather than executing kbuildsycoca5

[David Faure]

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

(Updated Sept. 17, 2015, 6:37 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Albert Astals Cid and Boudewijn Rempt.


Changes
---

Submitted with commit 87e346092d7d50b25ef930bbdb2effe6d9a103e4 by David Faure 
to branch master.


Repository: kservice


Description
---

This is indeed a bit faster.
kmimeassociationstest went from 11-12s to 10.5-11s
and ksycocathreadtest from 11-12s to 7-8s.

REVIEW: 125271


Diffs
-

  src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
  src/sycoca/kbuildsycoca_p.h c24413398694de29a788dd9498435572f5f605a3 
  src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 

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


Testing
---

ctest .


Thanks,

David Faure

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

Re: Review Request 125270: KBuildSycoca: use qWarning rather than fprintf(stderr, ...)

[David Faure]

> On Sept. 16, 2015, 11:46 p.m., Aleix Pol Gonzalez wrote:
> > Shouldn't it be qCWarning?

I'm not sure what's the use case for turning off error messages like "couldn't 
rebuild ksycoca, disk full?", but OK, let's be consistent ;)


- David


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


On Sept. 16, 2015, 9:16 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125270/
> ---
> 
> (Updated Sept. 16, 2015, 9:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> It's just more compact.
> And the message was wrong, too, it was showing "kbuildsycoca5", while
> this code is now in the lib.
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildsycoca.cpp 2afb8b5a6f2516b87dde73e7bf9af0348a962f43 
> 
> Diff: https://git.reviewboard.kde.org/r/125270/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Martin Gräßlin]

> On Sept. 16, 2015, 6:23 p.m., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.

> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.

That sounds of having other disadvantages: round trips as we would need to 
resolve the atom when needed. Also the SharedDataPointer in the patch would not 
work as it would detach them on fetch and cause multiple NETWinInfo to catch 
the atoms again.

So overall from the minimize roundtrip perspective fetching all atoms on first 
run still sounds like the best idea...


- Martin


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


On Sept. 16, 2015, 3:42 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 3:42 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring that we don't needless copy the atoms into
> each class.
> 
> CHANGELOG: Allow interacting with multiple X servers in the NETWM classes.
> 
> [autotests] NETWM tests get a new X server per test
> 
> Making use of new feature of allowing multiple X connections:
> start X server in init() instead of initTestCase().
> 
> 
> Diffs
> -
> 
>   autotests/netrootinfotestwm.cpp e918873a5281f6ecbcc1769de3dadcf8f6222f6a 
>   autotests/netwininfotestclient.cpp a5b10376b943ea914a9521b5c07f9ef13a65d2f1 
>   autotests/netwininfotestwm.cpp a98e12fd690b0250337c7588e1525af1d0dda38c 
>   src/platforms/xcb/netwm.cpp d99a925ad2b99d5e60ab1dafcb01400b4a6a4c93 
>   src/platforms/xcb/netwm_p.h 917a86ed5b6c83f36e73bbc346360b943d457243 
> 
> Diff: https://git.reviewboard.kde.org/r/125259/diff/
> 
> 
> Testing
> ---
> 
> see adjusted unit tests. Also tried it in kwin_wayland
> 
> 
> 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 125274: KBuildSycoca: remove writing of the ksycoca5stamp file.

[Burkhard Lück]

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



docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook (lines 27 - 28)


Please update  and 


- Burkhard Lück


On Sept. 16, 2015, 11:32 nachm., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125274/
> ---
> 
> (Updated Sept. 16, 2015, 11:32 nachm.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> The code for reading it was already removed as part of b0c8fd8e64f,
> the removal of the --checkstamps option.
> 
> The feature still exists anyway, a timestamp of "at least everything until
> that time is in the DB" is in the sycoca header, and it used by the timestamp
> check in ksycoca.cpp. This was an unnecessary separate file.
> 
> REVIEW: 125274
> 
> 
> Diffs
> -
> 
>   docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook 
> 3419e42cb29c6699bc17e6dd46bbc523139c59eb 
>   src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
> 
> Diff: https://git.reviewboard.kde.org/r/125274/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Martin Gräßlin]

> On Sept. 16, 2015, 6:23 p.m., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.
> 
> Martin Gräßlin wrote:
> > Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> That sounds of having other disadvantages: round trips as we would need 
> to resolve the atom when needed. Also the SharedDataPointer in the patch 
> would not work as it would detach them on fetch and cause multiple NETWinInfo 
> to catch the atoms again.
> 
> So overall from the minimize roundtrip perspective fetching all atoms on 
> first run still sounds like the best idea...
> 
> Thomas Lübking wrote:
> "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> > SharedDataPointer in the patch would not work as it would detach them 
> on fetch
> 
> For "Atom" i actually meant an enum, not KWin's XCB::Atom - I don't see 
> where this would detach anything, but the roundtrips would oc. occur.
> One could decide to load a "standard" set on init and the rest only on 
> demand, but you don't win memory by this (just "maybe" a little time on first 
> NETWM invocation)

> "to get something like KWin's Xcb::Atom in to also only fetch the atoms which 
> are actually used." ... :-P

KWin's Xcb::Atom doesn't cause roundtrips.

> I don't see where this would detach anything

>From QSharedDataPointer documentation "For write access operator ->() will 
>automatically call detach()". My understanding is that if we update the value 
>of an xcb_atom_t then it would detach. Granted one could change to 
>QExplicitlySharedDataPointer.


- Martin


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


On Sept. 16, 2015, 3:42 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 3:42 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring that we don't needless copy the atoms into
> each class.
> 
> CHANGELOG: Allow interacting with multiple X servers in the NETWM classes.
> 
> [autotests] NETWM tests get a new X server per test
> 
> Making use of new feature of allowing multiple X connections:

Re: Review Request 125270: KBuildSycoca: use qCWarning rather than fprintf(stderr, ...) or qWarning

[David Faure]

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

(Updated Sept. 17, 2015, 11:53 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Albert Astals Cid.


Changes
---

Submitted with commit 3353017a15c769569ffca503a29a3257deff5463 by David Faure 
to branch master.


Repository: kservice


Description
---

REVIEW: 125270


Diffs
-

  src/sycoca/kbuildservicefactory.cpp bebb76947672d0e2c100c7149c642406c30355f1 
  src/sycoca/kbuildservicegroupfactory.cpp 
6ac0fc491f42bb117954ba8488b2847c8954e6b3 
  src/sycoca/kbuildservicetypefactory.cpp 
2610c8d99beb12e02070365b078deb4455a6472a 
  src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
  src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
  src/sycoca/ksycocadict.cpp e90969c9acb7ef94ae8da2098400c7207e0aeb67 
  src/sycoca/ksycocafactory.cpp bc55fb54f0836f492ad04519ca1c9f93e9af3880 
  src/sycoca/vfolder_menu.cpp b17ef2d371a2804ebe6029a0b7abfbebe4298d50 

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


Testing
---


Thanks,

David Faure

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

Re: Review Request 125274: KBuildSycoca: remove writing of the ksycoca5stamp file.

[David Faure]

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

(Updated Sept. 17, 2015, 12:01 p.m.)


Review request for KDE Frameworks and Albert Astals Cid.


Changes
---

update date and releaseinfo in docbook, as requested


Repository: kservice


Description
---

The code for reading it was already removed as part of b0c8fd8e64f,
the removal of the --checkstamps option.

The feature still exists anyway, a timestamp of "at least everything until
that time is in the DB" is in the sycoca header, and it used by the timestamp
check in ksycoca.cpp. This was an unnecessary separate file.

REVIEW: 125274


Diffs (updated)
-

  src/sycoca/kbuildsycoca.cpp a009d62f24da093988066e8850f9ace5d6f0766d 
  docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook 
3419e42cb29c6699bc17e6dd46bbc523139c59eb 

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


Testing
---


Thanks,

David Faure

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

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Martin Gräßlin]

> On Sept. 16, 2015, 6:23 p.m., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.
> 
> Martin Gräßlin wrote:
> > Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> That sounds of having other disadvantages: round trips as we would need 
> to resolve the atom when needed. Also the SharedDataPointer in the patch 
> would not work as it would detach them on fetch and cause multiple NETWinInfo 
> to catch the atoms again.
> 
> So overall from the minimize roundtrip perspective fetching all atoms on 
> first run still sounds like the best idea...
> 
> Thomas Lübking wrote:
> "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> > SharedDataPointer in the patch would not work as it would detach them 
> on fetch
> 
> For "Atom" i actually meant an enum, not KWin's XCB::Atom - I don't see 
> where this would detach anything, but the roundtrips would oc. occur.
> One could decide to load a "standard" set on init and the rest only on 
> demand, but you don't win memory by this (just "maybe" a little time on first 
> NETWM invocation)
> 
> Martin Gräßlin wrote:
> > "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> KWin's Xcb::Atom doesn't cause roundtrips.
> 
> > I don't see where this would detach anything
> 
> From QSharedDataPointer documentation "For write access operator ->() 
> will automatically call detach()". My understanding is that if we update the 
> value of an xcb_atom_t then it would detach. Granted one could change to 
> QExplicitlySharedDataPointer.
> 
> David Faure wrote:
> QSharedDataPointer is used to write value classes, where const methods do 
> not detach, and non-const methods do detach.
> 
> It sounds like you wanted to use QSharedPointer instead (refcounting).

> It sounds like you wanted to use QSharedPointer instead

No, the class I added only has const methods.


- Martin


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


On Sept. 16, 2015, 3:42 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 3:42 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> 

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[David Faure]

> On Sept. 16, 2015, 4:23 p.m., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.
> 
> Martin Gräßlin wrote:
> > Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> That sounds of having other disadvantages: round trips as we would need 
> to resolve the atom when needed. Also the SharedDataPointer in the patch 
> would not work as it would detach them on fetch and cause multiple NETWinInfo 
> to catch the atoms again.
> 
> So overall from the minimize roundtrip perspective fetching all atoms on 
> first run still sounds like the best idea...
> 
> Thomas Lübking wrote:
> "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> > SharedDataPointer in the patch would not work as it would detach them 
> on fetch
> 
> For "Atom" i actually meant an enum, not KWin's XCB::Atom - I don't see 
> where this would detach anything, but the roundtrips would oc. occur.
> One could decide to load a "standard" set on init and the rest only on 
> demand, but you don't win memory by this (just "maybe" a little time on first 
> NETWM invocation)
> 
> Martin Gräßlin wrote:
> > "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> KWin's Xcb::Atom doesn't cause roundtrips.
> 
> > I don't see where this would detach anything
> 
> From QSharedDataPointer documentation "For write access operator ->() 
> will automatically call detach()". My understanding is that if we update the 
> value of an xcb_atom_t then it would detach. Granted one could change to 
> QExplicitlySharedDataPointer.

QSharedDataPointer is used to write value classes, where const methods do not 
detach, and non-const methods do detach.

It sounds like you wanted to use QSharedPointer instead (refcounting).


- David


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


On Sept. 16, 2015, 1:42 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 1:42 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring 

Re: Review Request 125270: KBuildSycoca: use qCWarning rather than fprintf(stderr, ...) or qWarning

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Sept. 17, 2015, 8:36 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125270/
> ---
> 
> (Updated Sept. 17, 2015, 8:36 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> REVIEW: 125270
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildservicefactory.cpp 
> bebb76947672d0e2c100c7149c642406c30355f1 
>   src/sycoca/kbuildservicegroupfactory.cpp 
> 6ac0fc491f42bb117954ba8488b2847c8954e6b3 
>   src/sycoca/kbuildservicetypefactory.cpp 
> 2610c8d99beb12e02070365b078deb4455a6472a 
>   src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
>   src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
>   src/sycoca/ksycocadict.cpp e90969c9acb7ef94ae8da2098400c7207e0aeb67 
>   src/sycoca/ksycocafactory.cpp bc55fb54f0836f492ad04519ca1c9f93e9af3880 
>   src/sycoca/vfolder_menu.cpp b17ef2d371a2804ebe6029a0b7abfbebe4298d50 
> 
> Diff: https://git.reviewboard.kde.org/r/125270/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125226: move EventForge from the desktop containment

[Marco Martin]

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

(Updated Sept. 17, 2015, 10:40 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and Eike Hein.


Changes
---

Submitted with commit 133e534b7dfa29f2f39d227ed12a3d9bc04ccb8e by Marco Martin 
to branch master.


Repository: kdeclarative


Description
---

EventForge can cancel the half-managed events of child items
in order to implement the move of them with press and hold,
not having the parent and the children battling for the same
press-move-release event sequence.

API-wise I think it's fine, apart perhaps the EventForge class name
(perhaps MouseEventGrabber is less technically correct, but still more clear?)


Diffs
-

  src/qmlcontrols/kquickcontrolsaddons/kquickcontrolsaddonsplugin.cpp a33e03f 
  src/qmlcontrols/kquickcontrolsaddons/eventgenerator.cpp PRE-CREATION 
  src/qmlcontrols/kquickcontrolsaddons/eventgenerator.h PRE-CREATION 
  src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 5b711e1 

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


Testing
---


Thanks,

Marco Martin

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

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Thomas Lübking]

> On Sept. 16, 2015, 4:23 nachm., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.
> 
> Martin Gräßlin wrote:
> > Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> That sounds of having other disadvantages: round trips as we would need 
> to resolve the atom when needed. Also the SharedDataPointer in the patch 
> would not work as it would detach them on fetch and cause multiple NETWinInfo 
> to catch the atoms again.
> 
> So overall from the minimize roundtrip perspective fetching all atoms on 
> first run still sounds like the best idea...

"to get something like KWin's Xcb::Atom in to also only fetch the atoms which 
are actually used." ... :-P

> SharedDataPointer in the patch would not work as it would detach them on fetch

For "Atom" i actually meant an enum, not KWin's XCB::Atom - I don't see where 
this would detach anything, but the roundtrips would oc. occur.
One could decide to load a "standard" set on init and the rest only on demand, 
but you don't win memory by this (just "maybe" a little time on first NETWM 
invocation)


- Thomas


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


On Sept. 16, 2015, 1:42 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 1:42 nachm.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able to interact with the X server the first NET class is
> connected to.
> 
> Normally applications don't need to interact with multiple X servers.
> An exception is kwin_wayland which needs a connection to its Xwayland
> server and on the x11 backend to the X server it renders to. So far
> KWin could not use the NET classes for it, causing the rendering window
> to e.g. not have a window title.
> 
> This change removes the implicit constraint on one X server by
> creating a hash of connection and atoms. For each created NET class
> we check whether we have already resolved the atoms, if yes we reuse
> otherwise we create them.
> 
> For the normal use case of one X11 connection the change should be
> rather minimal. Instead of performing the check whether the static
> atoms have been created, it now is a check whether the atoms for the
> connection have been created. The atoms are kept in a
> QSharedDataPointer ensuring that we don't needless copy the atoms into
> each class.
> 
> CHANGELOG: Allow interacting with multiple X servers in the NETWM classes.
> 
> [autotests] NETWM tests get a new X server per test
> 
> Making use of new feature of allowing multiple X connections:
> start X server in init() instead of initTestCase().
> 
> 
> Diffs
> -
> 
>   autotests/netrootinfotestwm.cpp e918873a5281f6ecbcc1769de3dadcf8f6222f6a 
>   autotests/netwininfotestclient.cpp a5b10376b943ea914a9521b5c07f9ef13a65d2f1 
>   autotests/netwininfotestwm.cpp a98e12fd690b0250337c7588e1525af1d0dda38c 
>   src/platforms/xcb/netwm.cpp d99a925ad2b99d5e60ab1dafcb01400b4a6a4c93 
>   src/platforms/xcb/netwm_p.h 917a86ed5b6c83f36e73bbc346360b943d457243 
> 
> Diff: https://git.reviewboard.kde.org/r/125259/diff/
> 
> 
> 

Review Request 125284: make install name of applications.menu file a cached cmake variable

[Harald Sitter]

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

Review request for KDE Frameworks, David Faure, Jonathan Riddell, and Rex 
Dieter.


Repository: kservice


Description
---

To not conflict with kde4runtime nor gnome nor xfce, distributions may
choose to have a different name for their applications.menu file.
To enable this without pitchy patching make the installed name fully
parameterized via the cmake variable APPLICATIONS_MENU_NAME.

This is based on the debian/kubuntu patch to rename the file:
http://anonscm.debian.org/cgit/pkg-kde/frameworks/kservice.git/tree/debian/patches/kubuntu_rename-application-menu-file.diff?h=kubuntu_wily_archive=de26b631f641b0aa5e2e184443ff6970ed5e8b56


Diffs
-

  CMakeLists.txt 958db4a4891a982b7e9a1bd5c903cb4d126e1cdc 
  src/CMakeLists.txt 5ea5b002411a098a0111a6ee552c554ec461cc28 
  src/sycoca/kbuildsycoca.cpp 60a60662ac014ddc73825ca40f604730e0596537 

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


Testing
---

clean cmake && make && make install
> -- Installing: ./etc/xdg/menus/applications.menu

changed cached var && make && make install
> -- Installing: ./etc/xdg/menus/kf5-applications.menu


Thanks,

Harald Sitter

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

kiconloader/qbuffer device not open puzzle?

[Boudewijn Rempt]

I'm wondering if others see this as well and if there's something simple
that I've missed that fixes it... For me, right now, with an up-to-date
home-grown Qt 5.5 and kdesrc-build-built frameworks, icons don't get
loaded once the cache exists.

In bool KIconLoaderPrivate::findCachedPixmapWithPath(const QString ,
QPixmap , QString ), this fragment:

QBuffer buffer;
buffer.setBuffer();
buffer.open(QIODevice::ReadOnly);

QDataStream inputStream();
inputStream.setVersion(QDataStream::Qt_4_6);

QString tempPath;
inputStream >> tempPath;

if (inputStream.status() == QDataStream::Ok) {
QPixmap tempPixmap;
inputStream >> tempPixmap;

Gives

krita(32136)/(default) unknown: QIODevice::seek (QBuffer): The device is not 
open

On the last line. I first thought it was a Krita porting bug, but kate
gives me the same, and commenting out this code gives me back my icons.

I sort of feel blind after spending two weeks porting calligra away
from kdelibs4support, so I might just be missing something here.

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

Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Jan Grulich]

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

Review request for KDE Frameworks and Ivan Romanov.


Repository: qca


Description
---

This is used also in other header files from include folder. I'm trying to use 
QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) in 
CMake due to NetworkManager and thus plasma-nm will not compile with QCA if 
signals/slots are used.


Diffs
-

  include/QtCrypto/qca_safetimer.h 68dde5c 

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


Testing
---


Thanks,

Jan Grulich

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

Re: Review Request 125259: Support multiple X servers in the NETWM classes

[Thomas Lübking]

> On Sept. 16, 2015, 4:23 nachm., Thomas Lübking wrote:
> > would one want to use the occasion to get rid of the bazillion variables 
> > mapped to a struct array and have atom(Atom a) and atomString(Atom a) for 
> > an array of atoms a matching string - maybe including some namespaces so 
> > that instead of p->atom->net_wm_to_many_underscores one would have 
> > atom(NET::WM::NoUnderscores) and atom(WM::NoUnderscores) etc.?
> > 
> > Ideally with a little preprocessor and include magic?
> > http://stackoverflow.com/questions/147267/easy-way-to-use-variables-of-enum-types-as-string-in-c
> 
> Martin Gräßlin wrote:
> rather orthogonal to the change. I'm not sure whether a huge switch 
> statemement will make it better. What might be more interesting is to get 
> something like KWin's Xcb::Atom in to also only fetch the atoms which are 
> actually used. Though the Atom class is of course considerable larger than 
> just an xcb_atom_t.
> 
> Thomas Lübking wrote:
> The switch in the demo code is just a usage demo - I'd promise red 
> overweight ;-)
> Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> The reason why I thought it might be a good idea *now* is because this 
> already requires a HUGE change that will block git blame.
> 
> Martin Gräßlin wrote:
> > Constantly using atom(Atom a) would of course allow a sparse array that 
> mostly contains of "0" and is only filled on demand.
> 
> That sounds of having other disadvantages: round trips as we would need 
> to resolve the atom when needed. Also the SharedDataPointer in the patch 
> would not work as it would detach them on fetch and cause multiple NETWinInfo 
> to catch the atoms again.
> 
> So overall from the minimize roundtrip perspective fetching all atoms on 
> first run still sounds like the best idea...
> 
> Thomas Lübking wrote:
> "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> > SharedDataPointer in the patch would not work as it would detach them 
> on fetch
> 
> For "Atom" i actually meant an enum, not KWin's XCB::Atom - I don't see 
> where this would detach anything, but the roundtrips would oc. occur.
> One could decide to load a "standard" set on init and the rest only on 
> demand, but you don't win memory by this (just "maybe" a little time on first 
> NETWM invocation)
> 
> Martin Gräßlin wrote:
> > "to get something like KWin's Xcb::Atom in to also only fetch the atoms 
> which are actually used." ... :-P
> 
> KWin's Xcb::Atom doesn't cause roundtrips.
> 
> > I don't see where this would detach anything
> 
> From QSharedDataPointer documentation "For write access operator ->() 
> will automatically call detach()". My understanding is that if we update the 
> value of an xcb_atom_t then it would detach. Granted one could change to 
> QExplicitlySharedDataPointer.
> 
> David Faure wrote:
> QSharedDataPointer is used to write value classes, where const methods do 
> not detach, and non-const methods do detach.
> 
> It sounds like you wanted to use QSharedPointer instead (refcounting).
> 
> Martin Gräßlin wrote:
> > It sounds like you wanted to use QSharedPointer instead
> 
> No, the class I added only has const methods.

Yes, but whenever or however you want to delay atom creation, that's done.

It would work for XCB::Atom only because "xcb_atom_t() const" performs a const 
cast on ::getReply(), so one would cheat "atom(AtomEnum a) const" in exactly 
the same way - and likely require a mutex in either case.

Afaiu, what you'd prefer in the end was an XCB::Atom-a-like approach, but w/ 
external m_connection pointer and a

union {
xcb_intern_atom_cookie_t cookie;
xcb_atom_t atom;
} u;

?

Apple has that [1] (but) it (only) works as the cookie struct consist of only 
one unsigned integer.

[1] 
http://www.opensource.apple.com/source/X11libs/X11libs-40/xcb-util/xcb-util-0.3.3/atom/xcb_atom.h


- Thomas


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


On Sept. 16, 2015, 1:42 nachm., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125259/
> ---
> 
> (Updated Sept. 16, 2015, 1:42 nachm.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> So far on first creation of a NETRootInfo or NETWinInfo a static
> initialization of atoms was performed. This meant that the NET classes
> are only able 

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Andrea Scarpino]

> On Set. 17, 2015, 4:13 p.m., Andrea Scarpino wrote:
> > Inviala!

And in Choqok too, thanks!

(please do not consider my "Ship It" :-)


- Andrea


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


On Set. 17, 2015, 2:26 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Set. 17, 2015, 2:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Review Request 125290: Refresh Solid's device list before querying in kio_trash

[Bartosz Sławianowski]

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

Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

We need to process events in Qt event loop before querying Solid's device list. 
Solid caches devices and relies on signals to refresh them (at the very least 
that's how fstab backend works) which is a problem if kio_trash process is 
reused.

Currently if kio_trash process is reused and there were some device changes 
(for example new NFS share was mounted) kio_trash will fail to choose valid 
trash directory for "trashing" file - or fail to find all trash directories (in 
case of list action). Next reuse of the same process would actually succeed 
because TrashImpl::move (which is called later in the process of trashing file) 
enters the loop.


Diffs
-

  src/ioslaves/trash/trashimpl.h 9881d8e 
  src/ioslaves/trash/trashimpl.cpp 5a2832a 

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


Testing
---

Arch Linux


Thanks,

Bartosz Sławianowski

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Aleix Pol Gonzalez]

> On Sept. 17, 2015, 4:13 p.m., Andrea Scarpino wrote:
> > Inviala!
> 
> Andrea Scarpino wrote:
> And in Choqok too, thanks!
> 
> (please do not consider my "Ship It" :-)

Oops... xD


- Aleix


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


On Sept. 17, 2015, 4:14 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Sept. 17, 2015, 4:14 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Aleix Pol Gonzalez]

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


+1 this was a problem in KDE Connect as well.

- Aleix Pol Gonzalez


On Sept. 17, 2015, 2:26 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Sept. 17, 2015, 2:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[David Faure]

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

Ship it!


Ship It!

- David Faure


On Sept. 17, 2015, 2:14 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Sept. 17, 2015, 2:14 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Jan Grulich]

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

(Updated Sept. 17, 2015, 2:14 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Ivan Romanov.


Changes
---

Submitted with commit 66b9754170759d9333d5fc1e348642814d0310dd by Jan Grulich 
to branch master.


Repository: qca


Description
---

This is used also in other header files from include folder. I'm trying to use 
QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) in 
CMake due to NetworkManager and thus plasma-nm will not compile with QCA if 
signals/slots are used.


Diffs
-

  include/QtCrypto/qca_safetimer.h 68dde5c 

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


Testing
---


Thanks,

Jan Grulich

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

Re: kiconloader/qbuffer device not open puzzle?

[David Faure]

On Thursday 17 September 2015 14:25:27 Boudewijn Rempt wrote:
> I'm wondering if others see this as well and if there's something simple
> that I've missed that fixes it... For me, right now, with an up-to-date
> home-grown Qt 5.5 and kdesrc-build-built frameworks, icons don't get
> loaded once the cache exists.
> 
> In bool KIconLoaderPrivate::findCachedPixmapWithPath(const QString ,
> QPixmap , QString ), this fragment:
> 
>  QBuffer buffer;
>  buffer.setBuffer();
>  buffer.open(QIODevice::ReadOnly);
> 
>  QDataStream inputStream();
>  inputStream.setVersion(QDataStream::Qt_4_6);
> 
>  QString tempPath;
>  inputStream >> tempPath;
> 
>  if (inputStream.status() == QDataStream::Ok) {
>  QPixmap tempPixmap;
>  inputStream >> tempPixmap;
> 
> Gives
> 
> krita(32136)/(default) unknown: QIODevice::seek (QBuffer): The device is not 
> open

Sounds like the pixmap loading reaches the end of the stream, maybe.

Did you try moving out the cache, so it gets recreated? In order to find out
if this is cache corruption or something else.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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

Re: kiconloader/qbuffer device not open puzzle?

[Boudewijn Rempt]

On Thu, 17 Sep 2015, Boudewijn Rempt wrote:


On Thu, 17 Sep 2015, David Faure wrote:


Sounds like the pixmap loading reaches the end of the stream, maybe.

Did you try moving out the cache, so it gets recreated? In order to find 

out

if this is cache corruption or something else.


Yes, removing the cache 'fixes' the issue -- the icon isn't in the cache,
the cache gets recreated and the next time I start a kf5 application, the
icons can't be loaded again.



I'll also set up a dev env on another system, and see if that is reproducible.
The weird thing is, Dmitry reports the same problem, too.

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Andrea Scarpino]

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

Ship it!


Inviala!

- Andrea Scarpino


On Set. 17, 2015, 2:26 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Set. 17, 2015, 2:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Re: kiconloader/qbuffer device not open puzzle?

[Boudewijn Rempt]

On Thu, 17 Sep 2015, David Faure wrote:


Sounds like the pixmap loading reaches the end of the stream, maybe.

Did you try moving out the cache, so it gets recreated? In order to find out
if this is cache corruption or something else.


Yes, removing the cache 'fixes' the issue -- the icon isn't in the cache,
the cache gets recreated and the next time I start a kf5 application, the
icons can't be loaded again.

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

Re: Review Request 125284: make install name of applications.menu file a cached cmake variable

[Hrvoje Senjan]

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


+1
This will save a lot of patch rebasing :D (i'm assuming every distro renames 
the file)

- Hrvoje Senjan


On Sept. 17, 2015, 11:32 a.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125284/
> ---
> 
> (Updated Sept. 17, 2015, 11:32 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, Jonathan Riddell, and Rex 
> Dieter.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> To not conflict with kde4runtime nor gnome nor xfce, distributions may
> choose to have a different name for their applications.menu file.
> To enable this without pitchy patching make the installed name fully
> parameterized via the cmake variable APPLICATIONS_MENU_NAME.
> 
> This is based on the debian/kubuntu patch to rename the file:
> http://anonscm.debian.org/cgit/pkg-kde/frameworks/kservice.git/tree/debian/patches/kubuntu_rename-application-menu-file.diff?h=kubuntu_wily_archive=de26b631f641b0aa5e2e184443ff6970ed5e8b56
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 958db4a4891a982b7e9a1bd5c903cb4d126e1cdc 
>   src/CMakeLists.txt 5ea5b002411a098a0111a6ee552c554ec461cc28 
>   src/sycoca/kbuildsycoca.cpp 60a60662ac014ddc73825ca40f604730e0596537 
> 
> Diff: https://git.reviewboard.kde.org/r/125284/diff/
> 
> 
> Testing
> ---
> 
> clean cmake && make && make install
> > -- Installing: ./etc/xdg/menus/applications.menu
> 
> changed cached var && make && make install
> > -- Installing: ./etc/xdg/menus/kf5-applications.menu
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

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

Re: Review Request 125274: KBuildSycoca: remove writing of the ksycoca5stamp file.

[Albert Astals Cid]

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

Ship it!


Ship It!

- Albert Astals Cid


On set. 17, 2015, 12:01 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125274/
> ---
> 
> (Updated set. 17, 2015, 12:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> The code for reading it was already removed as part of b0c8fd8e64f,
> the removal of the --checkstamps option.
> 
> The feature still exists anyway, a timestamp of "at least everything until
> that time is in the DB" is in the sycoca header, and it used by the timestamp
> check in ksycoca.cpp. This was an unnecessary separate file.
> 
> REVIEW: 125274
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildsycoca.cpp a009d62f24da093988066e8850f9ace5d6f0766d 
>   docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook 
> 3419e42cb29c6699bc17e6dd46bbc523139c59eb 
> 
> Diff: https://git.reviewboard.kde.org/r/125274/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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