Re: Review Request 123735: version of QmlObject with a static engine
--- 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
--- 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
--- 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
--- 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
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
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
--- 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
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
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
--- 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
--- 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
--- 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
--- 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
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
--- 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
--- 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
--- 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
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
--- 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
--- 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
--- 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
--- 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