Page 1 of 1

[2.1] some Cleanup questions

Posted: Sun May 29, 2016 2:09 pm
by Thyrion
Hi,

what are the cleanup plans for the 2.1 branch?

2.1 Cleanup

Components/Overlay -> replace with imgui? i would try to make a pull request , as an imgui component
Components/Paging -> 1.1 only -> remove it
Components/Property -> who is using it? -> remove it
Components/RTShaderSystem -> 1.1 only? -> remove it
Components/Terrain -> 1.1 only? -> remove it
Components/Volume -> 1.1 only? -> remove it
RenderSystems/Direct3D9 -> 1.1 only? -> remove it
RenderSystems/GLES2 -> in dev?
Plugins/BSPSceneManager -> 1.1 only? -> remove it
Plugins/CgProgramManager -> 1.1 only? -> remove it
Plugins/OctreeSceneManager -> 1.1 only? -> remove it
Plugins/OctreeZone -> 1.1 only? -> remove it
Plugins/PCZSceneManager -> 1.1 only? -> remove it
Plugins/ParticleFX -> add a warning. its working but not ported...

Tools: remove all and add only the 2.1 compatibles one

cmake Buildsystem:
add thirdparty dependecies for download source/click compile and it works ...:
- add SDL2 source
- add Freetype source
- add zlib source
- add Rapidjson source
- set stbimage as default codec not freeimage
- set samples2 as default
- OIS -> 1.1 only? -> remove it
- all old Samples -> 1.1 only? -> remove it
- add folders for windows like, this pullrequests:
-> https://bitbucket.org/sinbad/ogre/commi ... at=default
-> https://bitbucket.org/sinbad/ogre/commi ... at=default
-> https://bitbucket.org/sinbad/ogre/commi ... at=default
- add Cotire as an option?
-> http://www.ogre3d.org/forums/viewtopic.php?f=4&t=84895

Re: [2.1] some Cleanup questions

Posted: Sun May 29, 2016 6:50 pm
by Kohedlo
this is good. but.


there i see some curves in new architecture.

better make one universal renderer then two.

better add dx11 renderer, then add complete new core where 1000000 strokes code need to rewrite.

Re: [2.1] some Cleanup questions

Posted: Sun May 29, 2016 7:12 pm
by Thyrion
Kohedlo wrote:there i see some curves in new architecture.
better make one universal renderer then two.
better add dx11 renderer, then add complete new core where 1000000 strokes code need to rewrite.
i don't understand what you mean? O.o

Re: [2.1] some Cleanup questions

Posted: Mon May 30, 2016 12:18 am
by hydexon
Probably his English is broken as hell. But as i understand he probably mean:
There's some Issues with the new OGRE 2.1 architecture:
  • Make an universal renderer rather using DirectX 11 and OpenGL directly (maybe something similar like BGFX)?
  • An better DirectX11 Rendering Subsystem, maybe rewriten from scratch?
Also i would like to kill the ancient Overlay system to ImGui, is much better and easy to use!.

Re: [2.1] some Cleanup questions

Posted: Mon May 30, 2016 5:32 pm
by Thyrion
hydexon wrote:Probably his English is broken as hell. But as i understand he probably mean:
There's some Issues with the new OGRE 2.1 architecture:
  • Make an universal renderer rather using DirectX 11 and OpenGL directly (maybe something similar like BGFX)?
  • An better DirectX11 Rendering Subsystem, maybe rewriten from scratch?
Also i would like to kill the ancient Overlay system to ImGui, is much better and easy to use!.
Well, that's another topic ...

I think my questions/proposals are relatively simple and good for better usability :)

Re: [2.1] some Cleanup questions

Posted: Mon May 30, 2016 10:28 pm
by dark_sylinc
Regarding cleanup I was thinking this is something to be made when the final release date becomes inminent.
Thyrion wrote:Components/Paging -> 1.1 only -> remove it
Components/RTShaderSystem -> 1.1 only? -> remove it
Components/Terrain -> 1.1 only? -> remove it
Plugins/BSPSceneManager -> 1.1 only? -> remove it
Plugins/CgProgramManager -> 1.1 only? -> remove it
Plugins/OctreeSceneManager -> 1.1 only? -> remove it
Plugins/OctreeZone -> 1.1 only? -> remove it
Plugins/PCZSceneManager -> 1.1 only? -> remove it
Without breaking a sweat: yes.
Thyrion wrote:Components/Volume -> 1.1 only? -> remove it
Components/Property -> who is using it? -> remove it
Dunno about Property. As for Volume: I have never used it but I'm not a "volumetric enthusiast", but other people are really enthusiastic about volumetric rendering. If it's not too hard there may be interest in porting it to 2.1. I don't know how hard it would be I barely seen its code. It would be a shame to remove that code if it can be ported.
Thyrion wrote:Components/Overlay -> replace with imgui? i would try to make a pull request , as an imgui component
Until there is no replacement, it can't be removed.
I'm guessing you're talking about Dear Imgui (it opens to confusion because imgui stands for Immediate Mode GUI, it's like calling processor "CPU" as a brand), imgui is cool for debug stuff; have heard lots of good stuff; but I'm not particularly fond of its default appearance, and Overlay is a retained mode GUI which does have its value too (particularly in multithreading contexts). Dear Imgui doesn't have keyboard/gamepad only traversal either.

When it comes to gui, I like Nuklear, which is also another imgui but with much better appearance, and while it does not have keyboard/gamepad traversal out of the box; there has been people out there who have successfully integrated keyboard/gamepad support with their UIs.
Thyrion wrote:RenderSystems/Direct3D9 -> 1.1 only? -> remove it
RenderSystems/GLES2 -> in dev?
If it were by me D3D9 would be removed, but I'm constant fighting with Asaf about this. He might (0.0001% chance, but it's there) port it to 2.1. Do not have high hopes though. But I wouldn't delete the files yet in case a miracle happen.
GLES2 won't be removed. We have yet to port it.

Tools: remove all and add only the 2.1 compatibles one
add thirdparty dependecies for download source/click compile and it works ...:
- add SDL2 source
- add Freetype source
- add zlib source
- add Rapidjson source
I rather prefer avoiding giving the CMake script a chance to fail for stupid reasons (i.e. it didn't detect Dependencies are already downloaded so it starts downloading again but for some reason the download command failed so now CMake refuses to generate the project even though it has everything it needs). Also if I don't want zlib/Rapidjson/Freetype/SDL, it would waste the user's time (and bandwidth if he's got a data cap).
- set stbimage as default codec not freeimage
No. Stbimage reduces binary size. I don't know if it's faster or slower. But it supports less formats. Not just less extensions, but also less variants. For example it can't open interlaced jpg formats (which are rare but not incredibly rare), and some TGA variants.
I rather default to FreeImage than having to deal with users posting in the forums that "there is a bug" in Ogre because it's not opening their favourite texture.
- set samples2 as default
Probably yes, but first I need to check SDL2 automatic detection works as intended. In fact I think the SDL2.dll is not copied automatically to the bin folder.
- add folders for windows like, this pullrequests:
-> https://bitbucket.org/sinbad/ogre/commi ... at=default
That's a cool one.
That sounds really interesting.

Re: [2.1] some Cleanup questions

Posted: Mon May 30, 2016 10:34 pm
by dark_sylinc
A cleanup of CMake is indeed planned this is an excerpt from skype chat I ranted privately some weeks ago:
Our CMake is a clusterfuck. Particularly dependencies.
On Apple we don't append _d to the libraries.
On Windows it's zziplib_d & zziplib
On Linux it's zziplib
But on Apple it's zzip
WHY? BECAUSE F**K YOU ALL
We're going to need a cleanup to homogenize everything
Oh I forgot that on Linux it's more likely you're using zziplib installed in the system rather than the one in Dependency's folder.
FML

