From 6c6143f21e02bcf1472d949be1c8e7218daa20d9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 7 Jul 2015 16:56:05 -0700 Subject: [PATCH] remove NetworkPacket and replace with NodePacketPair --- .../src/octree/OctreeSendThread.h | 5 +- interface/src/Application.h | 59 +++++++++---------- libraries/networking/src/NetworkPacket.cpp | 55 ----------------- libraries/networking/src/NetworkPacket.h | 42 ------------- libraries/networking/src/NodeList.h | 2 + libraries/networking/src/PacketSender.cpp | 27 +++++---- libraries/networking/src/PacketSender.h | 5 +- .../src/ReceivedPacketProcessor.cpp | 9 ++- .../networking/src/ReceivedPacketProcessor.h | 6 +- 9 files changed, 59 insertions(+), 151 deletions(-) delete mode 100644 libraries/networking/src/NetworkPacket.cpp delete mode 100644 libraries/networking/src/NetworkPacket.h diff --git a/assignment-client/src/octree/OctreeSendThread.h b/assignment-client/src/octree/OctreeSendThread.h index 3316546913..78f480b5eb 100644 --- a/assignment-client/src/octree/OctreeSendThread.h +++ b/assignment-client/src/octree/OctreeSendThread.h @@ -15,7 +15,6 @@ #define hifi_OctreeSendThread_h #include -#include #include #include "OctreeQueryNode.h" @@ -28,7 +27,7 @@ class OctreeSendThread : public GenericThread { public: OctreeSendThread(OctreeServer* myServer, const SharedNodePointer& node); virtual ~OctreeSendThread(); - + void setIsShuttingDown(); static quint64 _totalBytes; @@ -51,7 +50,7 @@ private: int packetDistributor(OctreeQueryNode* nodeData, bool viewFrustumChanged); OctreePacketData _packetData; - + int _nodeMissingCount; bool _isShuttingDown; }; diff --git a/interface/src/Application.h b/interface/src/Application.h index 9a9b4cdea0..0131fdb19a 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -122,7 +121,7 @@ static const QString INFO_HELP_PATH = "html/interface-welcome.html"; static const QString INFO_EDIT_ENTITIES_PATH = "html/edit-commands.html"; #ifdef Q_OS_WIN -static const UINT UWM_IDENTIFY_INSTANCES = +static const UINT UWM_IDENTIFY_INSTANCES = RegisterWindowMessage("UWM_IDENTIFY_INSTANCES_{8AB82783-B74A-4258-955B-8188C22AA0D6}_" + qgetenv("USERNAME")); static const UINT UWM_SHOW_APPLICATION = RegisterWindowMessage("UWM_SHOW_APPLICATION_{71123FD6-3DA8-4DC1-9C27-8A12A6250CBA}_" + qgetenv("USERNAME")); @@ -199,11 +198,11 @@ public: bool isThrottleRendering() const; Camera* getCamera() { return &_myCamera; } - // Represents the current view frustum of the avatar. + // Represents the current view frustum of the avatar. ViewFrustum* getViewFrustum(); const ViewFrustum* getViewFrustum() const; - // Represents the view frustum of the current rendering pass, - // which might be different from the viewFrustum, i.e. shadowmap + // Represents the view frustum of the current rendering pass, + // which might be different from the viewFrustum, i.e. shadowmap // passes, mirror window passes, etc ViewFrustum* getDisplayViewFrustum(); const ViewFrustum* getDisplayViewFrustum() const; @@ -215,7 +214,7 @@ public: OctreeQuery& getOctreeQuery() { return _octreeQuery; } EntityTree* getEntityClipboard() { return &_entityClipboard; } EntityTreeRenderer* getEntityClipboardRenderer() { return &_entityClipboardRenderer; } - + bool isMousePressed() const { return _mousePressed; } bool isMouseHidden() const { return !_cursorVisible; } const glm::vec3& getMouseRayOrigin() const { return _mouseRayOrigin; } @@ -237,7 +236,7 @@ public: int getTrueMouseDragStartedX() const { return getTrueMouseDragStarted().x; } int getTrueMouseDragStartedY() const { return getTrueMouseDragStarted().y; } bool getLastMouseMoveWasSimulated() const { return _lastMouseMoveWasSimulated; } - + FaceTracker* getActiveFaceTracker(); FaceTracker* getSelectedFaceTracker(); @@ -254,7 +253,7 @@ public: virtual const Transform& getViewTransform() const { return _viewTransform; } void setViewTransform(const Transform& view); - + float getFieldOfView() { return _fieldOfView.get(); } void setFieldOfView(float fov) { _fieldOfView.set(fov); } @@ -319,7 +318,7 @@ public: QStringList getRunningScripts() { return _scriptEnginesHash.keys(); } ScriptEngine* getScriptEngine(QString scriptHash) { return _scriptEnginesHash.contains(scriptHash) ? _scriptEnginesHash[scriptHash] : NULL; } - + bool isLookingAtMyAvatar(Avatar* avatar); float getRenderResolutionScale() const; @@ -344,17 +343,17 @@ public: RunningScriptsWidget* getRunningScriptsWidget() { return _runningScriptsWidget; } Bookmarks* getBookmarks() const { return _bookmarks; } - + QString getScriptsLocation(); void setScriptsLocation(const QString& scriptsLocation); - + void initializeAcceptedFiles(); bool canAcceptURL(const QString& url); bool acceptURL(const QString& url); void setMaxOctreePacketsPerSecond(int maxOctreePPS); int getMaxOctreePacketsPerSecond(); - + render::ScenePointer getMain3DScene() { return _main3DScene; } render::EnginePointer getRenderEngine() { return _renderEngine; } @@ -373,7 +372,7 @@ signals: /// Fired when the import window is closed void importDone(); - + void scriptLocationChanged(const QString& newPath); void svoImportRequested(const QString& url); @@ -384,7 +383,7 @@ signals: void headURLChanged(const QString& newValue, const QString& modelName); void bodyURLChanged(const QString& newValue, const QString& modelName); void fullAvatarURLChanged(const QString& newValue, const QString& modelName); - + void beforeAboutToQuit(); public slots: @@ -408,7 +407,7 @@ public slots: bool askToSetAvatarUrl(const QString& url); bool askToLoadScript(const QString& scriptFilenameOrURL); - ScriptEngine* loadScript(const QString& scriptFilename = QString(), bool isUserLoaded = true, + ScriptEngine* loadScript(const QString& scriptFilename = QString(), bool isUserLoaded = true, bool loadScriptFromEditor = false, bool activateMainWindow = false, bool reload = false); void reloadScript(const QString& scriptName, bool isUserLoaded = true); void scriptFinished(const QString& scriptName); @@ -424,11 +423,11 @@ public slots: void friendsWindowClosed(); void packageModel(); - + void openUrl(const QUrl& url); void updateMyAvatarTransform(); - + void domainSettingsReceived(const QJsonObject& domainSettingsObject); void setVSyncEnabled(); @@ -439,14 +438,14 @@ public slots: void aboutApp(); void showEditEntitiesHelp(); - + void loadSettings(); void saveSettings(); void notifyPacketVersionMismatch(); void domainConnectionDenied(const QString& reason); - + void cameraMenuChanged(); private slots: @@ -454,7 +453,7 @@ private slots: void checkFPS(); void idle(); void aboutToQuit(); - + void handleScriptEngineLoaded(const QString& scriptFilename); void handleScriptLoadError(const QString& scriptFilename); @@ -464,7 +463,7 @@ private slots: void setFullscreen(bool fullscreen); void setEnable3DTVMode(bool enable3DTVMode); void setEnableVRMode(bool enableVRMode); - + void rotationModeChanged(); glm::vec2 getScaledScreenPoint(glm::vec2 projectedPoint); @@ -474,9 +473,9 @@ private slots: void shrinkMirrorView(); void manageRunningScriptsWidgetVisibility(bool shown); - + void runTests(); - + void audioMuteToggled(); void faceTrackerMuteToggled(); @@ -491,7 +490,7 @@ private: void initDisplay(); void init(); - + void cleanupBeforeQuit(); void update(float deltaTime); @@ -528,7 +527,7 @@ private: ToolWindow* _toolWindow; WebWindowClass* _friendsWindow; - + DatagramProcessor* _datagramProcessor; QUndoStack _undoStack; @@ -569,7 +568,7 @@ private: Camera _myCamera; // My view onto the world Camera _mirrorCamera; // Cammera for mirror view QRect _mirrorViewRect; - + Setting::Handle _firstRun; Setting::Handle _previousScriptLocation; Setting::Handle _scriptsLocationHandle; @@ -648,23 +647,23 @@ private: quint64 _lastSendDownstreamAudioStats; bool _isVSyncOn; - + bool _aboutToQuit; Bookmarks* _bookmarks; bool _notifiedPacketVersionMismatchThisDomain; - + QThread _settingsThread; QTimer _settingsTimer; - + GLCanvas* _glWidget = new GLCanvas(); // our GLCanvas has a couple extra features void checkSkeleton(); QWidget* _fullscreenMenuWidget = new QWidget(); int _menuBarHeight; - + QHash _acceptedExtensions; QList _domainConnectionRefusals; diff --git a/libraries/networking/src/NetworkPacket.cpp b/libraries/networking/src/NetworkPacket.cpp deleted file mode 100644 index b2cc9d6751..0000000000 --- a/libraries/networking/src/NetworkPacket.cpp +++ /dev/null @@ -1,55 +0,0 @@ -// -// NetworkPacket.cpp -// libraries/networking/src -// -// Created by Brad Hefta-Gaub on 8/9/13. -// Copyright 2013 High Fidelity, Inc. -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#include -#include -#include - -#include "SharedUtil.h" -#include "NetworkLogging.h" - -#include "NetworkPacket.h" - -NetworkPacket::NetworkPacket(const NetworkPacket& other) : - _node(other._node), - _nlPacket(other._nlPacket) { -} - -NetworkPacket::NetworkPacket(const SharedNodePointer& node, const NLPacket& packet) { - if (packet.getSizeWithHeader() && packet.getSizeWithHeader() <= MAX_PACKET_SIZE) { - _node = node; - _nlPacket = packet; - } else { - qCDebug(networking, ">>> NetworkPacket::copyContents() unexpected length = %d", packet.size()); - } -}; - -// copy assignment -NetworkPacket& NetworkPacket::operator=(NetworkPacket const& other) { - _node = other._node; - _nlPacket = NLPacket::createCopy(other._nlPacket); - return *this; -} - -#ifdef HAS_MOVE_SEMANTICS -// move, same as copy, but other packet won't be used further -NetworkPacket::NetworkPacket(NetworkPacket&& other) : - _node(std::move(other._node)), - _nlPacket(std::move(other._nlPacket)) { -} - -// move assignment -NetworkPacket& NetworkPacket::operator=(NetworkPacket&& other) { - _node = std::move(other._node); - _nlPacket = std::move(other._nlPacket); - return *this; -} -#endif diff --git a/libraries/networking/src/NetworkPacket.h b/libraries/networking/src/NetworkPacket.h deleted file mode 100644 index a4484f31d5..0000000000 --- a/libraries/networking/src/NetworkPacket.h +++ /dev/null @@ -1,42 +0,0 @@ -// -// NetworkPacket.h -// libraries/networking/src -// -// Created by Brad Hefta-Gaub on 8/9/13. -// Copyright 2013 High Fidelity, Inc. -// -// A really simple class that stores a network packet between being received and being processed -// -// Distributed under the Apache License, Version 2.0. -// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html -// - -#ifndef hifi_NetworkPacket_h -#define hifi_NetworkPacket_h - -#include "NodeList.h" - -/// Storage of not-yet processed inbound, or not yet sent outbound generic UDP network packet -class NetworkPacket { -public: - NetworkPacket(); - NetworkPacket(const NetworkPacket& packet); // copy constructor - NetworkPacket& operator= (const NetworkPacket& other); // copy assignment - -#ifdef HAS_MOVE_SEMANTICS - NetworkPacket(NetworkPacket&& other); // move?? // same as copy, but other packet won't be used further - NetworkPacket& operator= (NetworkPacket&& other); // move assignment -#endif - - NetworkPacket(const SharedNodePointer& node, const NLPacket& nlPacket); - - const SharedNodePointer& getNode() const { return _node; } - const NLPacket& getPacket() const { return _nlPacket; } - -private: - - SharedNodePointer _node; - NLPacket _nlPacket; -}; - -#endif // hifi_NetworkPacket_h diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 5e70b45b5f..6858a3f4b4 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -37,6 +37,8 @@ const quint64 DOMAIN_SERVER_CHECK_IN_MSECS = 1 * 1000; const int MAX_SILENT_DOMAIN_SERVER_CHECK_INS = 5; +using std::pair> NodePacketPair; + class Application; class Assignment; diff --git a/libraries/networking/src/PacketSender.cpp b/libraries/networking/src/PacketSender.cpp index 4481cd716c..31104661f6 100644 --- a/libraries/networking/src/PacketSender.cpp +++ b/libraries/networking/src/PacketSender.cpp @@ -48,11 +48,13 @@ PacketSender::~PacketSender() { } -void PacketSender::queuePacketForSending(const SharedNodePointer& destinationNode, const NLPacket& packet) { - NetworkPacket networkPacket(destinationNode, packet); +void PacketSender::queuePacketForSending(const SharedNodePointer& destinationNode, std::unique_ptr packet) { + NodePacketPair networkPacket(destinationNode, packet); + lock(); _packets.push_back(networkPacket); unlock(); + _totalPacketsQueued++; _totalBytesQueued += packet.getSizeWithHeader(); @@ -118,7 +120,7 @@ bool PacketSender::threadedProcess() { // if threaded and we haven't slept? We want to wait for our consumer to signal us with new packets if (!hasSlept) { - // wait till we have packets + // wait till we have packets _waitingOnPacketsMutex.lock(); _hasPackets.wait(&_waitingOnPacketsMutex); _waitingOnPacketsMutex.unlock(); @@ -264,21 +266,24 @@ bool PacketSender::nonThreadedProcess() { // Now that we know how many packets to send this call to process, just send them. while ((packetsSentThisCall < packetsToSendThisCall) && (packetsLeft > 0)) { lock(); - NetworkPacket& packet = _packets.front(); - NetworkPacket temporary = packet; // make a copy - _packets.erase(_packets.begin()); + + NodePacketPair& packetPair = _packets.pop_front(); packetsLeft = _packets.size(); + unlock(); // send the packet through the NodeList... - DependencyManager::get()->sendUnreliablePacket(temporary.getPacket(), temporary.getNode()); + DependencyManager::get()->sendUnreliablePacket(packetPair->second(), packetPair->first()); + packetsSentThisCall++; _packetsOverCheckInterval++; _totalPacketsSent++; - _totalBytesSent += temporary.getPacket().getSizeWithHeader(); - - emit packetSent(temporary.getPacket().getSizeWithHeader()); - + + int packetSize = packetPair->second().getSizeWithHeader(); + + _totalBytesSent += packetSize; + emit packetSent(packetSize); + _lastSendTime = now; } return isStillRunning(); diff --git a/libraries/networking/src/PacketSender.h b/libraries/networking/src/PacketSender.h index 6754da9825..e4792c55c3 100644 --- a/libraries/networking/src/PacketSender.h +++ b/libraries/networking/src/PacketSender.h @@ -17,7 +17,6 @@ #include #include "GenericThread.h" -#include "NetworkPacket.h" #include "NodeList.h" #include "SharedUtil.h" @@ -39,7 +38,7 @@ public: ~PacketSender(); /// Add packet to outbound queue. - void queuePacketForSending(const SharedNodePointer& destinationNode, const NLPacket& packet); + void queuePacketForSending(const SharedNodePointer& destinationNode, std::unique_ptr packet); void setPacketsPerSecond(int packetsPerSecond); int getPacketsPerSecond() const { return _packetsPerSecond; } @@ -100,7 +99,7 @@ protected: SimpleMovingAverage _averageProcessCallTime; private: - std::vector _packets; + std::list _packets; quint64 _lastSendTime; bool threadedProcess(); diff --git a/libraries/networking/src/ReceivedPacketProcessor.cpp b/libraries/networking/src/ReceivedPacketProcessor.cpp index 3c4b32b4ec..dba00fd08a 100644 --- a/libraries/networking/src/ReceivedPacketProcessor.cpp +++ b/libraries/networking/src/ReceivedPacketProcessor.cpp @@ -21,12 +21,14 @@ void ReceivedPacketProcessor::queueReceivedPacket(const SharedNodePointer& sendi // Make sure our Node and NodeList knows we've heard from this node. sendingNode->setLastHeardMicrostamp(usecTimestampNow()); - NetworkPacket networkPacket(sendingNode, packet); + // TODO: fix the NodePacketPair once we've figured out receive API + NodePacketPair networkPacket(sendingNode, NLPacket::create(PacketType::OctreeStats)); + lock(); _packets.push_back(networkPacket); _nodePacketCounts[sendingNode->getUUID()]++; unlock(); - + // Make sure to wake our actual processing thread because we now have packets for it to process. _hasPackets.wakeAll(); } @@ -50,7 +52,8 @@ bool ReceivedPacketProcessor::process() { unlock(); foreach(auto& packet, currentPackets) { - processPacket(packet.getNode(), packet.getByteArray()); + // TODO: Replace QByteArray() once NLPacket is coming through on receive side + processPacket(packet->first(), QByteArray()); midProcess(); } diff --git a/libraries/networking/src/ReceivedPacketProcessor.h b/libraries/networking/src/ReceivedPacketProcessor.h index bcc9f9a1f5..f70c823d2a 100644 --- a/libraries/networking/src/ReceivedPacketProcessor.h +++ b/libraries/networking/src/ReceivedPacketProcessor.h @@ -15,9 +15,8 @@ #include #include "GenericThread.h" -#include "NetworkPacket.h" -/// Generalized threaded processor for handling received inbound packets. +/// Generalized threaded processor for handling received inbound packets. class ReceivedPacketProcessor : public GenericThread { Q_OBJECT public: @@ -74,8 +73,7 @@ protected: virtual void postProcess() { } protected: - - QVector _packets; + std::list _packets; QHash _nodePacketCounts; QWaitCondition _hasPackets;