Support for external main loop #391

hhyyrylainen · 2019-08-19T17:22:55Z

This is a basic alternative class to Application with support for external main loop (https://discourse.bsframework.io/t/using-bf-framework-as-a-graphics-library/432?u=hhyyrylainen). This isn't yet in the best shape, so I'm making this a draft.

This basically has two issues:

There's code duplication in the frame function. Also probably some style issues etc.
I get crashes on shutdown. Probably I need something more as I call waitUntilFrameFinished before shutdown, but it still crashes often on shutdown (not always, though). The crash handler output is posted below.

And one extra feature I'd like (but I'm unsure how it should be done):

Ability to not use bs::CrashHandler, instead having a callback for doing a crash. Also I'd like to redirect the log to a callback (this might be possible already, but I didn't check yet)

Crash on shutdown:

[FATAL] A fatal error occurred and the program has to terminate!
  - Error: InternalErrorException
  - Description: Trying to access an object that has been destroyed.
  - In function: void bs::GameObjectHandleBase::throwIfDestroyed() const
  - In file: /home/hhyyrylainen/Projects/Leviathan/ThirdParty/bsf/Source/Foundation/bsfCore/Scene/BsGameObjectHandle.cpp:32

Stack trace: 
0) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::getStackTrace[abi:cxx11]()+0x1b9 [0x7f14f7512e69]
1) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::logErrorAndStackTrace(std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, unsigned int) const+0x323 [0x7f14f74f5973]
2) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::reportCrash(std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, unsigned int) const+0x9 [0x7f14f7512909]
3) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::GameObjectHandleBase::throwIfDestroyed() const+0x9e [0x7f14f7844f3e]
4) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::SceneManager::removeFromStateList(bs::GameObjectHandle<bs::Component> const&)+0x71 [0x7f14f785bed1]
5) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::SceneManager::_notifyComponentDestroyed(bs::GameObjectHandle<bs::Component> const&, bool)+0x13a [0x7f14f785df4a]
6) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::SceneObject::destroyInternal(bs::GameObjectHandleBase&, bool)+0x102 [0x7f14f784e182]
7) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::GameObjectManager::destroyQueuedObjects()+0x44 [0x7f14f7847124]
8) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::SceneManager::clearScene(bool)+0x1dc [0x7f14f785f23c]
9) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::Application::onShutDown()+0x16 [0x7f14f7af2f56]
10) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: bs::Module<bs::CoreApplication>::shutDown()+0x3e [0x7f1500ea013e]
11) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: Leviathan::Graphics::ShutdownBSF()+0x51 [0x7f1500e9a8c1]
12) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: Leviathan::Graphics::Release()+0x55 [0x7f1500e9a935]
13) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: Leviathan::Engine::Release(bool)+0x345 [0x7f1500db1645]
14) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: Leviathan::LeviathanApplication::Release()+0x68 [0x7f1500e807a8]
15) /home/hhyyrylainen/Projects/thrive/build/bin/libEngine.so: Leviathan::LeviathanApplication::RunMessageLoop()+0x101 [0x7f1500e80b81]
16) ./Thrive() [0x4205e3]
17) /lib64/libc.so.6: __libc_start_main+0xf3 [0x7f14f6950413]
18) ./Thrive() [0x422a8e]
[FATAL] A fatal error occurred and the program has to terminate!
  - Error: Segmentation fault
  - Description: Received fatal signal
  - In function: 
  - In file: :0

Stack trace: 
0) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::getStackTrace[abi:cxx11]()+0x1b9 [0x7f14f7512e69]
1) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::logErrorAndStackTrace(std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, unsigned int) const+0x323 [0x7f14f74f5973]
2) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::CrashHandler::reportCrash(std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, bs::StdAlloc<char, bs::GenAlloc> > const&, unsigned int) const+0x9 [0x7f14f7512909]
3) /home/hhyyrylainen/Projects/thrive/build/bin/lib/libbsf.so.1: bs::signalHandler(int, siginfo_t*, void*)+0x10c [0x7f14f7512a2c]
4) /lib64/libpthread.so.0: +0x13070 [0x7f14f6e3e070]
5) /lib64/libGL.so.1: +0x4ae69 [0x7f14ac73be69]
[WARNING] Cannot open file: //hhyyrylainen/Projects/thrive/build/bin/CrashReports/01190719_2013/log.html
		 in bs::FileDataStream::FileDataStream(const bs::Path&, bs::DataStream::AccessMode, bool) [/home/hhyyrylainen/Projects/Leviathan/ThirdParty/bsf/Source/Foundation/bsfUtility/FileSystem/BsDataStream.cpp:386]

at a time

BearishSun · 2019-08-19T19:13:38Z

Thanks for setting up the draft, it would be really nice functionality to have.

I'd love to find a way to restructure Application and/or CoreApplication so it allows external loop by default. This would eliminate the code duplication and allow those classes to use that same functionality internally.

Something like this?

Application::beginMainLoop();
while(Application::isMainLoopRunning())
{
	Application::runMainLoopFrame();
}
Application::endMainLoop();

For further control, we could also add extension points that execute user-code within runMainLoopFrame(). This would be similar how renderer extension work (See Renderer::addPlugin). They use an enum to determine at which point should the code execute, and we can extend these with more extension points as users need them. For now this might not be needed at all.

I'm open to callbacks for custom crash handlers and logging.

hhyyrylainen · 2019-08-20T17:29:59Z

I moved the things to bs::CoreApplication. I think I also figured out the crashing on shutdown. I added an extra frame render call to the shutdown, which seems to have solved the crashing there. So it might have been that I was destroying stuff after the final frame, and then on bsf shutdown the objects were attempted to be destroyed again.

For disabling the crash handler, would adding parameters for configuring callbacks to START_UP_DESC be suitable?

BearishSun · 2019-08-21T20:35:25Z

Yep that looks like what I imagined.

It feels that one extra cycle before shutdown should be unnecessary, there might be something else causing the crash, but I didn't have time to take a closer look. I don't think I'm doing an extra cycle in the existing loop.

Adding callbacks to START_UP_DESC sounds fine I think.

hhyyrylainen · 2019-08-28T14:04:24Z

I've now done the callbacks I'd like to have.
I didn't test on windows.

Is this an acceptable approach? I can move the final render frame out of the end loop method (and into my code) if that would be better.

BearishSun · 2019-08-29T08:10:48Z

Source/Foundation/bsfUtility/Error/BsCrashHandler.h

+	/** Struct for holding crash handler settings */
+	struct CrashHandlerSettings
+	{
+		/** Called at the start of a when a windows SEH exception is started to be handled. Return true to skip default action */


Comment seems to imply this is windows only

Whoops, I forgot to write the first part of the sentence, it should say both for normal exception handling and SEH.

BearishSun · 2019-08-29T08:12:02Z

Source/Foundation/bsfUtility/Error/BsCrashHandler.h

+			const String& description,
+			const String& function,
+			const String& file,
+			UINT32 line)> onBeforeReportCrash;


Members should have 'm' prefix (mOnBeforeReportCrash) and be listed before static memebers (i.e. before sFatalErrorMsg). Same goes for the two below.

Actually, can't we just keep a copy of CrashHandlerSettings here? Saves copying the fields and the doc comments as well.

BearishSun · 2019-08-29T08:14:32Z

Source/Foundation/bsfUtility/Error/BsCrashHandler.h

+		* Called after the crash callstack is written to log. Return true to skip writing to file and doing
+		* further other on crash actions
+		*/
+		std::function<bool()> onCrashPrintedToLog;


I'd prefer this be named onAfterReportCrash, to be more consistent with the naming + printing to log might not happen, we might just save a dump file, in which case its inacurrate.

I don't want BSF to actually write any log files. So I want this callback to be between the print to log and save log to file calls in the crash handler. The name change wouldn't completely capture the entire meaning.

Makes sense.

BearishSun · 2019-08-29T08:16:20Z

Crash handler callback stuff looks good, clean code and I like the addition of the settings object. I made some comments on minor issues.

Regarding the log callback, I don't think we should be adding a callback to Log class. It's more of a container. The callback should be part of Debug, but looking at it, it already has Debug::onLogEntryAdded event, which I believe should be adequate? If not we can reconsider.

hhyyrylainen · 2019-08-29T08:21:44Z

I'll address the other comments in a bit. But I don't think the current Debug callback is sufficient. It looks like that it doesn't immediately trigger, which is something I need in saving my custom log on crash. Also it doesn't have a way to suppress the default actions. Though moving the immediate callback to bs::Debug should work.

…ndler

hhyyrylainen · 2019-08-29T12:48:22Z

I made some changes.

BearishSun · 2019-08-30T14:05:15Z

Source/Foundation/bsfCore/BsCoreApplication.cpp

@@ -75,7 +75,7 @@ namespace bs
 		// Ensure all errors are reported properly
 		CrashHandler::startUp(desc.crashHandling);
 		if(desc.logCallback)
-			gDebug().getLog().setLogCallback(desc.logCallback);
+			gDebug().setLogCallback(desc.logCallback);


Perhaps it would be better to make this an Event? This way multiple things can subscribe to it.

Ah but I guess you need the return value. So that's not going to work.

Yeah, I want to skip the default thing. And it looks like the Event class can't return any callback result values.

BearishSun · 2019-08-30T14:06:44Z

Source/Foundation/bsfUtility/Debug/BsDebug.h

@@ -98,6 +105,8 @@ namespace bs
 	private:
 		UINT64 mLogHash = 0;
 		Log mLog;
+		/** @copydoc START_UP_DESC::logCallback */


We shouldn't copy-doc from START_UP_DESC. Anything in bsfUtility shouldn't reference classes in higher 'layers' (i.e. bsfEngine).

What would be the alternative, have the copydoc going the other way?

You can't copy-doc private stuff. I'd not document the private field at all since its setter is documented anyway.

I removed the documentation on it.

BearishSun · 2019-08-30T14:09:30Z

Source/Foundation/bsfCore/BsCoreApplication.cpp

+	void CoreApplication::endMainLoop()
+	{
+		// One more frame needs to be rendered here to not crash if the outer main loop has done stuff since the last frame.
+		runMainLoopFrame();


This bit is still bothering me. What exactly happens when you remove it? Does the crash happen with normal bsf code or only your app (e.g. does it happen in examples)? If it's only your code then I'd be happiest we removed it here.

I removed that call. I'm sure though that if anyone uses the single frame functions like I do (with releasing objects after the last frame), they are going to need to have the extra frame added in their code.

If anyone else gets crashes "when doing it correctly" then they need to add one extra runMainLoopFrame call before calling endMainLoop

BearishSun · 2019-08-30T14:54:08Z

Thanks for making the changes. Merged.

hhyyrylainen · 2019-08-30T15:03:20Z

thanks

Added a basic application alternative that allows rendering one frame

fb5084d

at a time

Moved the single frame stuff to CoreApplication

99ef938

hhyyrylainen mentioned this pull request Aug 24, 2019

Crash on loading skybox texture asset #394

Closed

Added callbacks for crash handling and log messages

c7683f5

hhyyrylainen marked this pull request as ready for review August 28, 2019 14:04

Added return values for the early SEH exits

86aa37c

BearishSun reviewed Aug 29, 2019

View reviewed changes

hhyyrylainen added 2 commits August 29, 2019 15:34

Changed crash reporter settings to be stored as struct in the CrashHa…

8e09f00

…ndler

Moved the custom log callback to Debug

50e64c7

BearishSun reviewed Aug 30, 2019

View reviewed changes

hhyyrylainen added 2 commits August 30, 2019 17:12

Removed the extra frame render before shutdown

d674fbf

If anyone else gets crashes "when doing it correctly" then they need to add one extra runMainLoopFrame call before calling endMainLoop

Removed copydoc from Debug::mCustomLogCallback

7582907

BearishSun merged commit f644161 into GameFoundry:master Aug 30, 2019

hhyyrylainen deleted the external_main_loop branch August 30, 2019 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for external main loop #391

Support for external main loop #391

hhyyrylainen commented Aug 19, 2019 •

edited

Loading

BearishSun commented Aug 19, 2019

hhyyrylainen commented Aug 20, 2019

BearishSun commented Aug 21, 2019

hhyyrylainen commented Aug 28, 2019

BearishSun Aug 29, 2019

hhyyrylainen Aug 29, 2019

BearishSun Aug 29, 2019

BearishSun Aug 29, 2019

BearishSun Aug 29, 2019

hhyyrylainen Aug 29, 2019

BearishSun Aug 29, 2019

BearishSun commented Aug 29, 2019

hhyyrylainen commented Aug 29, 2019

hhyyrylainen commented Aug 29, 2019

BearishSun Aug 30, 2019

BearishSun Aug 30, 2019

hhyyrylainen Aug 30, 2019

BearishSun Aug 30, 2019

hhyyrylainen Aug 30, 2019

BearishSun Aug 30, 2019

hhyyrylainen Aug 30, 2019

BearishSun Aug 30, 2019

hhyyrylainen Aug 30, 2019

BearishSun commented Aug 30, 2019

hhyyrylainen commented Aug 30, 2019

Support for external main loop #391

Support for external main loop #391

Conversation

hhyyrylainen commented Aug 19, 2019 • edited Loading

BearishSun commented Aug 19, 2019

hhyyrylainen commented Aug 20, 2019

BearishSun commented Aug 21, 2019

hhyyrylainen commented Aug 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BearishSun commented Aug 29, 2019

hhyyrylainen commented Aug 29, 2019

hhyyrylainen commented Aug 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BearishSun commented Aug 30, 2019

hhyyrylainen commented Aug 30, 2019

hhyyrylainen commented Aug 19, 2019 •

edited

Loading