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.

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

Fix error: member access into incomplete type 'SceneUtil::UnrefWorkItem' #1887

Conversation

pineapplemachine
Copy link
Contributor

Fixes compile error encountered on OSX 10.9 using g++

The compilation error:

[ 24%] Building CXX object apps/openmw/CMakeFiles/openmw.dir/mwrender/renderingmanager.cpp.o
In file included from /Users/pineapple/git/openmw/apps/openmw/mwrender/renderingmanager.cpp:1:
In file included from /Users/pineapple/git/openmw/apps/openmw/mwrender/renderingmanager.hpp:4:
/Users/pineapple/git/openmw/openmw-deps/include/osg/ref_ptr:35:36: error: member access into incomplete type 'SceneUtil::UnrefWorkItem'
        ~ref_ptr() { if (_ptr) _ptr->unref();  _ptr = 0; }
                                   ^
/Users/pineapple/git/openmw/./components/sceneutil/unrefqueue.hpp:14:11: note: in instantiation of member function
      'osg::ref_ptr<SceneUtil::UnrefWorkItem>::~ref_ptr' requested here
    class UnrefQueue : public osg::Referenced
          ^
/Users/pineapple/git/openmw/./components/sceneutil/unrefqueue.hpp:10:11: note: forward declaration of 'SceneUtil::UnrefWorkItem'
    class UnrefWorkItem;

The fix:

Move SceneUtil::UnrefWorkItem class definition out of renderingmanager.cpp and into renderingmanager.hpp.

Compiler info

sophie:build pineapple$ g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin13.4.0
Thread model: posix

Fixes compile error encountered on OSX 10.9 with g++

sophie:build pineapple$ g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin13.4.0
Thread model: posix

The compilation error:

[ 24%] Building CXX object apps/openmw/CMakeFiles/openmw.dir/mwrender/renderingmanager.cpp.o
In file included from /Users/pineapple/git/openmw/apps/openmw/mwrender/renderingmanager.cpp:1:
In file included from /Users/pineapple/git/openmw/apps/openmw/mwrender/renderingmanager.hpp:4:
/Users/pineapple/git/openmw/openmw-deps/include/osg/ref_ptr:35:36: error: member access into incomplete type 'SceneUtil::UnrefWorkItem'
        ~ref_ptr() { if (_ptr) _ptr->unref();  _ptr = 0; }
                                   ^
/Users/pineapple/git/openmw/./components/sceneutil/unrefqueue.hpp:14:11: note: in instantiation of member function
      'osg::ref_ptr<SceneUtil::UnrefWorkItem>::~ref_ptr' requested here
    class UnrefQueue : public osg::Referenced
          ^
/Users/pineapple/git/openmw/./components/sceneutil/unrefqueue.hpp:10:11: note: forward declaration of 'SceneUtil::UnrefWorkItem'
    class UnrefWorkItem;
//osg::Timer timer;
//size_t objcount = mObjects.size();
mObjects.clear();
//Log(Debug::Verbose) << "cleared " << objcount << " objects in " << timer.time_m();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh. You probably can remove these debugging leftovers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a blocking change?

It being my first PR, I was going for as low-impact as possible...

Copy link
Contributor

Choose a reason for hiding this comment

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

We generally do not encourage pure code cleanup changes, because they increase the chance of merge conflicts. However if you are already touching lines of code that need cleanup, then you should do the cleanup.

There shouldn't be any code that is commented out. That's what we got git for. Usually we catch that in the code review, but sometimes it can slip through.

Just add a new commit to this PR that removes the comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it - I committed the change

@pineapplemachine
Copy link
Contributor Author

See related issue https://gitlab.com/OpenMW/openmw/issues/4613

@@ -1,28 +1,15 @@
#include "unrefqueue.hpp"

#include <deque>

//#include <osg/Timer>

//#include <components/debug/debuglog.hpp>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The commented includes are unnecessary here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed, but zinnschlag commented

if you are already touching lines of code that need cleanup, then you should do the cleanup.

Since I wasn't already touching those lines I left them where they are

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove those too if it's blocking, I guess?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not blocking since you did not touch these lines.

@zinnschlag zinnschlag merged commit c412f99 into OpenMW:master Aug 27, 2018
zinnschlag added a commit that referenced this pull request Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants