Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make all Map.find's safe / KeyNotFound #277

Open
haraldsteinlechner opened this issue Jan 9, 2023 · 1 comment
Open

Make all Map.find's safe / KeyNotFound #277

haraldsteinlechner opened this issue Jan 9, 2023 · 1 comment
Assignees

Comments

@haraldsteinlechner
Copy link
Collaborator

I recently stumbled across several occurances of Map.find (mostly concerned with surfaces). This should all be refactored to Map.tryFind etc. Actually, i just even found a failure case which fails with KeyNotFound - load garden city/create a new scene.

isOpc: "D:\pro3d\GardenCity\MSL_Mastcam_Sol_929_id_48423\OPC_001_000"
 7: create DNS annotations
 7: mk lines ......................................................... 0.004 s
 7: creating points .................................................. 0.000 s
Unhandled exception. System.Collections.Generic.KeyNotFoundException: could not get key: a28e27ff-9978-411a-b270-05ffeb8a2d46
   at FSharp.Data.Adaptive.AMapModule.find@1441.Invoke(FSharpHashMap`2 m)
   at FSharp.Data.Adaptive.AValModule.MapVal`2.Compute(AdaptiveToken token)
   at FSharp.Data.Adaptive.AValModule.AbstractVal`1.GetValue(AdaptiveToken token)
...
   at FSharp.Data.Adaptive.AValModule.AbstractVal`1.FSharp.Data.Adaptive.IAdaptiveValue<'T>.GetValue(AdaptiveToken t)
   at Aardvark.Rendering.ShaderReflection.ShaderParameterWriter.Bind@424-1.PerformWrite(AdaptiveToken token, IntPtr target)
   at Aardvark.Rendering.ShaderReflection.ShaderParameterWriter.AdaptiveWriter.Write(AdaptiveToken token, IntPtr target)
   at Aardvark.Rendering.ShaderReflection.ShaderParameterWriter.AdaptiveWriter.Aardvark.Rendering.ShaderReflection.IAdaptiveWriter.Write(AdaptiveToken token, IntPtr target)
   at <StartupCode$Aardvark-Rendering-GL>.$ResourceManager.CreateUniformBuffer@102-1.Create(AdaptiveToken token, RenderToken rt, FSharpOption`1 old)
   at Aardvark.Rendering.Resource`2.PerformUpdate(AdaptiveToken token, RenderToken t)
@haraldsteinlechner haraldsteinlechner self-assigned this Jan 9, 2023
haraldsteinlechner added a commit that referenced this issue Jan 9, 2023
…gleSurfaceSg not to use unsafe lookUp function any longer.
@haraldsteinlechner
Copy link
Collaborator Author

the pattern is often:

if Map.contains id surfaces then
   let s = Map.find id surfaces

When passing such values to shaders, non-deterministic delete order can lead to particularly problematic, hard-to-debug exceptions. #278 fixes the most important one.

  • fix reproducable problem
  • mark most prominent unsafe functions as obsolete
  • mark all non-total functions unsafe.
  • remove all unsafe surface/annotation lookups - ban Map.find/AMap.find from Pro3D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant