From 29538851b61c17a8e8561dd1534942e535f40589 Mon Sep 17 00:00:00 2001 From: amantley Date: Mon, 11 Dec 2017 18:50:07 -0800 Subject: [PATCH 1/2] Made the changes from the latest code review, except getting rid of the while loop. --- .../src/RenderableModelEntityItem.cpp | 4 +-- .../src/RenderableModelEntityItem.h | 1 - .../entities/src/AnimationPropertyGroup.cpp | 9 +++--- libraries/entities/src/ModelEntityItem.cpp | 31 ++++++++++--------- libraries/entities/src/ModelEntityItem.h | 4 +-- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp index 4686cc94bc..e578e4858d 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.cpp @@ -992,7 +992,7 @@ void ModelEntityRenderer::animate(const TypedEntityPointer& entity) { return; } - { + { // the current frame is set on the server in update() in ModelEntityItem.cpp int animationCurrentFrame = (int)(glm::floor(entity->getAnimationCurrentFrame())); @@ -1313,7 +1313,7 @@ void ModelEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& sce } // The code to deal with the change of properties is now in ModelEntityItem.cpp - // That is where _currentFrame and _lastAnimated are updated. + // That is where _currentFrame and _lastAnimated were updated. if (_animating) { DETAILED_PROFILE_RANGE(simulation_physics, "Animate"); if (!jointsMapped()) { diff --git a/libraries/entities-renderer/src/RenderableModelEntityItem.h b/libraries/entities-renderer/src/RenderableModelEntityItem.h index 44ee82713d..b4f2665692 100644 --- a/libraries/entities-renderer/src/RenderableModelEntityItem.h +++ b/libraries/entities-renderer/src/RenderableModelEntityItem.h @@ -184,7 +184,6 @@ private: bool _shouldHighlight { false }; bool _animating { false }; uint64_t _lastAnimated { 0 }; - float _currentFrame { -1 }; }; } } // namespace diff --git a/libraries/entities/src/AnimationPropertyGroup.cpp b/libraries/entities/src/AnimationPropertyGroup.cpp index 932bdbf8c0..2af56fb6b2 100644 --- a/libraries/entities/src/AnimationPropertyGroup.cpp +++ b/libraries/entities/src/AnimationPropertyGroup.cpp @@ -22,24 +22,25 @@ const float AnimationPropertyGroup::MAXIMUM_POSSIBLE_FRAME = 100000.0f; bool operator==(const AnimationPropertyGroup& a, const AnimationPropertyGroup& b) { return - (a._url == b._url) && + (a._currentFrame == b._currentFrame) && (a._running == b._running) && (a._loop == b._loop) && + (a._hold == b._hold) && (a._firstFrame == b._firstFrame) && (a._lastFrame == b._lastFrame) && - (a._hold == b._hold); + (a._url == b._url); } bool operator!=(const AnimationPropertyGroup& a, const AnimationPropertyGroup& b) { return - (a._url != b._url) || (a._currentFrame != b._currentFrame) || (a._running != b._running) || (a._loop != b._loop) || + (a._hold != b._hold) || (a._firstFrame != b._firstFrame) || (a._lastFrame != b._lastFrame) || - (a._hold != b._hold); + (a._url != b._url); } diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index 260074da68..4ade724a82 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -207,11 +207,16 @@ void ModelEntityItem::update(const quint64& now) { // don't reset _lastAnimated here because we need the timestamp from the ModelEntityItem constructor for when the properties were set _currentFrame = currentAnimationProperties.getCurrentFrame(); setAnimationCurrentFrame(_currentFrame); + qCDebug(entities) << "setting first frame 1 " << _currentFrame; } else { - _lastAnimated = usecTimestampNow(); + _lastAnimated = usecTimestampNow(); _currentFrame = currentAnimationProperties.getFirstFrame(); setAnimationCurrentFrame(currentAnimationProperties.getFirstFrame()); + qCDebug(entities) << "setting first frame 2" << _currentFrame; } + } else if (!currentAnimationProperties.getRunning() && _previousAnimationProperties.getRunning()) { + _currentFrame = currentAnimationProperties.getFirstFrame(); + setAnimationCurrentFrame(_currentFrame); } else if (currentAnimationProperties.getCurrentFrame() != _previousAnimationProperties.getCurrentFrame()) { // don't reset _lastAnimated here because the currentFrame was set with the previous setting of _lastAnimated _currentFrame = currentAnimationProperties.getCurrentFrame(); @@ -228,13 +233,15 @@ void ModelEntityItem::update(const quint64& now) { // if the current frame is less than zero then we have restarted the server. if (_currentFrame < 0) { + //qCDebug(entities) << "setting first frame 3 " << _currentFrame; if ((currentAnimationProperties.getCurrentFrame() < currentAnimationProperties.getLastFrame()) && (currentAnimationProperties.getCurrentFrame() > currentAnimationProperties.getFirstFrame())) { - _currentFrame = currentAnimationProperties.getCurrentFrame(); + // _currentFrame = currentAnimationProperties.getCurrentFrame(); } else { - _currentFrame = currentAnimationProperties.getFirstFrame(); - setAnimationCurrentFrame(_currentFrame); - _lastAnimated = usecTimestampNow(); + //qCDebug(entities) << "setting first frame 4 " << _currentFrame; + // _currentFrame = currentAnimationProperties.getFirstFrame(); + // setAnimationCurrentFrame(_currentFrame); + // _lastAnimated = usecTimestampNow(); } } } @@ -269,7 +276,7 @@ void ModelEntityItem::updateFrameCount() { _lastAnimated = now; // if fps is negative then increment timestamp and return. - if (getAnimationFPS() < 0.0) { + if (getAnimationFPS() < 0.0f) { return; } @@ -280,8 +287,9 @@ void ModelEntityItem::updateFrameCount() { _currentFrame += (deltaTime * getAnimationFPS()); if (_currentFrame > getAnimationLastFrame()) { if (getAnimationLoop()) { - while ((_currentFrame - getAnimationFirstFrame()) > (updatedFrameCount - 1)) { - _currentFrame -= (updatedFrameCount - 1); + //_currentFrame = getAnimationFirstFrame() + (int)(glm::floor(_currentFrame - getAnimationFirstFrame())) % (updatedFrameCount - 1); + while (_currentFrame > (getAnimationFirstFrame() + (updatedFrameCount - 1))) { + _currentFrame = _currentFrame - (updatedFrameCount - 1); } } else { _currentFrame = getAnimationLastFrame(); @@ -293,6 +301,7 @@ void ModelEntityItem::updateFrameCount() { _currentFrame = getAnimationFirstFrame(); } } + qCDebug(entities) << "in update frame " << _currentFrame; setAnimationCurrentFrame(_currentFrame); } @@ -720,9 +729,3 @@ bool ModelEntityItem::isAnimatingSomething() const { (_animationProperties.getFPS() != 0.0f); }); } - -int ModelEntityItem::getLastKnownCurrentFrame() const { - return resultWithReadLock([&] { - return _lastKnownCurrentFrame; - }); -} diff --git a/libraries/entities/src/ModelEntityItem.h b/libraries/entities/src/ModelEntityItem.h index c0ca12c7f9..7fee022011 100644 --- a/libraries/entities/src/ModelEntityItem.h +++ b/libraries/entities/src/ModelEntityItem.h @@ -110,8 +110,6 @@ public: float getAnimationFPS() const; bool isAnimatingSomething() const; - int getLastKnownCurrentFrame() const; - static const QString DEFAULT_TEXTURES; const QString getTextures() const; void setTextures(const QString& textures); @@ -172,7 +170,7 @@ protected: private: uint64_t _lastAnimated{ 0 }; AnimationPropertyGroup _previousAnimationProperties; - float _currentFrame{ -1 }; + float _currentFrame{ -1.0f }; }; #endif // hifi_ModelEntityItem_h From 079d9639e477844bdb79f213cafc21334f15f806 Mon Sep 17 00:00:00 2001 From: amantley Date: Tue, 12 Dec 2017 09:12:11 -0800 Subject: [PATCH 2/2] Got rid of the while loop in updateFrameCount in ModelEntityItem_cpp --- libraries/entities/src/ModelEntityItem.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libraries/entities/src/ModelEntityItem.cpp b/libraries/entities/src/ModelEntityItem.cpp index 4ade724a82..3215ab9dd0 100644 --- a/libraries/entities/src/ModelEntityItem.cpp +++ b/libraries/entities/src/ModelEntityItem.cpp @@ -287,10 +287,7 @@ void ModelEntityItem::updateFrameCount() { _currentFrame += (deltaTime * getAnimationFPS()); if (_currentFrame > getAnimationLastFrame()) { if (getAnimationLoop()) { - //_currentFrame = getAnimationFirstFrame() + (int)(glm::floor(_currentFrame - getAnimationFirstFrame())) % (updatedFrameCount - 1); - while (_currentFrame > (getAnimationFirstFrame() + (updatedFrameCount - 1))) { - _currentFrame = _currentFrame - (updatedFrameCount - 1); - } + _currentFrame = getAnimationFirstFrame() + (int)(glm::floor(_currentFrame - getAnimationFirstFrame())) % (updatedFrameCount - 1); } else { _currentFrame = getAnimationLastFrame(); }