From 9474a9afeb7ff63538ee85b4b59e172e5d32db32 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Avatar: provide common logic for uploading; don't store Connection Although the setting part of the work is class(User or Room)-specific, the uploading part is common, so Avatar provides it now. Also: there's no need to store Connection, as it's only used in get() and upload() - just pass it as the parameter to the methods. --- user.cpp | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'user.cpp') diff --git a/user.cpp b/user.cpp index c80ec883..289f0bac 100644 --- a/user.cpp +++ b/user.cpp @@ -39,17 +39,15 @@ class User::Private public: Private(QString userId, Connection* connection) : userId(std::move(userId)), connection(connection) - , avatar(connection, QIcon::fromTheme(QStringLiteral("user-available"))) { } QString userId; QString name; QString bridged; Connection* connection; - Avatar avatar; - QPointer avatarUploadJob = nullptr; + Avatar avatar { QIcon::fromTheme(QStringLiteral("user-available")) }; - void setAvatar(UploadContentJob* job, User* q); + void setAvatar(QString contentUri, User* q); }; User::User(QString userId, Connection* connection) @@ -107,28 +105,20 @@ void User::rename(const QString& newName) bool User::setAvatar(const QString& fileName) { - if (isJobRunning(d->avatarUploadJob)) - return false; - d->setAvatar(d->connection->uploadFile(fileName), this); - return true; + return avatarObject().upload(d->connection, fileName, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } bool User::setAvatar(QIODevice* source) { - if (isJobRunning(d->avatarUploadJob) || !source->isReadable()) - return false; - d->setAvatar(d->connection->uploadContent(source), this); - return true; + return avatarObject().upload(d->connection, source, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } -void User::Private::setAvatar(UploadContentJob* job, User* q) +void User::Private::setAvatar(QString contentUri, User* q) { - avatarUploadJob = job; - connect(job, &BaseJob::success, q, [this,q] { - auto* j = connection->callApi( - userId, avatarUploadJob->contentUri()); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); - }); + auto* j = connection->callApi(userId, contentUri); + connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); } QString User::displayname() const @@ -159,17 +149,23 @@ QImage User::avatar(int dimension) QImage User::avatar(int width, int height) { - return d->avatar.get(width, height, [=] { emit avatarChanged(this); }); + return avatar(width, height, []{}); +} + +QImage User::avatar(int width, int height, Avatar::get_callback_t callback) +{ + return avatarObject().get(d->connection, width, height, + [this,callback] { emit avatarChanged(this); callback(); }); } QString User::avatarMediaId() const { - return d->avatar.mediaId(); + return avatarObject().mediaId(); } QUrl User::avatarUrl() const { - return d->avatar.url(); + return avatarObject().url(); } void User::processEvent(Event* event) -- cgit v1.2.3 From c12f5cc213ecbb40c506e304e6b41c1437ca0d33 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Support per-room user traits User::Private stores two new maps (for display names and avatars respectively) and is able to retrieve and store names/avatars on a per-room basis. The "most used" trait is stored separately and room lists are not kept for it (because most people have a single name and a single avatar across all or most rooms). TODO: The avatar container should be replaced with something less clumsy; the current code doesn't even compile with MSVC. A "handle" copyable structure that would hold the Avatar is one of options. Closes #141. --- user.cpp | 307 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 254 insertions(+), 53 deletions(-) (limited to 'user.cpp') diff --git a/user.cpp b/user.cpp index 289f0bac..2a3071af 100644 --- a/user.cpp +++ b/user.cpp @@ -19,9 +19,11 @@ #include "user.h" #include "connection.h" +#include "room.h" #include "avatar.h" #include "events/event.h" #include "events/roommemberevent.h" +#include "jobs/setroomstatejob.h" #include "jobs/generated/profile.h" #include "jobs/generated/content-repo.h" @@ -29,37 +31,208 @@ #include #include #include +#include #include +#include using namespace QMatrixClient; +using std::move; +using std::exchange; class User::Private { public: + static QIcon defaultIcon(); + static Avatar makeAvatar(QUrl url) { return { url, defaultIcon() }; } + Private(QString userId, Connection* connection) - : userId(std::move(userId)), connection(connection) + : userId(move(userId)), connection(connection) { } QString userId; - QString name; - QString bridged; Connection* connection; - Avatar avatar { QIcon::fromTheme(QStringLiteral("user-available")) }; + + QString mostUsedName; + QString bridged; + Avatar mostUsedAvatar { defaultIcon() }; + QMultiHash otherNames; + std::vector>> otherAvatars; + + mutable int totalRooms = 0; + + QString nameForRoom(const Room* r) const; + std::pair setNameForRoom(const Room* r, QString newName); + const Avatar& avatarForRoom(const Room* r) const; + bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); void setAvatar(QString contentUri, User* q); + }; +QIcon User::Private::defaultIcon() +{ + static const QIcon icon + { QIcon::fromTheme(QStringLiteral("user-available")) }; + return icon; +} + +QString User::Private::nameForRoom(const Room* r) const +{ + return otherNames.key(r, mostUsedName); +} + +static constexpr int MIN_JOINED_ROOMS_TO_LOG = 100; + +std::pair User::Private::setNameForRoom(const Room* r, + QString newName) +{ + if (totalRooms < 2) + { + Q_ASSERT_X(totalRooms > 0 && otherNames.empty(), __FUNCTION__, + "Internal structures inconsistency"); + // The below uses that initialization list evaluation is ordered + return { mostUsedName != newName, + exchange(mostUsedName, move(newName)) }; + } + QString oldName; + // The below works because QMultiHash iterators dereference to stored values + auto it = std::find(otherNames.begin(), otherNames.end(), r); + if (it != otherNames.end()) + { + oldName = it.key(); + if (oldName == newName) + return { false, oldName }; // old name and new name coincide + otherNames.erase(it); + } + if (newName != mostUsedName) + { + // Check if the newName is about to become most used. + if (otherNames.count(newName) >= totalRooms - otherNames.size()) + { + Q_ASSERT(totalRooms > 1); + QElapsedTimer et; + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + { + qCDebug(MAIN) << "Switching the most used name of user" << userId + << "from" << mostUsedName << "to" << newName; + qCDebug(MAIN) << "The user is in" << totalRooms << "rooms"; + et.start(); + } + + for (auto* r1: connection->roomMap()) + if (nameForRoom(r1) == mostUsedName) + otherNames.insert(mostUsedName, r1); + + mostUsedName = newName; + otherNames.remove(newName); + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + qCDebug(PROFILER) << et.elapsed() + << "ms to switch the most used name"; + } + else + otherNames.insert(newName, r); + } + return { true, oldName }; +} + +const Avatar& User::Private::avatarForRoom(const Room* r) const +{ + for (const auto& p: otherAvatars) + { + auto roomIt = p.second.find(r); + if (roomIt != p.second.end()) + return p.first; + } + return mostUsedAvatar; +} + +bool User::Private::setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl) +{ + if (totalRooms < 2) + { + Q_ASSERT_X(totalRooms > 0 && otherAvatars.empty(), __FUNCTION__, + "Internal structures inconsistency"); + return + exchange(mostUsedAvatar, makeAvatar(avatarUrl)).url() != avatarUrl; + } + for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + { + auto roomIt = it->second.find(r); + if (roomIt != it->second.end()) + { + if (it->first.url() == avatarUrl) + return false; // old url and new url coincide + it->second.erase(r); + if (avatarUrl == mostUsedAvatar.url()) + { + if (it->second.empty()) + otherAvatars.erase(it); + // The most used avatar will be used for this room + return true; + } + } + } + if (avatarUrl != mostUsedAvatar.url()) + { + size_t othersCount = 0; + auto entryToReplace = otherAvatars.end(); + for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + { + othersCount += it->second.size(); + if (it->first.url() == avatarUrl) + entryToReplace = it; + } + // Check if the new avatar is about to become most used. + if (entryToReplace != otherAvatars.end() && + entryToReplace->second.size() >= size_t(totalRooms) - othersCount) + { + QElapsedTimer et; + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + { + qCDebug(MAIN) << "Switching the most used avatar of user" << userId + << "from" << mostUsedAvatar.url().toDisplayString() + << "to" << avatarUrl.toDisplayString(); + et.start(); + } + entryToReplace->first = + exchange(mostUsedAvatar, makeAvatar(avatarUrl)); + entryToReplace->second.clear(); + for (const auto* r1: connection->roomMap()) + { + if (avatarForRoom(r1).url() == mostUsedAvatar.url()) + entryToReplace->second.insert(r1); + } + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) + qCDebug(PROFILER) << et.elapsed() + << "ms to switch the most used avatar"; + } + } + if (avatarUrl != mostUsedAvatar.url()) // It could have changed above + { + // Create a new entry if necessary and add the room to it. + auto it = find_if(otherAvatars.begin(), otherAvatars.end(), + [&avatarUrl] (const auto& p) { + return p.first.url() == avatarUrl; + }); + if (it == otherAvatars.end()) + { + otherAvatars.push_back({ Avatar(avatarUrl, defaultIcon()), {} }); + it = otherAvatars.end() - 1; + } + it->second.insert(r); + } + return true; +} + User::User(QString userId, Connection* connection) : QObject(connection), d(new Private(std::move(userId), connection)) { setObjectName(userId); } -User::~User() -{ - delete d; -} +User::~User() = default; QString User::id() const { @@ -75,34 +248,44 @@ bool User::isGuest() const return *it == ':'; } -QString User::name() const +QString User::name(const Room* room) const { - return d->name; + return d->nameForRoom(room); } -void User::updateName(const QString& newName) +void User::updateName(const QString& newName, const Room* room) { - const auto oldName = name(); - if (oldName != newName) + const auto setNameResult = d->setNameForRoom(room, newName); + if (setNameResult.first) { - d->name = newName; setObjectName(displayname()); - emit nameChanged(newName, oldName); + emit nameChanged(newName, setNameResult.second, room); } } -void User::updateAvatarUrl(const QUrl& newUrl) -{ - if (d->avatar.updateUrl(newUrl)) - emit avatarChanged(this); -} - void User::rename(const QString& newName) { auto job = d->connection->callApi(id(), newName); connect(job, &BaseJob::success, this, [=] { updateName(newName); }); } +void User::rename(const QString& newName, const Room* r) +{ + if (!r) + { + qCWarning(MAIN) << "Passing a null room to two-argument User::rename()" + "is incorrect; client developer, please fix it"; + rename(newName); + } + Q_ASSERT_X(r->memberJoinState(this) == JoinState::Join, __FUNCTION__, + "Attempt to rename a user that's not a room member"); + MemberEventContent evtC; + evtC.displayName = newName; + auto job = d->connection->callApi( + r->id(), id(), RoomMemberEvent(move(evtC))); + connect(job, &BaseJob::success, this, [=] { updateName(newName, r); }); +} + bool User::setAvatar(const QString& fileName) { return avatarObject().upload(d->connection, fileName, @@ -118,18 +301,20 @@ bool User::setAvatar(QIODevice* source) void User::Private::setAvatar(QString contentUri, User* q) { auto* j = connection->callApi(userId, contentUri); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); + connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q, nullptr); }); } -QString User::displayname() const +QString User::displayname(const Room* room) const { - return d->name.isEmpty() ? d->userId : d->name; + auto name = d->nameForRoom(room); + return name.isEmpty() ? d->userId : + room ? room->roomMembername(name) : name; } -QString User::fullName() const +QString User::fullName(const Room* room) const { - return d->name.isEmpty() ? d->userId : - d->name % " (" % d->userId % ')'; + auto name = d->nameForRoom(room); + return name.isEmpty() ? d->userId : name % " (" % d->userId % ')'; } QString User::bridged() const @@ -137,54 +322,70 @@ QString User::bridged() const return d->bridged; } -const Avatar& User::avatarObject() const +const Avatar& User::avatarObject(const Room* room) const { - return d->avatar; + return d->avatarForRoom(room); } -QImage User::avatar(int dimension) +QImage User::avatar(int dimension, const Room* room) { - return avatar(dimension, dimension); + return avatar(dimension, dimension, room); } -QImage User::avatar(int width, int height) +QImage User::avatar(int width, int height, const Room* room) { - return avatar(width, height, []{}); + return avatar(width, height, room, []{}); } -QImage User::avatar(int width, int height, Avatar::get_callback_t callback) +QImage User::avatar(int width, int height, const Room* room, Avatar::get_callback_t callback) { - return avatarObject().get(d->connection, width, height, - [this,callback] { emit avatarChanged(this); callback(); }); + return avatarObject(room).get(d->connection, width, height, + [=] { emit avatarChanged(this, room); callback(); }); } -QString User::avatarMediaId() const +QString User::avatarMediaId(const Room* room) const { - return avatarObject().mediaId(); + return avatarObject(room).mediaId(); } -QUrl User::avatarUrl() const +QUrl User::avatarUrl(const Room* room) const { - return avatarObject().url(); + return avatarObject(room).url(); } -void User::processEvent(Event* event) +void User::processEvent(RoomMemberEvent* event, const Room* room) { - if( event->type() == EventType::RoomMember ) + if (event->membership() != MembershipType::Invite && + event->membership() != MembershipType::Join) + return; + + auto aboutToEnter = room->memberJoinState(this) == JoinState::Leave && + (event->membership() == MembershipType::Join || + event->membership() == MembershipType::Invite); + if (aboutToEnter) + ++d->totalRooms; + + auto newName = event->displayName(); + // `bridged` value uses the same notification signal as the name; + // it is assumed that first setting of the bridge occurs together with + // the first setting of the name, and further bridge updates are + // exceptionally rare (the only reasonable case being that the bridge + // changes the naming convention). For the same reason room-specific + // bridge tags are not supported at all. + QRegularExpression reSuffix(" \\((IRC|Gitter|Telegram)\\)$"); + auto match = reSuffix.match(newName); + if (match.hasMatch()) { - auto e = static_cast(event); - if (e->membership() == MembershipType::Leave) - return; - - auto newName = e->displayName(); - QRegularExpression reSuffix(" \\((IRC|Gitter|Telegram)\\)$"); - auto match = reSuffix.match(newName); - if (match.hasMatch()) + if (d->bridged != match.captured(1)) { + if (!d->bridged.isEmpty()) + qCWarning(MAIN) << "Bridge for user" << id() << "changed:" + << d->bridged << "->" << match.captured(1); d->bridged = match.captured(1); - newName.truncate(match.capturedStart(0)); } - updateName(newName); - updateAvatarUrl(e->avatarUrl()); + newName.truncate(match.capturedStart(0)); } + updateName(event->displayName(), room); + if (d->setAvatarUrlForRoom(room, event->avatarUrl())) + emit avatarChanged(this, room); } -- cgit v1.2.3 From e09c740c85e2e9a861e84f706680ce1ec662a4bc Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 24 Feb 2018 14:43:03 +0900 Subject: User/Room: signal that a user/member is about to change the name Enables to address QMatrixClient/Quaternion#284. Also fixes a gibberish condition in Room::Private::renameMember() that led to improper warnings and a too early return. --- user.cpp | 58 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 25 deletions(-) (limited to 'user.cpp') diff --git a/user.cpp b/user.cpp index 2a3071af..8a9fa515 100644 --- a/user.cpp +++ b/user.cpp @@ -62,8 +62,8 @@ class User::Private mutable int totalRooms = 0; - QString nameForRoom(const Room* r) const; - std::pair setNameForRoom(const Room* r, QString newName); + QString nameForRoom(const Room* r, const QString& hint = {}) const; + void setNameForRoom(const Room* r, QString newName, QString oldName); const Avatar& avatarForRoom(const Room* r) const; bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); @@ -78,34 +78,29 @@ QIcon User::Private::defaultIcon() return icon; } -QString User::Private::nameForRoom(const Room* r) const +QString User::Private::nameForRoom(const Room* r, const QString& hint) const { + // If the hint is accurate, this function is O(1) instead of O(n) + if (hint == mostUsedName || otherNames.contains(hint, r)) + return hint; return otherNames.key(r, mostUsedName); } -static constexpr int MIN_JOINED_ROOMS_TO_LOG = 100; +static constexpr int MIN_JOINED_ROOMS_TO_LOG = 20; -std::pair User::Private::setNameForRoom(const Room* r, - QString newName) +void User::Private::setNameForRoom(const Room* r, QString newName, + QString oldName) { + Q_ASSERT(oldName != newName); + Q_ASSERT(oldName == mostUsedName || otherNames.contains(oldName, r)); if (totalRooms < 2) { Q_ASSERT_X(totalRooms > 0 && otherNames.empty(), __FUNCTION__, "Internal structures inconsistency"); - // The below uses that initialization list evaluation is ordered - return { mostUsedName != newName, - exchange(mostUsedName, move(newName)) }; - } - QString oldName; - // The below works because QMultiHash iterators dereference to stored values - auto it = std::find(otherNames.begin(), otherNames.end(), r); - if (it != otherNames.end()) - { - oldName = it.key(); - if (oldName == newName) - return { false, oldName }; // old name and new name coincide - otherNames.erase(it); + mostUsedName = move(newName); + return; } + otherNames.remove(oldName, r); if (newName != mostUsedName) { // Check if the newName is about to become most used. @@ -134,7 +129,6 @@ std::pair User::Private::setNameForRoom(const Room* r, else otherNames.insert(newName, r); } - return { true, oldName }; } const Avatar& User::Private::avatarForRoom(const Room* r) const @@ -255,11 +249,19 @@ QString User::name(const Room* room) const void User::updateName(const QString& newName, const Room* room) { - const auto setNameResult = d->setNameForRoom(room, newName); - if (setNameResult.first) + updateName(newName, d->nameForRoom(room), room); +} + +void User::updateName(const QString& newName, const QString& oldName, + const Room* room) +{ + Q_ASSERT(oldName == d->mostUsedName || d->otherNames.contains(oldName, room)); + if (newName != oldName) { + emit nameAboutToChange(newName, oldName, room); + d->setNameForRoom(room, newName, oldName); setObjectName(displayname()); - emit nameChanged(newName, setNameResult.second, room); + emit nameChanged(newName, oldName, room); } } @@ -337,7 +339,8 @@ QImage User::avatar(int width, int height, const Room* room) return avatar(width, height, room, []{}); } -QImage User::avatar(int width, int height, const Room* room, Avatar::get_callback_t callback) +QImage User::avatar(int width, int height, const Room* room, + Avatar::get_callback_t callback) { return avatarObject(room).get(d->connection, width, height, [=] { emit avatarChanged(this, room); callback(); }); @@ -385,7 +388,12 @@ void User::processEvent(RoomMemberEvent* event, const Room* room) } newName.truncate(match.capturedStart(0)); } - updateName(event->displayName(), room); + if (event->prevContent()) + updateName(event->displayName(), + d->nameForRoom(room, event->prevContent()->displayName), + room); + else + updateName(event->displayName(), room); if (d->setAvatarUrlForRoom(room, event->avatarUrl())) emit avatarChanged(this, room); } -- cgit v1.2.3 From e10927767faaf7a03a772ab97fe6292907cc4b4b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 24 Feb 2018 17:42:05 +0900 Subject: User: store avatars on the heap; use two containers to store avatars-to-rooms mapping Because they are uncopiable, unlike pointers to them; and a combination of QHash of avatars and a QMultiHash of rooms is much more convenient than a std::vector>. --- user.cpp | 159 ++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 75 insertions(+), 84 deletions(-) (limited to 'user.cpp') diff --git a/user.cpp b/user.cpp index 8a9fa515..9cdbb420 100644 --- a/user.cpp +++ b/user.cpp @@ -37,14 +37,13 @@ #include using namespace QMatrixClient; +using namespace std::placeholders; using std::move; -using std::exchange; class User::Private { public: - static QIcon defaultIcon(); - static Avatar makeAvatar(QUrl url) { return { url, defaultIcon() }; } + static Avatar* makeAvatar(QUrl url); Private(QString userId, Connection* connection) : userId(move(userId)), connection(connection) @@ -55,27 +54,28 @@ class User::Private QString mostUsedName; QString bridged; - Avatar mostUsedAvatar { defaultIcon() }; + const QScopedPointer mostUsedAvatar { makeAvatar({}) }; QMultiHash otherNames; - std::vector>> otherAvatars; + QHash otherAvatars; + QMultiHash avatarsToRooms; mutable int totalRooms = 0; QString nameForRoom(const Room* r, const QString& hint = {}) const; void setNameForRoom(const Room* r, QString newName, QString oldName); - const Avatar& avatarForRoom(const Room* r) const; - bool setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl); + QUrl avatarUrlForRoom(const Room* r, const QUrl& hint = {}) const; + void setAvatarForRoom(const Room* r, const QUrl& newUrl, + const QUrl& oldUrl); - void setAvatar(QString contentUri, User* q); + void setAvatarOnServer(QString contentUri, User* q); }; -QIcon User::Private::defaultIcon() +Avatar* User::Private::makeAvatar(QUrl url) { static const QIcon icon { QIcon::fromTheme(QStringLiteral("user-available")) }; - return icon; + return new Avatar(url, icon); } QString User::Private::nameForRoom(const Room* r, const QString& hint) const @@ -131,97 +131,67 @@ void User::Private::setNameForRoom(const Room* r, QString newName, } } -const Avatar& User::Private::avatarForRoom(const Room* r) const +QUrl User::Private::avatarUrlForRoom(const Room* r, const QUrl& hint) const { - for (const auto& p: otherAvatars) - { - auto roomIt = p.second.find(r); - if (roomIt != p.second.end()) - return p.first; - } - return mostUsedAvatar; + // If the hint is accurate, this function is O(1) instead of O(n) + if (hint == mostUsedAvatar->url() || avatarsToRooms.contains(hint, r)) + return hint; + auto it = std::find(avatarsToRooms.begin(), avatarsToRooms.end(), r); + return it == avatarsToRooms.end() ? mostUsedAvatar->url() : it.key(); } -bool User::Private::setAvatarUrlForRoom(const Room* r, const QUrl& avatarUrl) +void User::Private::setAvatarForRoom(const Room* r, const QUrl& newUrl, + const QUrl& oldUrl) { + Q_ASSERT(oldUrl != newUrl); + Q_ASSERT(oldUrl == mostUsedAvatar->url() || + avatarsToRooms.contains(oldUrl, r)); if (totalRooms < 2) { Q_ASSERT_X(totalRooms > 0 && otherAvatars.empty(), __FUNCTION__, "Internal structures inconsistency"); - return - exchange(mostUsedAvatar, makeAvatar(avatarUrl)).url() != avatarUrl; + mostUsedAvatar->updateUrl(newUrl); + return; } - for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) + avatarsToRooms.remove(oldUrl, r); + if (!avatarsToRooms.contains(oldUrl)) { - auto roomIt = it->second.find(r); - if (roomIt != it->second.end()) - { - if (it->first.url() == avatarUrl) - return false; // old url and new url coincide - it->second.erase(r); - if (avatarUrl == mostUsedAvatar.url()) - { - if (it->second.empty()) - otherAvatars.erase(it); - // The most used avatar will be used for this room - return true; - } - } + delete otherAvatars.value(oldUrl); + otherAvatars.remove(oldUrl); } - if (avatarUrl != mostUsedAvatar.url()) + if (newUrl != mostUsedAvatar->url()) { - size_t othersCount = 0; - auto entryToReplace = otherAvatars.end(); - for (auto it = otherAvatars.begin(); it != otherAvatars.end(); ++it) - { - othersCount += it->second.size(); - if (it->first.url() == avatarUrl) - entryToReplace = it; - } // Check if the new avatar is about to become most used. - if (entryToReplace != otherAvatars.end() && - entryToReplace->second.size() >= size_t(totalRooms) - othersCount) + if (avatarsToRooms.count(newUrl) >= totalRooms - avatarsToRooms.size()) { QElapsedTimer et; if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) { qCDebug(MAIN) << "Switching the most used avatar of user" << userId - << "from" << mostUsedAvatar.url().toDisplayString() - << "to" << avatarUrl.toDisplayString(); + << "from" << mostUsedAvatar->url().toDisplayString() + << "to" << newUrl.toDisplayString(); et.start(); } - entryToReplace->first = - exchange(mostUsedAvatar, makeAvatar(avatarUrl)); - entryToReplace->second.clear(); + avatarsToRooms.remove(newUrl); + auto* nextMostUsed = otherAvatars.take(newUrl); + std::swap(*mostUsedAvatar, *nextMostUsed); + otherAvatars.insert(nextMostUsed->url(), nextMostUsed); for (const auto* r1: connection->roomMap()) - { - if (avatarForRoom(r1).url() == mostUsedAvatar.url()) - entryToReplace->second.insert(r1); - } + if (avatarUrlForRoom(r1) == nextMostUsed->url()) + avatarsToRooms.insert(nextMostUsed->url(), r1); + if (totalRooms > MIN_JOINED_ROOMS_TO_LOG) qCDebug(PROFILER) << et.elapsed() << "ms to switch the most used avatar"; + } else { + otherAvatars.insert(newUrl, makeAvatar(newUrl)); + avatarsToRooms.insert(newUrl, r); } } - if (avatarUrl != mostUsedAvatar.url()) // It could have changed above - { - // Create a new entry if necessary and add the room to it. - auto it = find_if(otherAvatars.begin(), otherAvatars.end(), - [&avatarUrl] (const auto& p) { - return p.first.url() == avatarUrl; - }); - if (it == otherAvatars.end()) - { - otherAvatars.push_back({ Avatar(avatarUrl, defaultIcon()), {} }); - it = otherAvatars.end() - 1; - } - it->second.insert(r); - } - return true; } User::User(QString userId, Connection* connection) - : QObject(connection), d(new Private(std::move(userId), connection)) + : QObject(connection), d(new Private(move(userId), connection)) { setObjectName(userId); } @@ -265,6 +235,20 @@ void User::updateName(const QString& newName, const QString& oldName, } } +void User::updateAvatarUrl(const QUrl& newUrl, const QUrl& oldUrl, + const Room* room) +{ + Q_ASSERT(oldUrl == d->mostUsedAvatar->url() || + d->avatarsToRooms.contains(oldUrl, room)); + if (newUrl != oldUrl) + { + d->setAvatarForRoom(room, newUrl, oldUrl); + setObjectName(displayname()); + emit avatarChanged(this, room); + } + +} + void User::rename(const QString& newName) { auto job = d->connection->callApi(id(), newName); @@ -291,19 +275,20 @@ void User::rename(const QString& newName, const Room* r) bool User::setAvatar(const QString& fileName) { return avatarObject().upload(d->connection, fileName, - [this] (QString contentUri) { d->setAvatar(contentUri, this); }); + std::bind(&Private::setAvatarOnServer, d.data(), _1, this)); } bool User::setAvatar(QIODevice* source) { return avatarObject().upload(d->connection, source, - [this] (QString contentUri) { d->setAvatar(contentUri, this); }); + std::bind(&Private::setAvatarOnServer, d.data(), _1, this)); } -void User::Private::setAvatar(QString contentUri, User* q) +void User::Private::setAvatarOnServer(QString contentUri, User* q) { auto* j = connection->callApi(userId, contentUri); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q, nullptr); }); + connect(j, &BaseJob::success, q, + [=] { q->updateAvatarUrl(contentUri, avatarUrlForRoom(nullptr)); }); } QString User::displayname(const Room* room) const @@ -326,7 +311,8 @@ QString User::bridged() const const Avatar& User::avatarObject(const Room* room) const { - return d->avatarForRoom(room); + return *d->otherAvatars.value(d->avatarUrlForRoom(room), + d->mostUsedAvatar.data()); } QImage User::avatar(int dimension, const Room* room) @@ -389,11 +375,16 @@ void User::processEvent(RoomMemberEvent* event, const Room* room) newName.truncate(match.capturedStart(0)); } if (event->prevContent()) - updateName(event->displayName(), - d->nameForRoom(room, event->prevContent()->displayName), - room); - else + { + // FIXME: the hint doesn't work for bridged users + auto oldNameHint = + d->nameForRoom(room, event->prevContent()->displayName); + updateName(event->displayName(), oldNameHint, room); + updateAvatarUrl(event->avatarUrl(), + d->avatarUrlForRoom(room, event->prevContent()->avatarUrl), + room); + } else { updateName(event->displayName(), room); - if (d->setAvatarUrlForRoom(room, event->avatarUrl())) - emit avatarChanged(this, room); + updateAvatarUrl(event->avatarUrl(), d->avatarUrlForRoom(room), room); + } } -- cgit v1.2.3 From 979756e26af57e715efe64f8de8068243fa27e9f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Feb 2018 11:09:34 +0900 Subject: Deal with memory more carefully Plugs some memory leaks reported by Valgrind. --- user.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'user.cpp') diff --git a/user.cpp b/user.cpp index 9cdbb420..308b217c 100644 --- a/user.cpp +++ b/user.cpp @@ -48,6 +48,11 @@ class User::Private Private(QString userId, Connection* connection) : userId(move(userId)), connection(connection) { } + ~Private() + { + for (auto a: otherAvatars) + delete a; + } QString userId; Connection* connection; -- cgit v1.2.3