microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6k stars 2.12k forks source link

strange code in LevelSolver.cs #238

Closed izatar closed 7 years ago

izatar commented 7 years ago

Here is the InitializeSolver() function I see in my sync'd holotoolkit-unity:


    public bool InitializeSolver()
    {
        if (IsSolverInitialized ||
            !SpatialUnderstanding.Instance.AllowSpatialUnderstanding)
        {
            return IsSolverInitialized;
        }

        if (SpatialUnderstandingDllObjectPlacement.Solver_Init() > 1)
        {
            IsSolverInitialized = true;
        }
        return IsSolverInitialized;
    }

I see Solver_Init() resolving to:


#if UNITY_METRO && !UNITY_EDITOR
        [DllImport("SpatialUnderstanding")]
        public static extern int Solver_Init();
#else

which resolves to something in the C++ code in Holotoolkit:

EXTERN_C __declspec(dllexport) int Solver_Init()
{
    FRGSolver_W& solver = UnderstandingMgr_W::GetUnderstandingMgr().GetSolver();

    // Init the solver
    solver.Init(UnderstandingMgr_W::GetUnderstandingMgr().GetPlayspaceInfosPtr(),
        &UnderstandingMgr_W::GetUnderstandingMgr().GetPlayspaceInfos().m_TopologyAnalyzer,
        &UnderstandingMgr_W::GetUnderstandingMgr().GetPlayspaceInfos().m_ShapeAnalyzer);

    return 1;
}

It appears the Solver_Init() always returns 1. But, the function in LevelSolver.cs is checking for ">1" so it appears that " IsSolverInitialized = true;" never happens. I put in a debug there and it does in fact look like it never gets set right, and so appears to be calling InitializeSolver() on every update.

NeerajW commented 7 years ago

@jevertt looks like the check should be updated to == 1 instead of > 1?

jevertt commented 7 years ago

It looks like solver.Init() can be called multiple times without any bad side-effects, so this wasn't noticed. But, yes - that's not intentional - it's a bug.

keveleigh commented 7 years ago

Fixed with #371