I'd had no problem in replacing our entire CMake scripts with new ones. Old scripts have a lot of of code to workaround bugs CMake had (we were early adopters) and today CMake supports many functions we had to emulate back then.

It would work radically different and that would be welcomed by some members while hated by others.

The main block as of yet is that I did not know how to support all the platforms we currently support. But now iOS and OSX were added to the list.

Re: [2.1] some Cleanup questions

Posted: Tue May 31, 2016 9:07 pm
by Thyrion
dark_sylinc wrote:
Thyrion wrote:Components/Overlay -> replace with imgui? i would try to make a pull request , as an imgui component
Until there is no replacement, it can't be removed.
I'm guessing you're talking about Dear Imgui (it opens to confusion because imgui stands for Immediate Mode GUI, it's like calling processor "CPU" as a brand), imgui is cool for debug stuff; have heard lots of good stuff; but I'm not particularly fond of its default appearance, and Overlay is a retained mode GUI which does have its value too (particularly in multithreading contexts). Dear Imgui doesn't have keyboard/gamepad only traversal either.

When it comes to gui, I like Nuklear, which is also another imgui but with much better appearance, and while it does not have keyboard/gamepad traversal out of the box; there has been people out there who have successfully integrated keyboard/gamepad support with their UIs.
I don't like c and when i have to use it, then only with a wrapper around it :P
My vote goes to imgui.
dark_sylinc wrote:
add thirdparty dependecies for download source/click compile and it works ...:
- add SDL2 source
- add Freetype source
- add zlib source
- add Rapidjson source
I rather prefer avoiding giving the CMake script a chance to fail for stupid reasons (i.e. it didn't detect Dependencies are already downloaded so it starts downloading again but for some reason the download command failed so now CMake refuses to generate the project even though it has everything it needs). Also if I don't want zlib/Rapidjson/Freetype/SDL, it would waste the user's time (and bandwidth if he's got a data cap).
Well, i don't want to "waste the user's time". :)
dark_sylinc wrote:
- add folders for windows like, this pullrequests:
-> https://bitbucket.org/sinbad/ogre/commi ... at=default
That's a cool one.
Is it hard to merge this pullrequests from 1.1 to 2.1?

Re: [2.1] some Cleanup questions

Posted: Fri Jul 08, 2016 8:01 pm
by lingfors
When you clean up the CMake scripts, please PLEASE give the debug build targets the same name as the others (i.e. without the _d suffix).

Re: [2.1] some Cleanup questions

Posted: Fri Jul 08, 2016 8:19 pm
by xrgo
lingfors wrote:When you clean up the CMake scripts, please PLEASE give the debug build targets the same name as the others (i.e. without the _d suffix).
not sure I like that :P
... but I don't care that much to be honest

Re: [2.1] some Cleanup questions

Posted: Sat Jul 09, 2016 6:35 pm
by lingfors
xrgo wrote:
lingfors wrote:When you clean up the CMake scripts, please PLEASE give the debug build targets the same name as the others (i.e. without the _d suffix).
not sure I like that :P
... but I don't care that much to be honest
Why not?

Re: [2.1] some Cleanup questions

Posted: Sat Jul 09, 2016 6:52 pm
by dark_sylinc
Everyone has its own favourite, like Vim vs Emacs vs Visual Studio vs Notepad.

Some scripts are set to automatically append the "_d" suffix. It also helps recognizing Debug/Release on binary executables during troubleshooting.
On the other hand, other scripts are much easier if you assume the name convention stays the same but the libraries are located on a different folder. It also helps with loading plugins and config files (i.e. right now we need to have two plugins.cfg files, one for the debug version, another for the release; avoiding the suffix allows us to keep one shared cfg file for both builds).

It also depends on the tools you're using on the OS you are (e.g. Windows' MSVC gets much easier without the suffix). If you're doing esoteric stuff (like consciously mixing Debug and Release), having the "_d" suffix is better.

So long story short, neither of the forms is best for everyone.

Re: [2.1] some Cleanup questions

Posted: Wed Jul 13, 2016 5:03 pm
by paroj
dark_sylinc wrote:If you're doing esoteric stuff (like consciously mixing Debug and Release), having the "_d" suffix is better.
without "_d" stuff like this would crash in your face if you would try mixing.. which I would consider a good thing.

Re: [2.1] some Cleanup questions

Posted: Tue Jul 26, 2016 7:43 am
by lingfors
paroj wrote:
dark_sylinc wrote:If you're doing esoteric stuff (like consciously mixing Debug and Release), having the "_d" suffix is better.
without "_d" stuff like this would crash in your face if you would try mixing.. which I would consider a good thing.
No, mixing debug and release builds would crash in your face. The _d suffix doesn't prevent you in any way to do that.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 2:24 pm
by zxz
I see that a "fix_debug2" branch has been merged into 2.1, which claims to allow mixing of debug and release.

I don't see how these commits solve the problem though. In the class "IdString", NDEBUG is replaced with OGRE_DEBUG_MODE. However, when the build mode is "debug", OGRE_DEBUG_MODE is defined, which introduces incompatibilities. Also, many uses of NDEBUG remain, which alter the layout of structs and classes. For example in MovableObject (as linked by dark_sylinc).

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 3:27 pm
by dark_sylinc
zxz wrote:I see that a "fix_debug2" branch has been merged into 2.1, which claims to allow mixing of debug and release.
You cannot mix debug and release. Even with regular projects of Visual Studio. The CRT is different, the ABI can be different, the macro definitions are different. You will get linking errors, OS DLL loading errors, or crashes.

The problem here was that NDEBUG was being used by Ogre. And NDEBUG may or may not be defined in Release builds because it only controls whether assert works; meaning the bug was preventing Release DLLs from working with Release EXEs if they miss-matched NDEBUG.
zxz wrote:Also, many uses of NDEBUG remain, which alter the layout of structs and classes. For example in MovableObject (as linked by dark_sylinc).
Mmm... good point. Now that OGRE_DEBUG_MODE can be used instead, I'll fix those up quickly.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 3:40 pm
by zxz
Mixing was working great in previous Ogre releases, at least on Linux (GCC and LLVM). I can't speak for Visual Studio.

Our use case ís this: When we want to debug our framework, we often build our code in debug, while all or most our dependencies are still in release. This really helps with performance during debugging, if the bug is expected to be in our own code.

As long as Ogre's structs are layout-compatible between the build modes, I think it should work again.

In 1.8 there are no occurrences of NDEBUG in OgreMain/include, while there are many occurrences in 2.1. I think these need to be dealt with.

Also, if OGRE_DEBUG_MODE is enabled by default when the build mode is 'debug', there needs to be a way to opt-out. (Edit: this is only necessary in the reverse case (ogre in debug, client in release), which is not common for us).

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 3:59 pm
by dark_sylinc
I believe you are confused.

In Linux, this setting is controlled via CMake (depends on whether CMAKE_BUILD_TYPE is Debug), which will embed the setting in the autogenerated OgreBuildSettings.h.
Regardless of your program being Debug or Release, if your code uses Ogre, it will end up including OgreBuildSettings.h

This way in Linux you can mix a Debug library with a Release program; as long as you (re)compile your program with the right OgreBuildSettings.h file that was used to build Ogre (i.e. you cannot simply swap the .so files).

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 4:05 pm
by zxz
That makes sense. I didn't consider that the setting was written to OgreBuildSettings.h. I was thinking in terms of defines sent on the command line.

ETA: Thanks for fixing the remaining NDEBUG occurrences. I'll try it out.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 4:38 pm
by zxz
A couple of tiny issues remain.

