Re: Review Request 123735: version of QmlObject with a static engine

2015-06-10 Thread Marco Martin

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

(Updated June 10, 2015, 4:41 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 5f7909408e30869980552b9d74ded57a4f8a6621 by Marco Martin 
to branch master.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine

Adds a class called QuickViewSharedEngine that has the same behavior as 
QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() for 
each instance.
This is used by desktopviews and panelviews to share their engine.

Unfortunately it may not be possible to get the applet configuration dialogs to 
use this, since they still need a separed engine in order to have a different 
controls style (qstyle based) than the stuff in the desktop/panel


Diffs
-

  autotests/CMakeLists.txt adc1102 
  autotests/quickviewsharedengine.cpp PRE-CREATION 
  autotests/util.h PRE-CREATION 
  autotests/util.cpp PRE-CREATION 
  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
  src/quickaddons/CMakeLists.txt 777d07c 
  src/quickaddons/quickviewsharedengine.h PRE-CREATION 
  src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-23 Thread David Edmundson

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

Ship it!


...in 10 days

Great work on all this.

- David Edmundson


On May 21, 2015, 4:02 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 21, 2015, 4:02 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt adc1102 
   autotests/quickviewsharedengine.cpp PRE-CREATION 
   autotests/util.h PRE-CREATION 
   autotests/util.cpp PRE-CREATION 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-21 Thread Marco Martin

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

(Updated May 21, 2015, 4:02 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine

Adds a class called QuickViewSharedEngine that has the same behavior as 
QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() for 
each instance.
This is used by desktopviews and panelviews to share their engine.

Unfortunately it may not be possible to get the applet configuration dialogs to 
use this, since they still need a separed engine in order to have a different 
controls style (qstyle based) than the stuff in the desktop/panel


Diffs (updated)
-

  autotests/CMakeLists.txt adc1102 
  autotests/quickviewsharedengine.cpp PRE-CREATION 
  autotests/util.h PRE-CREATION 
  autotests/util.cpp PRE-CREATION 
  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
  src/quickaddons/CMakeLists.txt 777d07c 
  src/quickaddons/quickviewsharedengine.h PRE-CREATION 
  src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-20 Thread Vishesh Handa

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



src/quickaddons/quickviewsharedengine.h (line 41)
https://git.reviewboard.kde.org/r/123735/#comment55306

It would be awesome if we could have some tests for this class. 

Maybe we can just try and copy the QQuickView tests? 
http://code.qt.io/cgit/qt/qtdeclarative.git/tree/tests/auto/quick/qquickview/tst_qquickview.cpp


- Vishesh Handa


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-20 Thread Marco Martin


 On May 20, 2015, 5:24 p.m., Vishesh Handa wrote:
  src/quickaddons/quickviewsharedengine.h, line 41
  https://git.reviewboard.kde.org/r/123735/diff/8/?file=370104#file370104line41
 
  It would be awesome if we could have some tests for this class. 
  
  Maybe we can just try and copy the QQuickView tests? 
  http://code.qt.io/cgit/qt/qtdeclarative.git/tree/tests/auto/quick/qquickview/tst_qquickview.cpp

yeah, good idea, will give a try


- Marco


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


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-19 Thread Vishesh Handa


 On May 19, 2015, 3:04 p.m., Vishesh Handa wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 60
  https://git.reviewboard.kde.org/r/123735/diff/8/?file=370102#file370102line60
 
  I'm probably missing some parts of the picture. Could you please 
  explain why this needs to be static?
  
  Also, you could just combine the QSharedPointer and the normal pointer.
 
 Marco Martin wrote:
 the whole point is to have a single instance per process...
 or could be done differently, like with a Q_GLOBAL_STATIC...

So does each applet construct its own `QmlObjectSharedEngine`?

Maybe we can disable this class's constructor and just have a `static 
QQmlObjectSharedEngine* instance()` method? It'll make the semantics much 
clearer.


- Vishesh


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


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-19 Thread Vishesh Handa

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



src/kdeclarative/qmlobjectsharedengine.cpp (line 41)
https://git.reviewboard.kde.org/r/123735/#comment55280

On first run `s_engine` is 0, so this will not actually clean up the 
`QQmlEngine`



src/kdeclarative/qmlobjectsharedengine.cpp (line 60)
https://git.reviewboard.kde.org/r/123735/#comment55281

I'm probably missing some parts of the picture. Could you please explain 
why this needs to be static?

Also, you could just combine the QSharedPointer and the normal pointer.


- Vishesh Handa


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-19 Thread Marco Martin


 On May 19, 2015, 3:04 p.m., Vishesh Handa wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 60
  https://git.reviewboard.kde.org/r/123735/diff/8/?file=370102#file370102line60
 
  I'm probably missing some parts of the picture. Could you please 
  explain why this needs to be static?
  
  Also, you could just combine the QSharedPointer and the normal pointer.

the whole point is to have a single instance per process...
or could be done differently, like with a Q_GLOBAL_STATIC...


- Marco


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


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-19 Thread Marco Martin


 On May 19, 2015, 3:04 p.m., Vishesh Handa wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 60
  https://git.reviewboard.kde.org/r/123735/diff/8/?file=370102#file370102line60
 
  I'm probably missing some parts of the picture. Could you please 
  explain why this needs to be static?
  
  Also, you could just combine the QSharedPointer and the normal pointer.
 
 Marco Martin wrote:
 the whole point is to have a single instance per process...
 or could be done differently, like with a Q_GLOBAL_STATIC...
 
 Vishesh Handa wrote:
 So does each applet construct its own `QmlObjectSharedEngine`?
 
 Maybe we can disable this class's constructor and just have a `static 
 QQmlObjectSharedEngine* instance()` method? It'll make the semantics much 
 clearer.

yes, it must have its own QmlObjectSharedEngine, it can't be a singleton by 
itself, because each applet must have a different root QQmlContext


- Marco


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


On May 18, 2015, 8 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 8 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-18 Thread Marco Martin

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

(Updated May 18, 2015, 7:09 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description (updated)
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine

Adds a class called QuickViewSharedEngine that has the same behavior as 
QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() for 
each instance.
This is used by desktopviews and panelviews to share their engine.

Unfortunately it may not be possible to get the applet configuration dialogs to 
use this, since they still need a separed engine in order to have a different 
controls style (qstyle based) than the stuff in the desktop/panel


Diffs
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
  src/quickaddons/CMakeLists.txt 777d07c 
  src/quickaddons/quickviewsharedengine.h PRE-CREATION 
  src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-18 Thread Marco Martin

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

(Updated May 18, 2015, 7:02 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
  src/quickaddons/CMakeLists.txt 777d07c 
  src/quickaddons/quickviewsharedengine.h PRE-CREATION 
  src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-18 Thread David Edmundson

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



src/quickaddons/quickviewsharedengine.cpp (lines 35 - 39)
https://git.reviewboard.kde.org/r/123735/#comment55263

needed?



src/quickaddons/quickviewsharedengine.cpp (line 63)
https://git.reviewboard.kde.org/r/123735/#comment55267

initialise.



src/quickaddons/quickviewsharedengine.cpp (line 228)
https://git.reviewboard.kde.org/r/123735/#comment55269

syncWidth();
syncHeight();

here?

Maybe move to a private method to share with executionFinished()



src/quickaddons/quickviewsharedengine.cpp (line 252)
https://git.reviewboard.kde.org/r/123735/#comment55266

why the cast? it's already a QQmlComponent::Status?


- David Edmundson


On May 18, 2015, 7:09 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 18, 2015, 7:09 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 Adds a class called QuickViewSharedEngine that has the same behavior as 
 QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() 
 for each instance.
 This is used by desktopviews and panelviews to share their engine.
 
 Unfortunately it may not be possible to get the applet configuration dialogs 
 to use this, since they still need a separed engine in order to have a 
 different controls style (qstyle based) than the stuff in the desktop/panel
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/quickaddons/CMakeLists.txt 777d07c 
   src/quickaddons/quickviewsharedengine.h PRE-CREATION 
   src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-18 Thread Marco Martin

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

(Updated May 18, 2015, 8 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine

Adds a class called QuickViewSharedEngine that has the same behavior as 
QmlObjectSharedEngine(using it): static QQmlEngine, separed rootContexts() for 
each instance.
This is used by desktopviews and panelviews to share their engine.

Unfortunately it may not be possible to get the applet configuration dialogs to 
use this, since they still need a separed engine in order to have a different 
controls style (qstyle based) than the stuff in the desktop/panel


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
  src/quickaddons/CMakeLists.txt 777d07c 
  src/quickaddons/quickviewsharedengine.h PRE-CREATION 
  src/quickaddons/quickviewsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-13 Thread Marco Martin


 On May 12, 2015, 5:45 p.m., Mark Gaiser wrote:
  src/kdeclarative/qmlobjectsharedengine.h, line 57
  https://git.reviewboard.kde.org/r/123735/diff/5/?file=368396#file368396line57
 
  std::unique_ptr... ...
  then you can also forget about the delete in the destructor.

buh, fine


 On May 12, 2015, 5:45 p.m., Mark Gaiser wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 48
  https://git.reviewboard.kde.org/r/123735/diff/5/?file=368397#file368397line48
 
  static const QQmlEngine ...
  
  Prevents changing the pointer later on.

not at the moment, since it's initialized to 0 and then instantiated only if 
needed, and refcounted.
it may become const if it would get instantiated no matter what, but don't like 
it that much


- Marco


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


On May 12, 2015, 4:12 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 12, 2015, 4:12 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-13 Thread Marco Martin

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

(Updated May 13, 2015, 5:37 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Marco Martin

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

(Updated May 12, 2015, 4:10 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Marco Martin

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

(Updated May 12, 2015, 4:12 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Marco Martin


 On May 12, 2015, 3:52 p.m., David Edmundson wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 62
  https://git.reviewboard.kde.org/r/123735/diff/2/?file=368373#file368373line62
 
  this needs to be initialised.

line 65
QQmlEngine *QmlObjectSharedEnginePrivate::s_engine = 0;


 On May 12, 2015, 3:52 p.m., David Edmundson wrote:
  src/kdeclarative/qmlobjectsharedengine.cpp, line 69
  https://git.reviewboard.kde.org/r/123735/diff/2/?file=368373#file368373line69
 
  I'm not sure the ownership on the context is right.
  
  I think we want the context to last for the lifespan of the 
  QmlObjectSharedEngine (this), currently it's the lifespan of the enitre app

if i parent it it crashes on initialization, if i delete the context on the 
qmlobject destruction, it seems to work fine


 On May 12, 2015, 3:52 p.m., David Edmundson wrote:
  src/kdeclarative/qmlobject.cpp, line 176
  https://git.reviewboard.kde.org/r/123735/diff/2/?file=368371#file368371line176
 
  if we're using the shared engine we end up not setting the incubation 
  controller. I don't know what difference that will make?

ah, even if now is sync, the incubator is still needed for the 
initialProperties paramenter, added it


- Marco


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


On May 12, 2015, 4:05 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 12, 2015, 4:05 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Mark Gaiser

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



src/kdeclarative/qmlobjectsharedengine.h (line 57)
https://git.reviewboard.kde.org/r/123735/#comment55048

std::unique_ptr... ...
then you can also forget about the delete in the destructor.



src/kdeclarative/qmlobjectsharedengine.cpp (line 48)
https://git.reviewboard.kde.org/r/123735/#comment55050

static const QQmlEngine ...

Prevents changing the pointer later on.



src/kdeclarative/qmlobjectsharedengine.cpp (line 75)
https://git.reviewboard.kde.org/r/123735/#comment55049

remove if you decide to use a smart pointer for this one.


- Mark Gaiser


On mei 12, 2015, 4:12 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated mei 12, 2015, 4:12 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Aleix Pol Gonzalez

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


Can you make the diff against the branch root instead of origin/master? There's 
some unrelated stuff there...

- Aleix Pol Gonzalez


On May 12, 2015, 5:22 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 12, 2015, 5:22 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 
 Diffs
 -
 
   CMakeLists.txt e6155e2 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c788943 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
   src/qmlcontrols/kquickcontrolsaddons/clipboard.h 980ce54 
   src/qmlcontrols/kquickcontrolsaddons/plotter.h a564761 
   tests/helloworld/metadata.desktop dd188c7 
   tests/helloworldnowindow/metadata.desktop 15d8783 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Marco Martin

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

(Updated May 12, 2015, 3:35 p.m.)


Review request for KDE Frameworks and Plasma.


Repository: kdeclarative


Description
---

to make easier doing applications like plasma that use a lot of qml to have a 
single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
has a single, static QQmlEngine


Diffs (updated)
-

  src/kdeclarative/CMakeLists.txt d73bff0 
  src/kdeclarative/kdeclarative.cpp b3906e2 
  src/kdeclarative/qmlobject.h f26b67d 
  src/kdeclarative/qmlobject.cpp c483665 
  src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
  src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 

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


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 123735: version of QmlObject with a static engine

2015-05-12 Thread Aleix Pol Gonzalez

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



src/kdeclarative/qmlobject.h (line 81)
https://git.reviewboard.kde.org/r/123735/#comment55043

Empty comment?



src/kdeclarative/qmlobjectsharedengine.cpp (line 65)
https://git.reviewboard.kde.org/r/123735/#comment55044

Maybe it should better use Q_GLOBAL_STATIC?


- Aleix Pol Gonzalez


On May 12, 2015, 5:35 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123735/
 ---
 
 (Updated May 12, 2015, 5:35 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: kdeclarative
 
 
 Description
 ---
 
 to make easier doing applications like plasma that use a lot of qml to have a 
 single engine make a subclass of QmlObject called QmlObjectSharedEngine that 
 has a single, static QQmlEngine
 
 
 Diffs
 -
 
   src/kdeclarative/CMakeLists.txt d73bff0 
   src/kdeclarative/kdeclarative.cpp b3906e2 
   src/kdeclarative/qmlobject.h f26b67d 
   src/kdeclarative/qmlobject.cpp c483665 
   src/kdeclarative/qmlobjectsharedengine.h PRE-CREATION 
   src/kdeclarative/qmlobjectsharedengine.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/123735/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel