HeliumProject / Engine

C++ Game Engine (Under Construction!)
http://heliumproject.org/
Other
442 stars 70 forks source link

Can't run on windows in 64-bit #46

Closed kevindqc closed 11 years ago

kevindqc commented 11 years ago

Is it supposed to work? I had an access violation trying to run the editor, and I narrowed down the problem to :

template< typename BaseT > Helium::RefCountProxy< BaseT >* Helium::RefCountProxyContainer< BaseT >::Get( BaseT* pObject )

There's an AtomicCompareExchangeRelease going on, but on a pointer. AtomicCompareExchangeRelease works on 32-bit integers, so m_pProxy gets cut off :(

zach-brockway commented 11 years ago

I just pushed a fix for a recently introduced crash on startup in the editor. Can you update to revision 79ee6ab249127220b4e436fb442d161c9967e8f3 and let me know if you're still encountering this problem?

kevindqc commented 11 years ago

No, still the problem with the pointer value getting cut off

zach-brockway commented 11 years ago

I am able to build and run Editor in x64 configurations without encountering any access violation. It would be helpful if you could provide a call stack for the problem you're seeing.

kevindqc commented 11 years ago

Well I guess it depends. If you're lucky the pointer is like 0x0000000002346523, so it stays the same.

If you're like me, it's 0x000000CAD9139DC8 and gets cut to 0x00000000D9139DC8

kevindqc commented 11 years ago

Alright. Here's pProxy with the right address: http://i.imgur.com/uhpXYYG.png

After the compare and swap, m_pProxy is wrong: http://i.imgur.com/A8dqNNE.png

This is where it happens: http://i.imgur.com/op7vVzW.png

The actual data access violation happens here: http://i.imgur.com/WQber9K.png

it tries to access the proxy object and increments its counter, but it just increments some random memory address and crash

From what I understand, on pointers, _InterlockedCompareExchangePointer should be used, not _InterlockedCompareExchange. There's probably that problem on other platforms too if you're unlucky with the memory addresses

kevindqc commented 11 years ago

Alright, I changed the first line of this: http://imgur.com/IVw4T59

it was using HELIUM_CPU_X86, I don't think that's right.

I'm even wondering if there needs to be an if ?

the #else casts to void* instead of long - if we're on 32-bit, it's gonna take the int32 _InterlockedExchangePointer overload, if it's on 64-bit if's gonna take the int64 overload ?

kevindqc commented 11 years ago

There are other errors.

I tried to create a new project, then a new scene. I hit an assertion saying there were no world definition, no idea what that is, but it continued with no problem..

But after that there's a bitwise AND with this value and a pointer: const static uint32_t POOL_ALIGN_SIZE_MASK = ~(POOL_ALIGN_SIZE-1) which becomes 0xffffffe0 here again, the pointer gets cut off after the bitwise AND :(

I fixed it, there seems to be other problems there too. I'll try to get everything working on my computer and submit a patch

kevindqc commented 11 years ago

Alright well my problems seems to be connected with the first ASSERT I get.

m_World->GetComponents().GetFirst<GraphicsManagerComponent>()->GetGraphicsScene()

since my world seems to be a dummy world, there are no GraphicsManagerComponent - the GetFirst returns null.. yet we still call GetGraphicsScene()

How am I supposed to have a world definition?

aclysma commented 11 years ago

Howdy, thanks for looking at helium!

Right now, the editor doesn't really do anything. It should open and shut cleanly, but all the gameplay stuff (framework) is in heavy state of flux.

One of the most recent changes was to pull the renderer out of framework/world code, and instead allow any renderer to be injected. The long term goal is that you can BYO anything and inject it as a DLL which means we can't have World depend on graphics like it did. And that is the error you're looking at now.

You could get a world definition with a renderer component with data. The examples have this set up. But I don't think there is a path for you to tell the editor to load a particular world definition yet. It defaults to empty.

You should jump on IRC. I'll be home and online in about 20 minutes. Freenode #helium

Philip

On Jun 26, 2013, at 8:19 PM, Kevin Doyon notifications@github.com wrote:

Alright well my problems seems to be connected with the first ASSERT I get.

m_World->GetComponents().GetFirst()->GetGraphicsScene()

since my world seems to be a dummy world, there are no GraphicsManagerComponent - the GetFirst returns null.. yet we still call GetGraphicsScene()

How am I supposed to have a world definition?

— Reply to this email directly or view it on GitHub.

gorlak commented 11 years ago

That x86 conditional compile issue looks legit to me, and is probably my fault when the fiddling with that code when adding the gcc/clang equivalents weeks ago. Folks running x64 must be just getting lucky that they are getting 32-bit addressable pages of virtual address space. Also, clearly that uint32_t for POOL_ALIGN_SIZE_MASK should be a uintptr_t. I will fix these up right now, but If you have patches/pull request for this sort of stuff, I would be happy to merge them.

The other Graphics framework stuff is probably just half-baked code as aclysma has pointed out.

gorlak commented 11 years ago

@kevindqc: a1b8819a6983fde8f0303a78d70bb1c02b7d65ca fixes the issues you pointed out.

Everyone else: there were a bunch of 32-bit build bugs that I fixed as well. Tip of the day: you can't pass aligned types by value in 32-bit cl.exe! Pass by const-ref instead.

kevindqc commented 11 years ago

I dont see anything for the atomic operations causing problems when used with 64-bit pointers?

On Jun 28, 2013, at 2:50 PM, Geoff Evans notifications@github.com wrote:

@kevindqc https://github.com/kevindqc: a1b8819https://github.com/HeliumProject/Helium/commit/a1b8819a6983fde8f0303a78d70bb1c02b7d65cafixes the issues you pointed out.

Everyone else: there were a bunch of 32-bit build bugs that I fixed as well. Tip of the day: you can't pass aligned types by value in 32-bit cl.exe! Pass by const-ref instead.

— Reply to this email directly or view it on GitHubhttps://github.com/HeliumProject/Helium/issues/46#issuecomment-20206970 .

gorlak commented 11 years ago

Did you update your submodules?

kevindqc commented 11 years ago

oh nvm, I'm not home I just followed the link and didn't see anything. I was wondering what was the part about submodules at the bottom, I guess I just don't see their diff, only the commit id change :)

While you're working on Helium, I had some problems building it because of FBX - it was looking for libfbxsdk.lib, but on my computer it was on the form (seen here: http://docs.autodesk.com/FBX/2014/ENU/FBX-SDK-Documentation/files/GUID-6E7B1A2E-584C-49C6-999E-CD8367841B7C.htm) fbxsdk-2014.2.lib

On Fri, Jun 28, 2013 at 2:58 PM, Geoff Evans notifications@github.comwrote:

Did you update your submodules?

— Reply to this email directly or view it on GitHubhttps://github.com/HeliumProject/Helium/issues/46#issuecomment-20207498 .

gorlak commented 11 years ago

Hm, I think that documentation is for the incorrect version of FBX for Helium (2014.0). You should be using 2014.1, which on my machine does in fact include the 'lib' prefix in lib and dll file name. Installing the correct version should work, its been working fine for me.

kevindqc commented 11 years ago

My bad, you are again completely right. I have a beta 2014.1 installation apparently, I didn't remember.

ADDITIONAL NOTES: We removed the version number in the library names and moved the debug/release builds into subfolders. Because of this, if you installed a beta or release candidate version of FBX 2014.1, you will need to uninstall and clean the installation folder before you install FBX 2014.1 final release, otherwise the old libraries with be left out in the folder, and will cause link errors and other issues if used.

I'll test your commit on my computer when I have time (I'm moving) and close this if it works! Thanks

kevindqc commented 11 years ago

works well :)