There are a few asserts that access variables which now conditionally exist based on OGRE_DEBUG_MODE instead of NDEBUG. This means that if the client program is built with asserts enabled, while OGRE_DEBUG_MODE is false, this code will fail to compile due to the variables not existing.

I know you prefer a pull-request, but I'll throw this in here for now:

Code: Select all

diff -r e304acaeed34 OgreMain/include/Animation/OgreBone.h
--- a/OgreMain/include/Animation/OgreBone.h	Wed Jan 11 11:11:53 2017 +0100
+++ b/OgreMain/include/Animation/OgreBone.h	Wed Jan 11 16:34:39 2017 +0100
@@ -267,7 +267,9 @@
         */
         FORCEINLINE const SimpleMatrixAf4x3& _getLocalSpaceTransform(void) const
         {
-            assert( !mCachedTransformOutOfDate );
+#if OGRE_DEBUG_MODE
+          assert( !mCachedTransformOutOfDate );
+#endif
             return mTransform.mDerivedTransform[mTransform.mIndex];
         }
 
@@ -285,8 +287,10 @@
         */
         FORCEINLINE const SimpleMatrixAf4x3& _getFullTransform(void) const
         {
+#if OGRE_DEBUG_MODE
             assert( !mCachedTransformOutOfDate &&
                     (!mDebugParentNode || !mDebugParentNode->isCachedTransformOutOfDate()) );
+#endif
             return mTransform.mFinalTransform[mTransform.mIndex];
         }
 
diff -r e304acaeed34 OgreMain/include/OgreNode.h
--- a/OgreMain/include/OgreNode.h	Wed Jan 11 11:11:53 2017 +0100
+++ b/OgreMain/include/OgreNode.h	Wed Jan 11 16:34:39 2017 +0100
@@ -656,7 +656,9 @@
         */
         virtual_l2 FORCEINLINE const Matrix4& _getFullTransform(void) const
         {
+#if OGRE_DEBUG_MODE
             assert( !mCachedTransformOutOfDate );
+#endif
             return mTransform.mDerivedTransform[mTransform.mIndex];
         }
 
With these changes in place, together with your recent commit. I can mix the builds nicely.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 7:07 pm
by dark_sylinc
Thanks. Those are so bizarre. Originally I wanted to implement our own Assert (that would depend on OGRE_DEBUG_MODE) but OgreAssert was already taken and it seemed to consume a lot of time.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 8:00 pm
by paroj
dark_sylinc wrote:Thanks. Those are so bizarre. Originally I wanted to implement our own Assert (that would depend on OGRE_DEBUG_MODE) but OgreAssert was already taken and it seemed to consume a lot of time.
something like this? https://github.com/OGRECave/ogre/pull/3 ... 9586bd558f

or did I misunderstood what you need?

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 8:16 pm
by dark_sylinc
Dammit, OgreDbgAssert is a damn good name :) (considering OgreAssert is already taken)

As for implementation, no. I meant something like this:
http://cnicholson.net/2009/02/stupid-c- ... in-assert/

aka roll our own assert that does not depend on NDEBUG.

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 8:57 pm
by paroj
dark_sylinc wrote:Dammit, OgreDbgAssert is a damn good name :) (considering OgreAssert is already taken)

As for implementation, no. I meant something like this:
http://cnicholson.net/2009/02/stupid-c- ... in-assert/

aka roll our own assert that does not depend on NDEBUG.
why not just throw?

Re: [2.1] some Cleanup questions

Posted: Wed Jan 11, 2017 9:09 pm
by dark_sylinc
paroj wrote:why not just throw?
Because throws cannot be faithfully caught by the debugger at the place of the error if the code actually catches the exception. The debugger could be set to break on exceptions thrown, but that may also mean the debugger may step a lot if there's a lot of trivial exceptions being thrown that are meant to be caught; which hinders developer productivity.

Furthermore it's not only about what doing to break into the debugger, but also about getting the macro right.