[arkit] Update for iOS 11 beta 1 (new framework) #2187

VincentDondain · 2017-06-08T01:11:24Z

Updated scenekit.cs SCNSceneRendererDelegate to avoid:
ARKit/ARSCNViewDelegate.g.cs(189,43): error CS0432: Alias 'Iglobal' not found

Part 2

Fixed the generator to avoid ARKit/ARSCNViewDelegate.g.cs(189,43): error CS0432: Alias 'Iglobal' not found
ARPointCloud can be tested with this sample: https://www.dropbox.com/s/0xlz0sdzjj4507s/ARPointCloudTest.zip?dl=0
It requires deploying on devices and moving the camera around so ARKit can return some points. After 5 seconds it should print the points.
Added code comments about generator bugs.
Fixed styling.
Added missing simd.
Fixed intro tests.

monojenkins · 2017-06-08T04:01:19Z

Build failure

spouliot · 2017-06-08T13:13:50Z

src/arkit.cs

+		nuint Count { get; }
+
+		[Export ("points")]
+		IntPtr Points { get; }


Can't we expose something better ?

spouliot · 2017-06-08T13:17:25Z

src/arkit.cs

+
+		[Static]
+		[Wrap ("(SCNDebugOptions)_ARSCNDebugOptionShowWorldOrigin")]
+		SCNDebugOptions ShowWorldOrigin { get; }


BindAsAttribute ? Cc @dalexsoto

I do not recall adding explicit support for Fields + BindAs but it is worth a try

Tried, not working, opened bug: https://bugzilla.xamarin.com/show_bug.cgi?id=57537

spouliot · 2017-06-08T13:17:32Z

src/arkit.cs

+
+		[Static]
+		[Wrap ("(SCNDebugOptions)_ARSCNDebugOptionShowFeaturePoints")]
+		SCNDebugOptions ShowFeaturePoints { get; }


spouliot · 2017-06-08T13:18:29Z

src/scenekit.cs

