[vlc-devel] [PATCH 1/2] qt/dialogmodel: Update error handling before the GUI display
Pierre Lamot
pierre at videolabs.io
Tue Mar 16 10:01:04 UTC 2021
On 2021-03-16 10:40, Romain Vimont wrote:
> On Tue, Mar 16, 2021 at 10:32:57AM +0100, Benjamin Arnaud wrote:
>> ---
>> .../gui/qt/dialogs/dialogs/dialogmodel.cpp | 246
>> ++++++++++++++----
>> .../gui/qt/dialogs/dialogs/dialogmodel.hpp | 177 ++++++++++---
>> modules/gui/qt/maininterface/mainui.cpp | 5 +-
>> 3 files changed, 333 insertions(+), 95 deletions(-)
>>
>> diff --git a/modules/gui/qt/dialogs/dialogs/dialogmodel.cpp
>> b/modules/gui/qt/dialogs/dialogs/dialogmodel.cpp
>> index 9db269a9de..e8bd9b8856 100644
>> --- a/modules/gui/qt/dialogs/dialogs/dialogmodel.cpp
>> +++ b/modules/gui/qt/dialogs/dialogs/dialogmodel.cpp
>> @@ -1,5 +1,7 @@
>>
>> /*****************************************************************************
>> - * Copyright (C) 2019 VLC authors and VideoLAN
>> + * Copyright (C) 2021 VLC authors and VideoLAN
>> + *
>> + * Authors: Benjamin Arnaud <bunjee at omega.gg>
>> *
>> * This program is free software; you can redistribute it and/or
>> modify
>> * it under the terms of the GNU General Public License as published
>> by
>> @@ -15,99 +17,235 @@
>> * along with this program; if not, write to the Free Software
>> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
>> 02110-1301, USA.
>>
>> *****************************************************************************/
>> +
>> #include "dialogmodel.hpp"
>>
>> -static void displayErrorCb(void *p_data, const char *psz_title, const
>> char *psz_text)
>> +// VLC includes
>> +#include <vlc_dialog.h>
>> +#include <qt.hpp>
>> +
>> +//=================================================================================================
>> +// DialogErrorModel
>> +//=================================================================================================
>> +
>> +/* explicit */ DialogErrorModel::DialogErrorModel(QObject * parent)
>> + : QAbstractListModel(parent), m_model(nullptr) {}
>> +
>> +//-------------------------------------------------------------------------------------------------
>> +// QAbstractItemModel implementation
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +QVariant DialogErrorModel::data(const QModelIndex & index, int role)
>> const /* override */
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->errorDisplayed(psz_title, psz_text);
>> + if (m_model == nullptr)
>> + return QVariant();
>> +
>> + const QList<DialogModel::DialogError> & data = m_model->m_data;
>> +
>> + int row = index.row();
>> +
>> + if (row < 0 || row >= data.count())
>> + return QVariant();
>> +
>> + switch (role)
>> + {
>> + case DIALOG_TITLE:
>> + return QVariant::fromValue(data.at(row).title);
>> + case DIALOG_TEXT:
>> + return QVariant::fromValue(data.at(row).text);
>> + default:
>> + return QVariant();
>> + }
>> }
>>
>> -static void displayLoginCb(void *p_data, vlc_dialog_id *dialogId,
>> - const char *psz_title, const char *psz_text,
>> - const char *psz_default_username,
>> - bool b_ask_store)
>> +int DialogErrorModel::rowCount(const QModelIndex &) const /* override
>> */
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->loginDisplayed( dialogId, psz_title, psz_text,
>> psz_default_username, b_ask_store );
>> + return count();
>> }
>>
>> -static void displayQuestionCb(void *p_data, vlc_dialog_id *dialogId,
>> - const char *psz_title, const char *psz_text,
>> - vlc_dialog_question_type i_type,
>> - const char *psz_cancel, const char
>> *psz_action1,
>> - const char *psz_action2)
>> +//-------------------------------------------------------------------------------------------------
>> +// QAbstractItemModel reimplementation
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +QHash<int, QByteArray> DialogErrorModel::roleNames() const /*
>> override */
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->questionDisplayed( dialogId, psz_title, psz_text,
>> static_cast<int>(i_type), psz_cancel, psz_action1, psz_action2 );
>> + QHash<int, QByteArray> roles;
>> +
>> + roles.insert(DialogErrorModel::DIALOG_TITLE, "title");
>> + roles.insert(DialogErrorModel::DIALOG_TEXT, "text");
>> +
>> + return roles;
>> }
>>
>> -static void displayProgressCb(void *p_data, vlc_dialog_id *dialogId,
>> - const char *psz_title, const char
>> *psz_text,
>> - bool b_indeterminate, float f_position,
>> - const char *psz_cancel)
>> +//-------------------------------------------------------------------------------------------------
>> +// Private slots
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +void DialogErrorModel::onErrorBegin()
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->progressDisplayed( dialogId, psz_title, psz_text,
>> b_indeterminate, f_position, psz_cancel);
>> + int row = m_model->m_data.count();
>> +
>> + beginInsertRows(QModelIndex(), row, row);
>> }
>>
>> -static void cancelCb(void *p_data, vlc_dialog_id *dialogId)
>> +void DialogErrorModel::onErrorEnd()
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->cancelled(dialogId);
>> + endInsertRows();
>> +
>> + emit countChanged();
>> }
>>
>> -static void updateProgressCb(void *p_data, vlc_dialog_id *dialogId,
>> float f_value, const char *psz_text)
>> +//-------------------------------------------------------------------------------------------------
>> +// Properties
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +DialogModel * DialogErrorModel::model() const
>> {
>> - DialogModel* that = static_cast<DialogModel*>(p_data);
>> - emit that->progressUpdated( dialogId, f_value, psz_text );
>> + return m_model;
>> }
>>
>> -const vlc_dialog_cbs cbs = {
>> - displayErrorCb,
>> - displayLoginCb,
>> - displayQuestionCb,
>> - displayProgressCb,
>> - cancelCb,
>> - updateProgressCb
>> -};
>> -
>> -DialogModel::DialogModel(QObject *parent)
>> - : QObject(parent)
>> +void DialogErrorModel::setModel(DialogModel * model)
>> {
>> + if (m_model == model) return;
>> +
>> + if (m_model)
>> + disconnect(model, 0, this, 0);
>> +
>> + beginResetModel();
>> +
>> + m_model = model;
>> +
>> + endResetModel();
>> +
>> + if (model)
>> + {
>> + connect(model, &DialogModel::errorBegin, this,
>> &DialogErrorModel::onErrorBegin);
>> + connect(model, &DialogModel::errorEnd, this,
>> &DialogErrorModel::onErrorEnd);
>> + }
>> +
>> + emit modelChanged();
>> }
>>
>> -DialogModel::~DialogModel()
>> +int DialogErrorModel::count() const
>> {
>> - if (m_mainCtx)
>> - vlc_dialog_provider_set_callbacks(m_mainCtx->getIntf(),
>> nullptr, nullptr);
>> + if (m_model == nullptr)
>> + return 0;
>> +
>> + return m_model->m_data.count();
>> }
>>
>> -void DialogModel::dismiss(DialogId dialogId)
>> +//=================================================================================================
>> +// DialogModel
>> +//=================================================================================================
>> +
>> +/* explicit */ DialogModel::DialogModel(intf_thread_t * intf, QObject
>> * parent)
>> + : QObject(parent), m_intf(intf)
>> {
>> - vlc_dialog_id_dismiss(dialogId.m_id);
>> + const vlc_dialog_cbs cbs =
>> + {
>> + onError, onLogin, onQuestion, onProgress, onCancelled,
>> onProgressUpdated
>> + };
>> +
>> + vlc_dialog_provider_set_callbacks(intf, &cbs, this);
>> }
>>
>> -void DialogModel::post_login(DialogId dialogId, const QString&
>> username, const QString& password, bool store)
>> +//-------------------------------------------------------------------------------------------------
>> +// Interface
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +/* Q_INVOKABLE */ void DialogModel::post_login(DialogId dialogId,
>> const QString & username,
>> + const QString &
>> password, bool store)
>> {
>> vlc_dialog_id_post_login(dialogId.m_id, qtu(username),
>> qtu(password), store);
>> }
>>
>> -void DialogModel::post_action1(DialogId dialogId)
>> +/* Q_INVOKABLE */ void DialogModel::post_action1(DialogId dialogId)
>> {
>> vlc_dialog_id_post_action(dialogId.m_id, 1);
>> }
>>
>> -void DialogModel::post_action2(DialogId dialogId)
>> +/* Q_INVOKABLE */ void DialogModel::post_action2(DialogId dialogId)
>> {
>> vlc_dialog_id_post_action(dialogId.m_id, 2);
>> }
>>
>> -void DialogModel::setMainCtx(QmlMainContext* ctx)
>> +/* Q_INVOKABLE */ void DialogModel::dismiss(DialogId dialogId)
>> +{
>> + vlc_dialog_id_dismiss(dialogId.m_id);
>> +}
>> +
>> +//-------------------------------------------------------------------------------------------------
>> +// Private functions
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +void DialogModel::pushError(const DialogError & error)
>> +{
>> + emit errorBegin();
>
> Why do you use signals/slots here for errorBegin() and errorEnd()?
>
> I think yoy could directly call the function (currently called
> onErrorBegin()), right?
>
> Regards
actually there are two objects, the DialogModel which handles the dialog
events from the core and the DialogErrorModel wich provides an
AbstractListModel interface to represent the error list
DialogErrorModel is created on QML side and listen to the events from
the DialogModel, DialogModel doesn't knows about the instances of
DialogErrorModel, hence the custom signals
>> +
>> + m_data.append(error);
>> +
>> + emit errorEnd();
>> +}
>> +
>> +//-------------------------------------------------------------------------------------------------
>> +// Private static functions
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +/* static */ void DialogModel::onError(void * p_data,
>> + const char * psz_title, const
>> char * psz_text)
>> +{
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + DialogError error { psz_title, psz_text };
>> +
>> + QMetaObject::invokeMethod(model, [model, error =
>> std::move(error)]()
>> + {
>> + model->pushError(error);
>> + });
>> +}
>> +
>> +/* static */ void DialogModel::onLogin(void * p_data, vlc_dialog_id *
>> dialogId,
>> + const char * psz_title, const
>> char * psz_text,
>> + const char *
>> psz_default_username, bool b_ask_store)
>> {
>> - if (ctx)
>> - vlc_dialog_provider_set_callbacks(ctx->getIntf(), &cbs,
>> this);
>> - else if (m_mainCtx)
>> - vlc_dialog_provider_set_callbacks(m_mainCtx->getIntf(),
>> nullptr, nullptr);
>> - m_mainCtx = ctx;
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + emit model->login(dialogId, psz_title, psz_text,
>> psz_default_username, b_ask_store);
>> +}
>> +
>> +/* static */ void DialogModel::onQuestion(void * p_data,
>> vlc_dialog_id * dialogId,
>> + const char * psz_title,
>> const char * psz_text,
>> + vlc_dialog_question_type
>> i_type,
>> + const char * psz_cancel,
>> const char * psz_action1,
>> + const char * psz_action2)
>> +{
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + emit model->question(dialogId, psz_title, psz_text,
>> static_cast<int>(i_type), psz_cancel,
>> + psz_action1, psz_action2);
>> +}
>> +
>> +/* static */ void DialogModel::onProgress(void * p_data,
>> vlc_dialog_id * dialogId,
>> + const char * psz_title,
>> const char * psz_text,
>> + bool b_indeterminate, float
>> f_position,
>> + const char * psz_cancel)
>> +{
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + emit model->progress(dialogId, psz_title, psz_text,
>> b_indeterminate, f_position, psz_cancel);
>> +}
>> +
>> +/* static */ void DialogModel::onProgressUpdated(void * p_data,
>> vlc_dialog_id * dialogId,
>> + float f_value, const
>> char * psz_text)
>> +{
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + emit model->progressUpdated(dialogId, f_value, psz_text);
>> +}
>> +
>> +/* static */ void DialogModel::onCancelled(void * p_data,
>> vlc_dialog_id * dialogId)
>> +{
>> + DialogModel * model = static_cast<DialogModel *>(p_data);
>> +
>> + emit model->cancelled(dialogId);
>> }
>> diff --git a/modules/gui/qt/dialogs/dialogs/dialogmodel.hpp
>> b/modules/gui/qt/dialogs/dialogs/dialogmodel.hpp
>> index 40cd932f84..432746d2f1 100644
>> --- a/modules/gui/qt/dialogs/dialogs/dialogmodel.hpp
>> +++ b/modules/gui/qt/dialogs/dialogs/dialogmodel.hpp
>> @@ -1,5 +1,7 @@
>>
>> /*****************************************************************************
>> - * Copyright (C) 2019 VLC authors and VideoLAN
>> + * Copyright (C) 2021 VLC authors and VideoLAN
>> + *
>> + * Authors: Benjamin Arnaud <bunjee at omega.gg>
>> *
>> * This program is free software; you can redistribute it and/or
>> modify
>> * it under the terms of the GNU General Public License as published
>> by
>> @@ -15,6 +17,7 @@
>> * along with this program; if not, write to the Free Software
>> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
>> 02110-1301, USA.
>>
>> *****************************************************************************/
>> +
>> #ifndef DIALOGMODEL_HPP
>> #define DIALOGMODEL_HPP
>>
>> @@ -22,79 +25,173 @@
>> # include "config.h"
>> #endif
>>
>> +// VLC includes
>> #include <vlc_common.h>
>> #include <vlc_dialog.h>
>>
>> -#include <QObject>
>> +// Qt includes
>> +#include <QAbstractListModel>
>>
>> -#include "util/qml_main_context.hpp"
>> +// Forward declarations
>> +class DialogModel;
>>
>> +//-------------------------------------------------------------------------------------------------
>> +// DialogId
>> +//-------------------------------------------------------------------------------------------------
>>
>> -class DialogId {
>> +class DialogId
>> +{
>> Q_GADGET
>> +
>> public:
>> - DialogId(vlc_dialog_id * id = nullptr)
>> - : m_id(id)
>> - {}
>> + DialogId(vlc_dialog_id * id = nullptr) : m_id(id) {}
>>
>> - bool operator ==(const DialogId& other) const {
>> +public: // Operators
>> + bool operator ==(const DialogId & other) const
>> + {
>> return m_id == other.m_id;
>> }
>> - vlc_dialog_id *m_id;
>> +
>> +public: // Variables
>> + vlc_dialog_id * m_id;
>> };
>>
>> Q_DECLARE_METATYPE(DialogId)
>>
>> -class DialogModel : public QObject
>> +//-------------------------------------------------------------------------------------------------
>> +// DialogErrorModel
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +class DialogErrorModel : public QAbstractListModel
>> {
>> Q_OBJECT
>>
>> -public:
>> - Q_PROPERTY(QmlMainContext* mainCtx READ getMainCtx WRITE
>> setMainCtx NOTIFY mainCtxChanged)
>> + Q_ENUMS(DialogRoles)
>> +
>> + Q_PROPERTY(DialogModel * model READ model WRITE setModel NOTIFY
>> modelChanged)
>>
>> - enum QuestionType {
>> - QUESTION_NORMAL,
>> - QUESTION_WARNING,
>> - QUESTION_CRITICAL
>> + Q_PROPERTY(int count READ count NOTIFY countChanged)
>> +
>> +public: // Enums
>> + enum DialogRoles
>> + {
>> + DIALOG_TITLE = Qt::UserRole + 1,
>> + DIALOG_TEXT
>> };
>> - Q_ENUM(QuestionType)
>>
>> public:
>> - explicit DialogModel(QObject *parent = nullptr);
>> - ~DialogModel();
>> + explicit DialogErrorModel(QObject * parent = nullptr);
>> +
>> +public: // QAbstractItemModel implementation
>> + QVariant data(const QModelIndex & index, int role =
>> Qt::DisplayRole) const override;
>> +
>> + int rowCount(const QModelIndex & parent = QModelIndex()) const
>> override;
>>
>> - inline QmlMainContext* getMainCtx() const { return m_mainCtx; }
>> - void setMainCtx(QmlMainContext*);
>> +public: // QAbstractItemModel reimplementation
>> + QHash<int, QByteArray> roleNames() const override;
>> +
>> +private slots:
>> + void onErrorBegin();
>> + void onErrorEnd ();
>>
>> signals:
>> - void errorDisplayed(const QString &title, const QString &text);
>> - void loginDisplayed(DialogId dialogId, const QString &title,
>> - const QString &text, const QString
>> &defaultUsername,
>> + void modelChanged();
>> +
>> + void countChanged();
>> +
>> +public: // Properties
>> + DialogModel * model() const;
>> + void setModel(DialogModel * model);
>> +
>> + int count() const;
>> +
>> +private: // Variables
>> + DialogModel * m_model;
>> +};
>> +
>> +//-------------------------------------------------------------------------------------------------
>> +// DialogModel
>> +//-------------------------------------------------------------------------------------------------
>> +
>> +class DialogModel : public QObject
>> +{
>> + Q_OBJECT
>> +
>> + Q_ENUMS(QuestionType)
>> +
>> +private:
>> + struct DialogError
>> + {
>> + QString title;
>> + QString text;
>> + };
>> +
>> +public: // Enums
>> + // NOTE: Is it really useful to have this declared here ?
>> + enum QuestionType { QUESTION_NORMAL, QUESTION_WARNING,
>> QUESTION_CRITICAL };
>> +
>> +public:
>> + explicit DialogModel(intf_thread_t * intf, QObject * parent =
>> nullptr);
>> +
>> +public: // Interface
>> + Q_INVOKABLE void post_login(DialogId dialogId, const QString &
>> username,
>> + const QString & password, bool store
>> = false);
>> +
>> + Q_INVOKABLE void post_action1(DialogId dialogId);
>> + Q_INVOKABLE void post_action2(DialogId dialogId);
>> +
>> + Q_INVOKABLE void dismiss(DialogId dialogId);
>> +
>> +private: // Functions
>> + void pushError(const DialogError & error);
>> +
>> +private: // Static functions
>> + static void onError(void * p_data, const char * psz_title, const
>> char * psz_text);
>> +
>> + static void onLogin(void * p_data, vlc_dialog_id * dialogId,
>> const char * psz_title,
>> + const char * psz_text, const char *
>> psz_default_username,
>> bool b_ask_store);
>>
>> - void questionDisplayed(DialogId dialogId, const QString &title,
>> - const QString &text, int type,
>> - const QString &cancel, const QString
>> &action1,
>> - const QString &action2);
>> + static void onQuestion(void * p_data, vlc_dialog_id * dialogId,
>> const char * psz_title,
>> + const char * psz_text,
>> vlc_dialog_question_type i_type,
>> + const char * psz_cancel, const char *
>> psz_action1,
>> + const char * psz_action2);
>>
>> - void progressDisplayed(DialogId dialogId, const QString &title,
>> - const QString &text, bool b_indeterminate,
>> - float f_position, const QString &cancel);
>> + static void onProgress(void * p_data, vlc_dialog_id * dialogId,
>> const char * psz_title,
>> + const char * psz_text, bool
>> b_indeterminate, float f_position,
>> + const char *psz_cancel);
>>
>> - void cancelled(DialogId dialogId);
>> + static void onProgressUpdated(void * p_data, vlc_dialog_id *
>> dialogId, float f_value,
>> + const char * psz_text);
>> +
>> + static void onCancelled(void * p_data, vlc_dialog_id * dialogId);
>> +
>> +
>> +signals:
>> + void errorBegin();
>> + void errorEnd ();
>>
>> - void progressUpdated(DialogId dialogId, float f_value, const
>> QString &text);
>> + void login(DialogId dialogId, const QString & title,
>> + const QString & text, const QString & defaultUsername,
>> + bool b_ask_store);
>> +
>> + void question(DialogId dialogId, const QString & title, const
>> QString & text, int type,
>> + const QString & cancel, const QString & action1,
>> const QString & action2);
>> +
>> + void progress(DialogId dialogId, const QString & title, const
>> QString & text,
>> + bool b_indeterminate, float f_position, const
>> QString & cancel);
>> +
>> + void progressUpdated(DialogId dialogId, float f_value, const
>> QString & text);
>> +
>> + void cancelled(DialogId dialogId);
>>
>> - void mainCtxChanged();
>> +private: // Variables
>> + intf_thread_t * m_intf;
>>
>> -public slots:
>> - void dismiss(DialogId dialogId);
>> - void post_login(DialogId dialogId, const QString& username, const
>> QString& password, bool store = false);
>> - void post_action1(DialogId dialogId);
>> - void post_action2(DialogId dialogId);
>> + QList<DialogError> m_data;
>>
>> private:
>> - QmlMainContext* m_mainCtx;
>> + friend class DialogErrorModel;
>> };
>>
>> #endif // DIALOGMODEL_HPP
>> diff --git a/modules/gui/qt/maininterface/mainui.cpp
>> b/modules/gui/qt/maininterface/mainui.cpp
>> index 1b06d85678..a1c9e07e80 100644
>> --- a/modules/gui/qt/maininterface/mainui.cpp
>> +++ b/modules/gui/qt/maininterface/mainui.cpp
>> @@ -103,6 +103,7 @@ bool MainUI::setup(QQmlEngine* engine)
>> rootCtx->setContextProperty( "topWindow", m_interfaceWindow);
>> rootCtx->setContextProperty( "dialogProvider",
>> DialogsProvider::getInstance());
>> rootCtx->setContextProperty( "systemPalette", new
>> SystemPalette(this));
>> + rootCtx->setContextProperty( "dialogModel", new
>> DialogModel(m_intf, this));
>>
>> if (m_mainInterface->hasMediaLibrary())
>> rootCtx->setContextProperty( "medialib",
>> m_mainInterface->getMediaLibrary() );
>> @@ -227,8 +228,10 @@ void MainUI::registerQMLTypes()
>> qmlRegisterType<PlaylistControllerModel>( "org.videolan.vlc", 0,
>> 1, "PlaylistControllerModel" );
>>
>> qmlRegisterType<AboutModel>( "org.videolan.vlc", 0, 1,
>> "AboutModel" );
>> +
>> + qmlRegisterUncreatableType<DialogModel>("org.videolan.vlc", 0, 1,
>> "DialogModel", "");
>> + qmlRegisterType<DialogErrorModel>( "org.videolan.vlc", 0, 1,
>> "DialogErrorModel");
>> qRegisterMetaType<DialogId>();
>> - qmlRegisterType<DialogModel>("org.videolan.vlc", 0, 1,
>> "DialogModel");
>>
>> qmlRegisterType<QmlEventFilter>( "org.videolan.vlc", 0, 1,
>> "EventFilter" );
>>
>> --
>> 2.25.1
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list