From d17836685e0bed232db84b75b7fe94d03e21f070 Mon Sep 17 00:00:00 2001 From: "arthur.jamet" Date: Mon, 14 Jun 2021 17:34:06 +0200 Subject: [PATCH] fix memory errors realted to cache --- a | 10 ++++ b | 44 +++++++++++++++++ lib/Ray/sources/Model/ModelAnimation.cpp | 13 ++--- lib/Ray/sources/Model/ModelAnimation.hpp | 4 +- lib/Ray/sources/Model/ModelAnimations.cpp | 2 +- lib/Ray/sources/Utils/Cache.hpp | 48 +++++++++++-------- .../Animation/AnimationsComponent.cpp | 6 +-- .../Animation/AnimationsComponent.hpp | 2 +- sources/Runner/GameScene.cpp | 2 +- 9 files changed, 94 insertions(+), 37 deletions(-) create mode 100644 a create mode 100644 b diff --git a/a b/a new file mode 100644 index 00000000..17048edc --- /dev/null +++ b/a @@ -0,0 +1,10 @@ +WARNING: SHADER: [ID 5] Failed to find shader attribute: vertexTexCoord2 +WARNING: SHADER: [ID 5] Failed to find shader attribute: vertexNormal +WARNING: SHADER: [ID 5] Failed to find shader attribute: vertexTangent +WARNING: SHADER: [ID 5] Failed to find shader attribute: vertexColor +WARNING: SHADER: [ID 5] Failed to find shader uniform: view +WARNING: SHADER: [ID 5] Failed to find shader uniform: projection +WARNING: SHADER: [ID 5] Failed to find shader uniform: matNormal +WARNING: SHADER: [ID 5] Failed to find shader uniform: colDiffuse +WARNING: SHADER: [ID 5] Failed to find shader uniform: texture1 +WARNING: SHADER: [ID 5] Failed to find shader uniform: texture2 diff --git a/b b/b new file mode 100644 index 00000000..97b28710 --- /dev/null +++ b/b @@ -0,0 +1,44 @@ +==6548== Memcheck, a memory error detector +==6548== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. +==6548== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info +==6548== Command: ./build/bomberman +==6548== +==6548== Conditional jump or move depends on uninitialised value(s) +==6548== at 0x541752: RAY::ModelAnimations::ModelAnimations(std::__cxx11::basic_string, std::allocator > const&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x4FD08E: BBM::Runner::createPlayer(WAL::Scene&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x4C9EF6: BBM::LobbySystem::switchToGame(WAL::Wal&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x51918A: BBM::Runner::loadLobbyScene()::{lambda(WAL::Entity&, WAL::Wal&)#3}::operator()(WAL::Entity&, WAL::Wal&) const (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x520084: void std::__invoke_impl(std::__invoke_other, BBM::Runner::loadLobbyScene()::{lambda(WAL::Entity&, WAL::Wal&)#3}&, WAL::Entity&, WAL::Wal&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x51F0A0: std::enable_if, std::enable_if>::type std::__invoke_r(void&&, (BBM::Runner::loadLobbyScene()::{lambda(WAL::Entity&, WAL::Wal&)#3}&)...) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x51E27A: std::_Function_handler::_M_invoke(std::_Any_data const&, WAL::Entity&, WAL::Wal&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x4515E6: std::function::operator()(WAL::Entity&, WAL::Wal&) const (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x451481: WAL::Callback::operator()(WAL::Entity&, WAL::Wal&) const (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x49A22D: BBM::MenuControllableSystem::_updateCurrentButton(bool, BBM::Vector2) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x49A401: BBM::MenuControllableSystem::onFixedUpdate(WAL::ViewEntity&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x49F7A7: WAL::System::fixedUpdate() (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== +==6548== +==6548== Process terminating with default action of signal 1 (SIGHUP) +==6548== at 0x494951: WAL::ViewIterator<__gnu_cxx::__normal_iterator, std::reference_wrapper, std::reference_wrapper >*, std::vector, std::reference_wrapper, std::reference_wrapper >, std::allocator, std::reference_wrapper, std::reference_wrapper > > > >, BBM::PositionComponent, BBM::CollisionComponent>::operator++() (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x4944A2: BBM::CollisionSystem::onFixedUpdate(WAL::ViewEntity&) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x49937B: WAL::System::fixedUpdate() (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x41154E: void WAL::Wal::_run(WAL::Callback const&, BBM::GameState) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x40DA60: void WAL::Wal::run(WAL::Callback const&, BBM::GameState) (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x40871B: BBM::Runner::run() (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== by 0x406F15: main (in /home/arthurjamet/Desktop/B4/YEP/Bomberman/build/bomberman) +==6548== +==6548== HEAP SUMMARY: +==6548== in use at exit: 85,100,138 bytes in 63,937 blocks +==6548== total heap usage: 200,879 allocs, 136,942 frees, 426,519,550 bytes allocated +==6548== +==6548== LEAK SUMMARY: +==6548== definitely lost: 408 bytes in 1 blocks +==6548== indirectly lost: 0 bytes in 0 blocks +==6548== possibly lost: 6,687,735 bytes in 41,704 blocks +==6548== still reachable: 78,411,995 bytes in 22,232 blocks +==6548== suppressed: 0 bytes in 0 blocks +==6548== Rerun with --leak-check=full to see details of leaked memory +==6548== +==6548== Use --track-origins=yes to see where uninitialised values come from +==6548== For lists of detected and suppressed errors, rerun with: -s +==6548== ERROR SUMMARY: 4383708 errors from 1 contexts (suppressed: 0 from 0) diff --git a/lib/Ray/sources/Model/ModelAnimation.cpp b/lib/Ray/sources/Model/ModelAnimation.cpp index c74707a5..af20acb7 100644 --- a/lib/Ray/sources/Model/ModelAnimation.cpp +++ b/lib/Ray/sources/Model/ModelAnimation.cpp @@ -8,7 +8,7 @@ #include #include "Model/ModelAnimation.hpp" -RAY::ModelAnimation::ModelAnimation(::ModelAnimation *animation): +RAY::ModelAnimation::ModelAnimation(::ModelAnimation animation): _animation(animation), _frameCounter(0) { } @@ -20,26 +20,23 @@ size_t RAY::ModelAnimation::getFrameCounter() const size_t RAY::ModelAnimation::getFrameCount() const { - return this->_animation->frameCount; + return this->_animation.frameCount; } RAY::ModelAnimation &RAY::ModelAnimation::setFrameCounter(size_t frameCounter) { - std::cout << this << std::endl; - std::cout << this->_animation << std::endl; - std::cout << this->_animation->frameCount << std::endl; - this->_frameCounter = frameCounter % this->_animation->frameCount; + this->_frameCounter = frameCounter % this->_animation.frameCount; return *this; } RAY::ModelAnimation &RAY::ModelAnimation::incrementFrameCounter() { - this->_frameCounter = (this->_frameCounter + 1) % this->_animation->frameCount; + this->_frameCounter = (this->_frameCounter + 1) % this->_animation.frameCount; return *this; } RAY::ModelAnimation::operator ::ModelAnimation() const { - return *this->_animation; + return this->_animation; } \ No newline at end of file diff --git a/lib/Ray/sources/Model/ModelAnimation.hpp b/lib/Ray/sources/Model/ModelAnimation.hpp index 63de25d6..1fa32ec1 100644 --- a/lib/Ray/sources/Model/ModelAnimation.hpp +++ b/lib/Ray/sources/Model/ModelAnimation.hpp @@ -17,7 +17,7 @@ namespace RAY { public: //! @brief A Model animation constructor //! @param animationPtr an animation pointer, returned by the nimation-loading function - explicit ModelAnimation(::ModelAnimation *animationPtr); + explicit ModelAnimation(::ModelAnimation animation); //! @brief A default copy-constructor ModelAnimation(const ModelAnimation &) = default; @@ -41,7 +41,7 @@ namespace RAY { ~ModelAnimation() = default; private: - ::ModelAnimation *_animation; + ::ModelAnimation _animation; size_t _frameCounter; INTERNAL: diff --git a/lib/Ray/sources/Model/ModelAnimations.cpp b/lib/Ray/sources/Model/ModelAnimations.cpp index 7f1af249..6d41832d 100644 --- a/lib/Ray/sources/Model/ModelAnimations.cpp +++ b/lib/Ray/sources/Model/ModelAnimations.cpp @@ -16,7 +16,7 @@ RAY::ModelAnimations::ModelAnimations(const std::string &filePath): ::ModelAnimation *ptr = this->_animationsPtr.get(); for (int i = 0; i < this->_animationCount; i++) - this->_animations.emplace_back(ptr + i); + this->_animations.emplace_back(ptr[i]); } RAY::ModelAnimation &RAY::ModelAnimations::operator[](int index) diff --git a/lib/Ray/sources/Utils/Cache.hpp b/lib/Ray/sources/Utils/Cache.hpp index 73e6a8b6..fb819d9c 100644 --- a/lib/Ray/sources/Utils/Cache.hpp +++ b/lib/Ray/sources/Utils/Cache.hpp @@ -73,24 +73,6 @@ namespace RAY { template<> class Cache<::ModelAnimation> { - public: - Cache(std::function<::ModelAnimation *(const char *, int *)> dataLoader, std::functiondataUnloader): - _dataLoader(std::move(dataLoader)), _dataUnloader(std::move(dataUnloader)) - {}; - std::shared_ptr<::ModelAnimation> fetch(const std::string &path, int *counter) - { - if (this->_cache.find(path) != this->_cache.end()) - return this->_cache[path]; - - ::ModelAnimation *animations = this->_dataLoader(path.c_str(), counter); - unsigned int animCount = *counter; - - this->_cache.emplace(path, std::shared_ptr<::ModelAnimation>( - animations, [this, animCount](::ModelAnimation *p) { - this->_dataUnloader(p, animCount); - })); - return this->_cache[path]; - }; private: //! @brief function to call to load data std::function<::ModelAnimation *(const char *, int *)> _dataLoader; @@ -98,10 +80,34 @@ namespace RAY { //! @brief function to call when the ray data will be unloaded std::function _dataUnloader; - //! @brief map storing shared ptr of caches - std::unordered_map> _cache; - }; + typedef struct { + std::shared_ptr<::ModelAnimation> animations; + int animationsCount; + } AnimationsHolder; + //! @brief map storing shared ptr of caches + std::unordered_map _cache; + public: + Cache(std::function<::ModelAnimation *(const char *, int *)> dataLoader, std::functiondataUnloader): + _dataLoader(std::move(dataLoader)), _dataUnloader(std::move(dataUnloader)) + {}; + std::shared_ptr<::ModelAnimation> fetch(const std::string &path, int *counter) + { + if (this->_cache.find(path) != this->_cache.end()) { + *counter = this->_cache[path].animationsCount; + } else { + ::ModelAnimation *animations = this->_dataLoader(path.c_str(), counter); + int animCount = *counter; + + this->_cache.emplace(path, (AnimationsHolder){ + std::shared_ptr<::ModelAnimation>( + animations, [this, animCount](::ModelAnimation *p) { + this->_dataUnloader(p, animCount); + }), animCount}); + } + return this->_cache[path].animations; + }; + }; template<> class Cache<::Shader> { diff --git a/sources/Component/Animation/AnimationsComponent.cpp b/sources/Component/Animation/AnimationsComponent.cpp index 4461cc8e..26367a0b 100644 --- a/sources/Component/Animation/AnimationsComponent.cpp +++ b/sources/Component/Animation/AnimationsComponent.cpp @@ -8,9 +8,9 @@ namespace BBM { - AnimationsComponent::AnimationsComponent(WAL::Entity &entity, RAY::ModelAnimations modelAnimation, int animIndex, bool play) + AnimationsComponent::AnimationsComponent(WAL::Entity &entity, const std::string &path, int animIndex, bool play) : WAL::Component(entity), - _modelAnimation(std::move(modelAnimation)), + _modelAnimation(path), _currentAnimIndex(animIndex), _animDisabled(play) { @@ -20,7 +20,7 @@ namespace BBM WAL::Component *AnimationsComponent::clone(WAL::Entity &entity) const { return new AnimationsComponent(entity, - RAY::ModelAnimations(this->_modelAnimation.getFilePath()), + this->_modelAnimation.getFilePath(), this->_currentAnimIndex); } diff --git a/sources/Component/Animation/AnimationsComponent.hpp b/sources/Component/Animation/AnimationsComponent.hpp index 463f241f..36f22a6a 100644 --- a/sources/Component/Animation/AnimationsComponent.hpp +++ b/sources/Component/Animation/AnimationsComponent.hpp @@ -52,7 +52,7 @@ namespace BBM bool isAnimDisabled() const; //! @brief ctor entity and the path of the animation file - explicit AnimationsComponent(WAL::Entity &entity, RAY::ModelAnimations modelAnimation, int animIndex, bool play = true); + explicit AnimationsComponent(WAL::Entity &entity, const std::string &path, int animIndex, bool play = true); //! @brief copy ctor AnimationsComponent(const AnimationsComponent &) = default; //! @brief dtor diff --git a/sources/Runner/GameScene.cpp b/sources/Runner/GameScene.cpp index 110cd6b7..de8c3ccd 100644 --- a/sources/Runner/GameScene.cpp +++ b/sources/Runner/GameScene.cpp @@ -49,7 +49,7 @@ namespace BBM .addComponent() // .addComponent("assets/shaders/glsl330/predator.fs") .addComponent>() -// .addComponent(RAY::ModelAnimations("assets/player/player.iqm"), 3) + .addComponent("assets/player/player.iqm", 3) .addComponent(BBM::Vector3f{0.25, 0, 0.25}, BBM::Vector3f{.75, 2, .75}) .addComponent() .addComponent(soundPath)