@@ -2329,22 +2331,22 @@ interface SCNSceneRenderer {
 	interface SCNSceneRendererDelegate {

 		[Export ("renderer:willRenderScene:atTime:")]
-		void WillRenderScene ([Protocolize]SCNSceneRenderer renderer, SCNScene scene, double timeInSeconds);
+		void WillRenderScene (ISCNSceneRenderer renderer, SCNScene scene, double timeInSeconds);



That breaks classic and it seems the API is available on macOS (so we still care)

spouliot · 2017-06-08T13:18:39Z

src/scenekit.cs


 		[Export ("renderer:didRenderScene:atTime:")]
-		void DidRenderScene ([Protocolize]SCNSceneRenderer renderer, SCNScene scene, double timeInSeconds);
+		void DidRenderScene (ISCNSceneRenderer renderer, SCNScene scene, double timeInSeconds);


spouliot · 2017-06-08T13:18:47Z

src/scenekit.cs


 		[Mac (10,10)]
 		[Export ("renderer:updateAtTime:")]
-		void Update ([Protocolize]SCNSceneRenderer renderer, double timeInSeconds);
+		void Update (ISCNSceneRenderer renderer, double timeInSeconds);


spouliot · 2017-06-08T13:18:53Z

src/scenekit.cs


 		[Mac (10,10)]
 		[Export ("renderer:didApplyAnimationsAtTime:")]
-		void DidApplyAnimations ([Protocolize]SCNSceneRenderer renderer, double timeInSeconds);
+		void DidApplyAnimations (ISCNSceneRenderer renderer, double timeInSeconds);


spouliot · 2017-06-08T13:19:00Z

src/scenekit.cs


 		[Mac (10,10)]
 		[Export ("renderer:didSimulatePhysicsAtTime:")]
-		void DidSimulatePhysics ([Protocolize]SCNSceneRenderer renderer, double timeInSeconds);
+		void DidSimulatePhysics (ISCNSceneRenderer renderer, double timeInSeconds);


dalexsoto

Very good job, just some minor details 👍

dalexsoto · 2017-06-08T14:11:39Z

src/arkit.cs

+	[BaseType (typeof(NSObject))]
+	[DisableDefaultCtor]
+	interface ARCamera : NSCopying
+	{


Minor, Codig guidelines, should be on the above line. Also this is present in all interfaces

dalexsoto · 2017-06-08T14:12:10Z

src/arkit.cs

+
+	[iOS (11,0)]
+	[NoWatch, NoTV, NoMac]
+	[BaseType (typeof(NSObject))]


Minor, coding guidelines space typeof (NSObject), also present in all typeof declarations

rolfbjarne · 2017-06-15T11:39:27Z

src/arkit.cs

+	[BaseType (typeof(ARAnchor))]
+	interface ARPlaneAnchor
+	{
+		// [Export ("initWithTransform:")] marked as NS_UNAVAILABLE


Are there any constructors? Is [DisableDefaultCtor] needed?

Good catch, it's not available on ARAnchor so not on ARPlaneAnchor either.

rolfbjarne · 2017-06-15T11:44:11Z

src/arkit.cs

+	{
+		[Internal]
+		[Field ("ARSCNDebugOptionShowWorldOrigin")]
+		int _ARSCNDebugOptionShowWorldOrigin { get; }


Shouldn't this be nint?

@rolfbjarne Here's the enum signature: public enum SCNDebugOptions : nuint

If I use nuint instead of int I'm getting: error BI1014: bgen: Unsupported type for Fields: global::System.UInt32

If I use nint instead of int I'm getting: ARKit/ARSCNDebugOptions.g.cs(53,65): error CS0030: Cannot convert type 'System.nint' to 'SceneKit.SCNDebugOptions'

It looks like it's time to look at the generator again 😄

Both of those (nuint/nint) should work.

rolfbjarne · 2017-06-15T11:44:31Z

src/arkit.cs

+
+		[Internal]
+		[Field ("ARSCNDebugOptionShowFeaturePoints")]
+		int _ARSCNDebugOptionShowFeaturePoints { get; }


monojenkins · 2017-06-15T22:45:08Z

Build failure

monojenkins · 2017-06-15T23:29:30Z

Build failure

monojenkins · 2017-06-16T17:04:46Z

Build failure

spouliot

Please include a link to your test app in the commit message

VincentDondain · 2017-06-16T18:47:22Z

@spouliot I did, it's on the commit message, the PR message and I will keep it when squashing.

VincentDondain · 2017-06-16T19:59:15Z

build

monojenkins · 2017-06-16T21:37:37Z

Build failure

spouliot · 2017-06-17T17:01:10Z

The intro error is likely because the framework does not exists on the sim
Make sure it works on device (without a model)
If that's the case then we need to exclude it from sim tests

monojenkins · 2017-06-17T18:41:54Z

Build failure

VincentDondain · 2017-06-19T16:56:19Z

The intro error is likely because the framework does not exists on the sim

@spouliot looks like not: https://gist.github.com/VincentDondain/65785d63d6daa85a0b83a54245642616

This is intro on an iOS 11 device with [Model] disabled for ARSessionObserver.

dalexsoto · 2017-06-19T17:01:07Z

@VincentDondain The introspection tests are correct 😉

You had in your first commit

[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[Protocol]
[BaseType (typeof(NSObject))]
interface ARSessionObserver

If no ModelAttribute then also no BaseType should be present so just remove Model and BaseType and your binding should be good:

[iOS (11,0)]
[NoWatch, NoTV, NoMac]
[Protocol]
interface ARSessionObserver

VincentDondain · 2017-06-19T17:08:18Z

Aaaaaaaah indeed indeed, nice catch thanks! <3

dalexsoto · 2017-06-19T17:09:57Z

I totally missed that in the first review so we are even 😛

monojenkins · 2017-06-19T19:05:26Z

Build failure

VincentDondain · 2017-06-19T19:26:45Z

Unrelated mtouch failures. Tracked here: https://bugzilla.xamarin.com/show_bug.cgi?id=57601

VincentDondain · 2017-06-19T20:10:08Z

@rolfbjarne and you re-review and approve (especially since there are some bindings-generator.cs and generator.cs changes (:

monojenkins · 2017-06-19T21:10:11Z

Build failure

dalexsoto · 2017-06-20T02:04:57Z

src/arkit.cs

+		[Field ("ARSCNDebugOptionShowWorldOrigin")]
+		// FIXME: Could be using [BindAs (typeof (SCNDebugOptions))]: https://bugzilla.xamarin.com/show_bug.cgi?id=57537
+		// FIXME: Should be nuint: https://bugzilla.xamarin.com/show_bug.cgi?id=57535
+		int _ARSCNDebugOptionShowWorldOrigin { get; }


You can use nuint / nint / nfloat only if XAMCORE_2_0 btw

You also need to use nuint/nint for in Unified, otherwise it will be wrong for 64-bit code.

rolfbjarne

Can you check in the test project somewhere? Maybe xamarin/ios-samples, or a personal git repo. This makes it possible for others to fix any bugs 😄

rolfbjarne · 2017-06-20T12:07:27Z

src/arkit.cs

+		[Field ("ARSCNDebugOptionShowWorldOrigin")]
+		// FIXME: Could be using [BindAs (typeof (SCNDebugOptions))]: https://bugzilla.xamarin.com/show_bug.cgi?id=57537
+		// FIXME: Should be nuint: https://bugzilla.xamarin.com/show_bug.cgi?id=57535
+		int _ARSCNDebugOptionShowWorldOrigin { get; }


You also need to use nuint/nint for in Unified, otherwise it will be wrong for 64-bit code.

rolfbjarne · 2017-06-20T12:08:26Z

src/arkit.cs

+		[Field ("ARSCNDebugOptionShowFeaturePoints")]
+		// FIXME: Could be using [BindAs (typeof (SCNDebugOptions))]: https://bugzilla.xamarin.com/show_bug.cgi?id=57537
+		// FIXME: Should be nuint: https://bugzilla.xamarin.com/show_bug.cgi?id=57535
+		int _ARSCNDebugOptionShowFeaturePoints { get; }


Same: you must use a type of the right size.

VincentDondain · 2017-06-20T15:10:22Z

@rolfbjarne https://github.com/VincentDondain/ios-testcases/tree/master/ARPointCloudTest

dalexsoto · 2017-06-20T18:47:14Z

@VincentDondain if you rebase you should be able to use FieldAttribite with enums now

VincentDondain · 2017-06-20T18:50:05Z

Woot will try, thanks @dalexsoto !

Updated scenekit.cs `SCNSceneRendererDelegate` to avoid: `ARKit/ARSCNViewDelegate.g.cs(189,43): error CS0432: Alias `Iglobal' not found`

- Fixed the generator to avoid `ARKit/ARSCNViewDelegate.g.cs(189,43): error CS0432: Alias 'Iglobal' not found` - ARPointCloud can be tested with this samples: https://www.dropbox.com/s/0xlz0sdzjj4507s/ARPointCloudTest.zip?dl=0 It requires deploying on devices and moving the camera around so ARKit can return some points. After 5 seconds it should print the points. - Fixed styling. - Added missing simd. - Fixed intro tests. Note: not having [Model] on ARSessionObserver got us: ``` [FAIL] iOSApiProtocolTest.ApiProtocolTest.GeneralCase : 1 types do not really conform to the protocol interfaces [FAIL] Could not load ARKit.ARSessionObserver at Introspection.ApiBaseTest.AssertIfErrors (System.String s, System.Object[] parameters) [0x00054] in /Users/vidondai/Documents/xi-xcode9/xamarin-macios/tests/introspection/ApiBaseTest.cs:143 at Introspection.ApiProtocolTest.GeneralCase () [0x001ec] in /Users/vidondai/Documents/xi-xcode9/xamarin-macios/tests/introspection/ApiProtocolTest.cs:342 at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in /Users/vidondai/Documents/xi-xcode9/xamarin-macios/external/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 ```

VincentDondain · 2017-06-20T21:11:10Z

Need #2233 for the enum with Field to work (;

monojenkins · 2017-06-20T21:15:10Z

Build failure

monojenkins · 2017-06-20T21:55:12Z

Build failure

VincentDondain · 2017-06-21T15:06:00Z

build

monojenkins · 2017-06-21T18:17:40Z

Build failure

monojenkins · 2017-06-22T16:59:48Z

Build success

VincentDondain · 2017-06-22T23:56:28Z

Woot ARKit is now green!!! Merging.

dalexsoto · 2017-06-22T23:58:43Z

Merge, Merge, Merge!!

VincentDondain requested review from dalexsoto, rolfbjarne and spouliot June 8, 2017 01:11

dnfclas added the cla-already-signed label Jun 8, 2017

spouliot suggested changes Jun 8, 2017

View reviewed changes

dalexsoto requested changes Jun 8, 2017

View reviewed changes

rolfbjarne requested changes Jun 15, 2017

View reviewed changes

VincentDondain force-pushed the arkit-b1 branch 2 times, most recently from d0443db to c6a5164 Compare June 15, 2017 22:45

VincentDondain force-pushed the arkit-b1 branch from c6a5164 to e2cb18b Compare June 16, 2017 15:26

spouliot approved these changes Jun 16, 2017

View reviewed changes

spouliot closed this Jun 17, 2017

spouliot reopened this Jun 17, 2017

dnfclas added the cla-already-signed label Jun 17, 2017

dalexsoto approved these changes Jun 19, 2017

View reviewed changes

dalexsoto reviewed Jun 20, 2017

View reviewed changes

rolfbjarne requested changes Jun 20, 2017

View reviewed changes

VincentDondain added 3 commits June 20, 2017 15:16

[arkit] Update for iOS 11 beta 1

5b31bc5

Updated scenekit.cs `SCNSceneRendererDelegate` to avoid: `ARKit/ARSCNViewDelegate.g.cs(189,43): error CS0432: Alias `Iglobal' not found`

[arkit] Remove Model and BaseType on ARSessionObserver

c0a6fad

VincentDondain force-pushed the arkit-b1 branch from 2dae51c to e81fef2 Compare June 20, 2017 21:07

[arkit] Add XAMCORE_2_0, fix ARSCNDebugOptions

eb3c3bd

VincentDondain force-pushed the arkit-b1 branch from e81fef2 to eb3c3bd Compare June 20, 2017 21:10

rolfbjarne approved these changes Jun 21, 2017

View reviewed changes

Merge branch 'xcode9' into arkit-b1

aec89fa

VincentDondain merged commit ad90642 into xamarin:xcode9 Jun 23, 2017

VincentDondain deleted the arkit-b1 branch June 23, 2017 00:02

[arkit] Update for iOS 11 beta 1 (new framework) #2187

[arkit] Update for iOS 11 beta 1 (new framework) #2187

Conversation

VincentDondain commented Jun 8, 2017 • edited Loading

monojenkins commented Jun 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalexsoto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Jun 15, 2017

monojenkins commented Jun 15, 2017

monojenkins commented Jun 16, 2017

spouliot left a comment

Choose a reason for hiding this comment

VincentDondain commented Jun 16, 2017

VincentDondain commented Jun 16, 2017

monojenkins commented Jun 16, 2017

spouliot commented Jun 17, 2017

monojenkins commented Jun 17, 2017

VincentDondain commented Jun 19, 2017 • edited Loading

dalexsoto commented Jun 19, 2017 • edited Loading

VincentDondain commented Jun 19, 2017

dalexsoto commented Jun 19, 2017

monojenkins commented Jun 19, 2017

VincentDondain commented Jun 19, 2017

VincentDondain commented Jun 19, 2017

monojenkins commented Jun 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolfbjarne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VincentDondain commented Jun 20, 2017

dalexsoto commented Jun 20, 2017

VincentDondain commented Jun 20, 2017

VincentDondain commented Jun 20, 2017

monojenkins commented Jun 20, 2017

monojenkins commented Jun 20, 2017

VincentDondain commented Jun 21, 2017

monojenkins commented Jun 21, 2017

monojenkins commented Jun 22, 2017

VincentDondain commented Jun 22, 2017

dalexsoto commented Jun 22, 2017

VincentDondain commented Jun 8, 2017 •

edited

Loading

VincentDondain commented Jun 19, 2017 •

edited

Loading

dalexsoto commented Jun 19, 2017 •

edited

Loading