AR Toolkit - fixes for iOS, MAUI iOS #466
Conversation
mstefarov
force-pushed
the
ncastle/v200-ios-ar
branch
from
424bb37 to
2233a0f
Compare
| camera1.Heading == camera2.Heading && | ||
| camera1.Pitch == camera2.Pitch && | ||
| camera1.Roll == camera2.Roll; | ||
| Math.Abs(camera1.Heading - camera2.Heading) < 0.01 && | ||
| Math.Abs(camera1.Pitch - camera2.Pitch) < 0.01 && | ||
| Math.Abs(camera1.Roll - camera2.Roll) < 0.01; |
There was a problem hiding this comment.
Undo, or of really needed, reduce tolerance to around Epsilon.
| return Encoding.UTF8.GetString(ms.ToArray()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Bring back. Not sure why we don't want to use it any longer? Commit jsut says "Fix download manager to work with .net6-ios", but I only see removed code, no fix
| task = previousTask; | ||
| tempFile = task.Filename; | ||
| } | ||
| } |
| @@ -77,7 +93,7 @@ private async Task InitializeAsync() | |||
|
|
|||
| private void MoveVertical(double offset) | |||
| { | |||
| ARView.SetInitialTransformation(ARView.InitialTransformation + TransformationMatrix.Create(0, 0, 0, 1, 0, -offset, 0)); | |||
| ARView?.SetInitialTransformation(ARView.InitialTransformation + TransformationMatrix.Create(0, 0, 0, 1, 0, -offset * 10, 0)); | |||
There was a problem hiding this comment.
Why this change? We should undo
| ARView.Scene = scene; | ||
| //await scene.LoadAsync(); | ||
|
|
||
| ARView!.TranslationFactor = 1; |
There was a problem hiding this comment.
Why didn't we need to set this before? Probably undo
| #if WINDOWS || __IOS__ || __ANDROID__ | ||
| : ViewHandler<IARSceneView, Esri.ArcGISRuntime.ARToolkit.ARSceneView> | ||
| #else | ||
| #if WINDOWS || __IOS__ || __ANDROID__ | ||
| : ArcGISRuntime.Maui.Handlers.SceneViewHandler | ||
| #else |
There was a problem hiding this comment.
This doesn't feel quite right. We need IARSceneView in the viewhandler interface.
| protected override void ConnectHandler(ARToolkit.ARSceneView platformView) | ||
| protected override void ConnectHandler(Esri.ArcGISRuntime.UI.Controls.SceneView platformView) |
There was a problem hiding this comment.
This is another artifact of inheriting from the wrong thing.
| if (arSceneView.RenderVideoFeed) | ||
| { | ||
| arSceneView.SpaceEffect = arview.SpaceEffect = SpaceEffect.None; | ||
| arSceneView.AtmosphereEffect = arview.AtmosphereEffect = AtmosphereEffect.None; | ||
| } | ||
| else | ||
| { | ||
| arSceneView.SpaceEffect = arview.SpaceEffect = SpaceEffect.Stars; | ||
| arSceneView.AtmosphereEffect = arview.AtmosphereEffect = AtmosphereEffect.HorizonOnly; | ||
| } |
There was a problem hiding this comment.
Shouldn't the native control do this? If it's to sync up with native control, can we get a property change to react to, or just grab the value from arview instead of assigning both?
| @@ -141,37 +141,41 @@ private void OnCameraDidChangeTrackingState(ARSession session, ARCamera camera) | |||
|
|
|||
| private void OnWillRenderScene(ISCNSceneRenderer renderer, SCNScene scene, double timeInSeconds) | |||
| { | |||
| if (_tracking && ARSCNView != null) | |||
| CoreFoundation.DispatchQueue.MainQueue.DispatchAsync(() => | |||
There was a problem hiding this comment.
I don't have context why this change is now required. This shouldn't be necessary. Commit comment is "Fix crash AR sceneview ios" with no reference to issue.
| var t = pov.WorldPosition; | ||
| if (_controller != null) | ||
| _controller.TransformationMatrix = InitialTransformation + TransformationMatrix.Create(q.X, q.Y, q.Z, q.W, t.X, t.Y, t.Z); |
There was a problem hiding this comment.
All this says "Fix issue with AR position on iOS" in commit. I'm not quite following why this change is needed. See commit f24a385
I didn't fix all nullability warnings. There are quite a few nullability warnings on iOS that aren't likely to cause problems in practice and VS for Mac isn't giving any editor support (something wrong with multitargeting). It didn't seem worth the time to fix given we won't be releasing AR Toolkit at 200.