#500 closed defect (fixed)
[PATCH] Make Atlas work on OS X
Reported by: | Philip Taylor | Owned by: | historic_bruno |
---|---|---|---|
Priority: | Nice to Have | Milestone: | Alpha 8 |
Component: | Atlas editor | Keywords: | simple, mac, osx |
Cc: | Jan Wassenberg | Patch: |
Description
When running ./pyrogenesis_dbg -editor
on OS X, it starts up the editor window and then the window freezes (though all the code threads continue running).
In the current design, pyrogenesis_dbg
starts up first, via SDL's main
function, which calls [NSApplication run]
and eventually ends up in our main
(source/main.cpp
).
We then detect the -editor
flag, load libAtlasUI_dbg.dll
, start up a new thread, and run Atlas_StartWindow
(source/tools/atlas/AtlasUI/Misc/DLLInterface.cpp
) in the new thread, which calls wxEntry
to start the wxWidgets event loop.
As far as I can tell, the problem is that we have two Cocoa event loops, one in SDL and one in wxWidgets, and the GUI thread doesn't get the input events it needs. (See here etc for some stuff about Cocoa threading.)
If that's true, we need to do, uh, something to fix it. (I have no idea what, but it might involve a redesign of the game/editor startup process). If that's not true (I'm not an OS X developer so I may easily be mistaken), we need to work out what the problem really is and then do something to fix it.
Attachments (2)
Change History (18)
comment:1 by , 14 years ago
Milestone: | Unclassified |
---|
comment:2 by , 14 years ago
Milestone: | → Backlog |
---|---|
Owner: | set to |
comment:3 by , 13 years ago
A thing to try: When we launch Atlas, perhaps we could run wxWidgets on the main thread (so it should receive Cocoa events) instead of a new UI thread, and then run the rest of the engine in a new thread. So the engine code decides to run Atlas and then gets shunted across into a new thread instead of continuing as normal. I'm not sure how crazy/possible that will be.
comment:4 by , 13 years ago
Owner: | removed |
---|
comment:5 by , 13 years ago
Keywords: | review added |
---|---|
Owner: | set to |
Status: | new → assigned |
Summary: | Make Atlas work on OS X → [PATCH] Make Atlas work on OS X |
Attaching a patch which runs Atlas window in the main thread and shifts the game loop into a new thread. This has been tested and works on Windows (wxMSW 2.8.12) and OS X 10.6 (wxOSX/Cocoa 2.9.2).
by , 13 years ago
Attachment: | atlas-threading-osxfix-08282011.patch added |
---|
comment:6 by , 13 years ago
Oh and to clarify the problem on OS X without this patch: wxWidgets ties into various parts of the Cocoa API which do thread-safety checks, requiring the UI code to run on the main thread, and these cause fatal errors.
Can't test this patch with wxWidgets 2.8 and Carbon, because it won't build on a 64-bit system.
comment:7 by , 13 years ago
Milestone: | Backlog → Alpha 7 |
---|
comment:8 by , 13 years ago
Just noticed that the patch seems to break opening Atlas from the main menu.
comment:9 by , 13 years ago
The associated error and callstack:
First-chance exception at 0x75a9b9bc in pyrogenesis_dbg.exe: 0x800401F0: CoInitialize has not been called. wmi.cpp(97): Function call failed: return value was -1 (Function failed (no details available))
pyrogenesis_dbg.exe!sys_display_error(const wchar_t * text, unsigned int flags) Line 315 + 0x1d bytes C++ pyrogenesis_dbg.exe!CallDisplayError(const wchar_t * text, unsigned int flags) Line 377 + 0xd bytes C++ pyrogenesis_dbg.exe!debug_DisplayError(const wchar_t * description, unsigned int flags, void * context, const wchar_t * lastFuncToSkip, const wchar_t * pathname, int line, const char * func, volatile int * suppress) Line 468 + 0xd bytes C++ pyrogenesis_dbg.exe!debug_OnError(__int64 err, volatile int * suppress, const wchar_t * file, int line, const char * func) Line 545 + 0x2c bytes C++> pyrogenesis_dbg.exe!wmi_GetClass(const wchar_t * className, std::map<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,tagVARIANT,std::less<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >,std::allocator<std::pair<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const ,tagVARIANT> > > & wmiMap) Line 97 + 0x1a bytes C++ pyrogenesis_dbg.exe!wgfx_CardName(wchar_t * cardName, unsigned int numChars) Line 141 + 0xe bytes C++ pyrogenesis_dbg.exe!gfx::CardName() Line 43 + 0x11 bytes C++ pyrogenesis_dbg.exe!RunHardwareDetection() Line 190 + 0x10 bytes C++ pyrogenesis_dbg.exe!InitGraphics(const CmdLineArgs & args, int flags) Line 867 C++ pyrogenesis_dbg.exe!AtlasMessage::fInitGraphics(AtlasMessage::mInitGraphics * msg) Line 78 + 0xd bytes C++ pyrogenesis_dbg.exe!AtlasMessage::fInitGraphics_wrapper(AtlasMessage::IMessage * msg) Line 56 + 0x5f bytes C++ pyrogenesis_dbg.exe!RunGame(void * data) Line 169 + 0x19 bytes C++ pyrogenesis_dbg.exe!thread_start(void * param) Line 585 + 0x9 bytes C++ msvcr100d.dll!_callthreadstartex() Line 314 + 0xf bytes C msvcr100d.dll!_threadstartex(void * ptd) Line 297 C
Reason why this would break when moving graphics init to a new thread:
CoInitialize Function Initializes the COM library on the current thread and identifies the concurrency model as single-thread apartment (STA).
There is no error when using the -editor flag because the COM library has not been initialized yet, it does so on the new thread when InitGraphics() is called. Maybe we should shut down the associated module when exiting the game and before starting Atlas, so it enters the uninitialized state. I don't know if that would have other unintended consequences, but is worth testing.
comment:10 by , 13 years ago
If it's complex/messy to move the engine thread on Windows, we could keep the old Atlas behaviour on Windows and do the thread-swapping only on non-Windows (since we use a load of low-level Win32 API stuff, unlike on other OSes). (Best to keep Linux and OS X consistent if possible so that that code path gets regular testing). Would be nicer if the engine can shut down and start up again on a different thread without these errors if it's not too much of a pain, though.
comment:11 by , 13 years ago
To shut down properly we have to balance each (successful) CoInitialize with a CoUninitialize, which seems simple enough as we don't use them very often. In fact we haven't been calling wmi_Shutdown at all. Attaching a new patch that does that and fixes the Atlas loading.
by , 13 years ago
Attachment: | atlasthreading-wmifix-09032011.patch added |
---|
comment:12 by , 13 years ago
Cc: | added |
---|
comment:13 by , 13 years ago
Milestone: | Alpha 7 → Alpha 8 |
---|---|
Priority: | Should Have → Nice to Have |
comment:14 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:15 by , 13 years ago
Keywords: | review removed |
---|
Milestone Unclassified deleted