Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve: speedup viewing the 2D map #7189

Draft
wants to merge 14 commits into
base: development
Choose a base branch
from
Draft

Improve: speedup viewing the 2D map #7189

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2402a77
Performance improvements so far
vadi2 Mar 17, 2024
2e0976e
Re-use pens for drawing rooms
vadi2 Mar 17, 2024
cb47083
Remove another pen
vadi2 Mar 17, 2024
44c6459
Don't set pen to transparent when it'll be overridden later
vadi2 Mar 17, 2024
042f373
Remove painter save/restore
vadi2 Mar 17, 2024
7776ae0
Disable benchmarking code
vadi2 Mar 17, 2024
e09e127
Don't include nanobench, qmake breaks
vadi2 Mar 17, 2024
a996558
Re-use brushes
vadi2 Mar 19, 2024
36f5de2
Add border width for pen
vadi2 Mar 21, 2024
1347d87
Merge branch 'development' into speedup-map-panning
vadi2 Mar 29, 2024
5e84fc7
Code cleanup
vadi2 Mar 29, 2024
0bf8ed9
Final cleanup
vadi2 Mar 29, 2024
865b8d4
Update src/CMakeLists.txt
vadi2 Apr 6, 2024
39eaf92
Merge branch 'development' into speedup-map-panning
vadi2 May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
View file
Open in desktop
Expand Up @@ -544,7 +544,7 @@ target_link_libraries(
ZIP::ZIP
ZLIB::ZLIB
# PLACEMARKER: sample benchmarking code
# nanobench
# nanobench
vadi2 marked this conversation as resolved.
Show resolved Hide resolved
${QT_LIBS})

if(USE_UPDATER)
Expand Down
106 changes: 56 additions & 50 deletions src/T2DMap.cpp
View file
Open in desktop
Expand Up @@ -44,6 +44,7 @@
#include "pre_guard.h"
#include <QtEvents>
#include <QtUiTools>
#include <QDebug>
#include "post_guard.h"

#include "mapInfoContributorManager.h"
Expand Down Expand Up @@ -583,25 +584,22 @@ void T2DMap::initiateSpeedWalk(const int speedWalkStartRoomId, const int speedWa
}
}

// This has been refactored to a separate function out of the paintEven() code
// because we need to use it in two places - one for every room that is not the
// player's room and then, AFTER all those have been drawn, once for the
// player's room if it is visible. This is so it is drawn LAST (and any effects,
// or extra markings for it do not get overwritten by the drawing of the other
// rooms)...
Comment on lines -586 to -591
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vadi2 You think this is unhelpful then?

inline void T2DMap::drawRoom(QPainter& painter,
QFont& roomVNumFont,
QFont& mapNameFont,
QPen& pen,
QPen& innerPen,
QPen& roomBorderPen,
QBrush& brush,
QBrush& innerBrush,
TRoom* pRoom,
const bool isGridMode,
const bool areRoomIdsLegible,
const bool showRoomName,
const int speedWalkStartRoomId,
const float rx,
const float ry,
const QMap<int, QPointF>& areaExitsMap)
{
const QMap<int, QPointF>& areaExitsMap) {
const int currentRoomId = pRoom->getId();
pRoom->rendered = false;
QRectF roomRectangle;
Expand Down Expand Up @@ -687,37 +685,32 @@ inline void T2DMap::drawRoom(QPainter& painter,
}
}

const bool isRoomSelected = (mPick && roomClickTestRectangle.contains(mPHighlight)) || mMultiSelectionSet.contains(currentRoomId);
QLinearGradient selectionBg(roomRectangle.topLeft(), roomRectangle.bottomRight());
selectionBg.setColorAt(0.25, roomColor);
selectionBg.setColorAt(1, Qt::blue);
const bool roomSelected = (mPick && roomClickTestRectangle.contains(mPHighlight)) || mMultiSelectionSet.contains(currentRoomId);

QPen roomPen(Qt::transparent);
roomPen.setWidth(borderWidth);
painter.setBrush(roomColor);

if (shouldDrawBorder && mRoomWidth >= 12) {
roomPen.setColor(mpHost->mRoomBorderColor);
pen.setColor(mpHost->mRoomBorderColor);
} else if (shouldDrawBorder) {
auto fadingColor = QColor(mpHost->mRoomBorderColor);
fadingColor.setAlpha(255 * (mRoomWidth / 12));
roomPen.setColor(fadingColor);
}

if (isRoomSelected) {
pen.setColor(fadingColor);
} else if (roomSelected) {
QLinearGradient selectionBg(roomRectangle.topLeft(), roomRectangle.bottomRight());
selectionBg.setColorAt(0.2, roomColor);
selectionBg.setColorAt(1, Qt::blue);
roomPen.setColor(QColor(255, 50, 50));
pen.setColor(QColor(255, 50, 50));
painter.setBrush(selectionBg);
} else {
pen.setColor(Qt::transparent);
}
Comment on lines 692 to 706
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the logic has stayed the same here - it looks as though you are combining the boarder drawing with the room selection drawing - and I'm not entirely sure that is the case. OTOH I didn't do the room borders so I might be missing something...

😕


painter.setPen(roomPen);
painter.setPen(pen);

if (mBubbleMode) {
const float roomRadius = 0.5 * rSize * mRoomWidth;
const QPointF roomCenter = QPointF(rx, ry);
if (!isRoomSelected) {
if (!roomSelected) {
// CHECK: The use of a gradient fill to a white center on round
// rooms might look nice in some situations but not in all:
QRadialGradient gradient(roomCenter, roomRadius);
Expand All @@ -732,7 +725,7 @@ inline void T2DMap::drawRoom(QPainter& painter,
painter.drawRect(roomRectangle);
}

if (isRoomSelected) {
if (roomSelected) {
mPick = false;
if (mStartSpeedWalk) {
mStartSpeedWalk = false;
Expand All @@ -751,10 +744,9 @@ inline void T2DMap::drawRoom(QPainter& painter,
gradient.setColorAt(0.799, QColor(150, 100, 100, 100));
gradient.setColorAt(0.7, QColor(255, 0, 0, 200));
gradient.setColorAt(0, Qt::white);
const QPen transparentPen(Qt::transparent);
pen.setColor(Qt::transparent);
QPainterPath diameterPath;
painter.setBrush(gradient);
painter.setPen(transparentPen);
diameterPath.addEllipse(roomCenter, roomRadius, roomRadius);
painter.drawPath(diameterPath);

Expand Down Expand Up @@ -820,10 +812,9 @@ inline void T2DMap::drawRoom(QPainter& painter,
QRadialGradient gradient(roomCenter, roomRadius);
gradient.setColorAt(0.85, pRoom->highlightColor);
gradient.setColorAt(0, pRoom->highlightColor2);
const QPen transparentPen(Qt::transparent);
pen.setColor(Qt::transparent);
QPainterPath diameterPath;
painter.setBrush(gradient);
painter.setPen(transparentPen);
diameterPath.addEllipse(roomCenter, roomRadius, roomRadius);
painter.drawPath(diameterPath);
}
Expand All @@ -837,7 +828,7 @@ inline void T2DMap::drawRoom(QPainter& painter,
} else {
roomIdColor = QColor(Qt::white);
}
painter.setPen(QPen(roomIdColor));
pen.setColor(roomIdColor);
painter.setFont(roomVNumFont);
painter.drawText(roomRectangle, Qt::AlignCenter, QString::number(currentRoomId));
painter.restore();
Expand Down Expand Up @@ -878,7 +869,7 @@ inline void T2DMap::drawRoom(QPainter& painter,
}
auto roomNameColor = QColor((mpHost->mBgColor_2.lightness() > 127)
? Qt::black : Qt::white);
painter.setPen(QPen(roomNameColor));
pen.setColor(roomNameColor);
painter.setFont(mapNameFont);
painter.drawText(roomNameRectangle, Qt::AlignCenter, pRoom->name);
painter.restore();
Expand All @@ -903,23 +894,15 @@ inline void T2DMap::drawRoom(QPainter& painter,
} else {
lc = QColorConstants::White;
}
pen = painter.pen();
pen.setColor(lc);
pen.setCosmetic(mMapperUseAntiAlias);
pen.setCapStyle(Qt::RoundCap);
pen.setJoinStyle(Qt::RoundJoin);
QPen innerPen = pen;
painter.save();

QBrush innerBrush = painter.brush();
innerBrush.setStyle(Qt::NoBrush);
if (pRoom->getUp() > 0 || pRoom->exitStubs.contains(DIR_UP)) {
QPolygonF poly_up;
poly_up.append(QPointF(rx, ry + (mRoomHeight * rSize * allInsideTipOffsetFactor)));
poly_up.append(QPointF(rx - (mRoomWidth * rSize * upDownXOrYFactor), ry + (mRoomHeight * rSize * upDownXOrYFactor)));
poly_up.append(QPointF(rx + (mRoomWidth * rSize * upDownXOrYFactor), ry + (mRoomHeight * rSize * upDownXOrYFactor)));
bool isDoor = true;
QBrush brush = painter.brush();
switch (pRoom->doors.value(key_up)) {
case 1:
brush.setColor(mOpenDoorColor);
Expand All @@ -946,7 +929,6 @@ inline void T2DMap::drawRoom(QPainter& painter,
innerPen.setWidthF(mRoomWidth * rSize * innerStubDoorPenThicknessFactor);
brush.setStyle(Qt::DiagCrossPattern);
}
painter.setPen(pen);
painter.setBrush(brush);
painter.drawPolygon(poly_up);
if (isDoor) {
Expand All @@ -965,7 +947,6 @@ inline void T2DMap::drawRoom(QPainter& painter,
poly_down.append(QPointF(rx - (mRoomWidth * rSize * upDownXOrYFactor), ry - (mRoomHeight * rSize * upDownXOrYFactor)));
poly_down.append(QPointF(rx + (mRoomWidth * rSize * upDownXOrYFactor), ry - (mRoomHeight * rSize * upDownXOrYFactor)));
bool isDoor = true;
QBrush brush = painter.brush();
switch (pRoom->doors.value(key_down)) {
case 1:
brush.setColor(mOpenDoorColor);
Expand Down Expand Up @@ -1012,7 +993,6 @@ inline void T2DMap::drawRoom(QPainter& painter,
poly_in_right.append(QPointF(rx + (mRoomWidth * rSize * inOuterXFactor), ry + (mRoomHeight * rSize * inUpDownYFactor)));
poly_in_right.append(QPointF(rx + (mRoomWidth * rSize * inOuterXFactor), ry - (mRoomHeight * rSize * inUpDownYFactor)));
bool isDoor = true;
QBrush brush = painter.brush();
switch (pRoom->doors.value(key_in)) {
case 1:
brush.setColor(mOpenDoorColor);
Expand Down Expand Up @@ -1061,7 +1041,6 @@ inline void T2DMap::drawRoom(QPainter& painter,
poly_out_right.append(QPointF(rx + (mRoomWidth * rSize * outInterXFactor), ry + (mRoomHeight * rSize * outUpDownYFactor)));
poly_out_right.append(QPointF(rx + (mRoomWidth * rSize * outInterXFactor), ry - (mRoomHeight * rSize * outUpDownYFactor)));
bool isDoor = true;
QBrush brush = painter.brush();
switch (pRoom->doors.value(key_out)) {
case 1:
brush.setColor(mOpenDoorColor);
Expand Down Expand Up @@ -1100,7 +1079,6 @@ inline void T2DMap::drawRoom(QPainter& painter,
}
}

painter.restore();
if (!isGridMode) {
QMapIterator<int, QPointF> it(areaExitsMap);
while (it.hasNext()) {
Expand Down Expand Up @@ -1400,6 +1378,7 @@ void T2DMap::paintEvent(QPaintEvent* e)
paintRoomExits(painter, pen, exitList, oneWayExits, pDrawnArea, zLevel, exitWidth, areaExitsMap);
}


// Draw label sizing or group selection box
if (mSizeLabel) {
painter.fillRect(mMultiRect, QColor(250, 190, 0, 190));
Expand All @@ -1409,6 +1388,32 @@ void T2DMap::paintEvent(QPaintEvent* e)

QPointF playerRoomOnWidgetCoordinates;
bool isPlayerRoomVisible = false;

// keep a few brushes and pens around for drawing all rooms in bulk
// re-creating the objects on a million-room map is costly, as is changing their properties
QBrush brush, innerBrush;

QPen roomPen = pen;
roomPen.setCosmetic(mMapperUseAntiAlias);
roomPen.setCapStyle(Qt::RoundCap);
roomPen.setJoinStyle(Qt::RoundJoin);
QPen innerRoomPen = roomPen;

const bool shouldDrawBorder = mpHost->mMapperShowRoomBorders && !pDrawnArea->gridMode;
const int borderWidth = 1 / eSize * mRoomWidth * rSize;
QPen roomBorderPen = roomPen;

if (shouldDrawBorder && mRoomWidth >= 12) {
roomBorderPen.setColor(mpHost->mRoomBorderColor);
} else if (shouldDrawBorder) {
auto fadingColor = QColor(mpHost->mRoomBorderColor);
fadingColor.setAlpha(255 * (mRoomWidth / 12));
roomBorderPen.setColor(fadingColor);
}
roomBorderPen.setWidth(borderWidth);



// Draw the rooms:
QSetIterator<int> itRoom(pDrawnArea->getAreaRooms());
while (itRoom.hasNext()) {
Expand All @@ -1434,12 +1439,13 @@ void T2DMap::paintEvent(QPaintEvent* e)
playerRoomOnWidgetCoordinates = QPointF(static_cast<qreal>(rx), static_cast<qreal>(ry));
} else {
// Not the player's room:
drawRoom(painter, roomVNumFont, mapNameFont, pen, room, pDrawnArea->gridMode, isFontBigEnoughToShowRoomVnum, showRoomNames, playerRoomId, rx, ry, areaExitsMap);
drawRoom(painter, roomVNumFont, mapNameFont, roomPen, innerRoomPen, roomBorderPen, brush, innerBrush, room, pDrawnArea->gridMode, isFontBigEnoughToShowRoomVnum, showRoomNames, playerRoomId, rx, ry, areaExitsMap);
}
} // End of while loop for each room in area

if (isPlayerRoomVisible) {
drawRoom(painter, roomVNumFont, mapNameFont, pen, pPlayerRoom, pDrawnArea->gridMode, isFontBigEnoughToShowRoomVnum, showRoomNames, playerRoomId, static_cast<float>(playerRoomOnWidgetCoordinates.x()), static_cast<float>(playerRoomOnWidgetCoordinates.y()), areaExitsMap);
drawRoom(painter, roomVNumFont, mapNameFont, roomPen, innerRoomPen, roomBorderPen, brush, innerBrush, pPlayerRoom, pDrawnArea->gridMode, isFontBigEnoughToShowRoomVnum, showRoomNames, playerRoomId, static_cast<float>(playerRoomOnWidgetCoordinates.x()), static_cast<float>(playerRoomOnWidgetCoordinates.y()), areaExitsMap);

painter.save();
const QPen transparentPen(Qt::transparent);
QPainterPath myPath;
Expand Down Expand Up @@ -1611,7 +1617,7 @@ void T2DMap::paintEvent(QPaintEvent* e)
// the "exitLine". Various features of the QPen that is used are redefined
// as appropriate - but they are restored afterwards so there should be
// no change to the QPainter as a result of calling this method.
void T2DMap::drawDoor(QPainter& painter, const TRoom& room, const QString& dirKey, const QLineF& exitLine)
void T2DMap::drawDoor(QPainter& painter, const TRoom& room, const QString& dirKey, const QLineF& exitLine) const
{
// A set of numbers that can be converted to "static" type and be frobbed
// during development:
Expand Down Expand Up @@ -1678,7 +1684,7 @@ void T2DMap::drawDoor(QPainter& painter, const TRoom& room, const QString& dirKe
painter.restore();
}

void T2DMap::paintRoomExits(QPainter& painter, QPen& pen, QList<int>& exitList, QList<int>& oneWayExits, const TArea* pArea, int zLevel, float exitWidth, QMap<int, QPointF>& areaExitsMap)
void T2DMap::paintRoomExits(QPainter& painter, QPen& pen, QList<int>& exitList, QList<int>& oneWayExits, const TArea* pArea, int zLevel, float exitWidth, QMap<int, QPointF>& areaExitsMap) const
{
const float exitArrowScale = (mLargeAreaExitArrows ? 2.0f : 1.0f);
const float widgetWidth = width();
Expand Down Expand Up @@ -1719,9 +1725,9 @@ void T2DMap::paintRoomExits(QPainter& painter, QPen& pen, QList<int>& exitList,
}
}
}
QSetIterator<int> itRoom2(pArea->getAreaRooms());
while (itRoom2.hasNext()) {
SlySven marked this conversation as resolved.
Show resolved Hide resolved
const int _id = itRoom2.next();
const auto& areaRooms = pArea->getAreaRooms();
for (auto i = areaRooms.cbegin(), end = areaRooms.cend(); i != end; ++i) {
const int _id = *i;
TRoom* room = mpMap->mpRoomDB->getRoom(_id);
if (!room) {
continue;
Expand Down Expand Up @@ -2093,7 +2099,7 @@ void T2DMap::paintRoomExits(QPainter& painter, QPen& pen, QList<int>& exitList,
}
}

for (const int& k : exitList) {
for (const int& k : qAsConst(exitList)) {
SlySven marked this conversation as resolved.
Show resolved Hide resolved
const int rID = k;
if (rID <= 0) {
continue;
Expand Down
6 changes: 3 additions & 3 deletions src/T2DMap.h
View file
Open in desktop
Expand Up @@ -254,12 +254,12 @@ public slots:
std::pair<int, int> getMousePosition();
bool checkButtonIsForGivenDirection(const QPushButton*, const QString&, const int&);
bool sizeFontToFitTextInRect(QFont&, const QRectF&, const QString&, const quint8 percentageMargin = 10, const qreal minFontSize = 7.0);
void drawRoom(QPainter&, QFont&, QFont&, QPen&, TRoom*, const bool isGridMode, const bool areRoomIdsLegible, const bool showRoomNames, const int, const float, const float, const QMap<int, QPointF>&);
void drawRoom(QPainter&, QFont&, QFont&, QPen&, QPen&, QPen&, QBrush&, QBrush&, TRoom*, const bool isGridMode, const bool areRoomIdsLegible, const bool showRoomNames, const int, const float, const float, const QMap<int, QPointF>&);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Can you nano bench this to see if an inline (effectively cutting out one method call in the room interation) improves things at all?

void paintMapInfo(const QElapsedTimer& renderTimer, QPainter& painter, const int displayAreaId, QColor& infoColor);
int paintMapInfoContributor(QPainter&, int xOffset, int yOffset, const MapInfoProperties& properties);
void paintRoomExits(QPainter&, QPen&, QList<int>& exitList, QList<int>& oneWayExits, const TArea*, int, float, QMap<int, QPointF>&);
void paintRoomExits(QPainter&, QPen&, QList<int>& exitList, QList<int>& oneWayExits, const TArea*, int, float, QMap<int, QPointF>&) const;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Like drawRoom(...) (and drawDoor(...) this might produce an improvement if it were inlined - if the compiler doesn't anyhow - but it might need testing to confim.

void initiateSpeedWalk(const int speedWalkStartRoomId, const int speedWalkTargetRoomId);
inline void drawDoor(QPainter&, const TRoom&, const QString&, const QLineF&);
inline void drawDoor(QPainter&, const TRoom&, const QString&, const QLineF&) const;
void updateMapLabel(QRectF labelRectangle, int labelId, TArea* pArea);

bool mDialogLock = false;
Expand Down
6 changes: 1 addition & 5 deletions src/TRoomDB.cpp
View file
Open in desktop
Expand Up @@ -51,11 +51,7 @@ TRoom* TRoomDB::getRoom(int id)
if (id < 0) {
return nullptr;
}
const auto i = rooms.constFind(id);
if (i != rooms.constEnd() && i.key() == id) {
return i.value();
}
return nullptr;
return rooms.value(id, nullptr);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qt's own implementation was quicker.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second argument is unneeded in this usage - https://doc.qt.io/qt-5/qhash.html#value :

Suggested change
return rooms.value(id, nullptr);
return rooms.value(id);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen that link and I am unsure why you think this. The default is not a null pointer but a default-constructed value.

Copy link
Member

@SlySven SlySven Apr 6, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A default constructed value in this case is a nullptr! Remember that the (QHash<K,V>) rooms has a <int, TRoom*> type...

}

bool TRoomDB::addRoom(int id)
Expand Down