Skip to content

Remove unnecessary conditional defines from scenekit.cs #14503

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.

Sign up for GitHub

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

Merged
mandel-macaque merged 6 commits into from
Mar 31, 2022
Merged

Remove unnecessary conditional defines from scenekit.cs #14503

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54ba19d
Remove Defines
Mar 24, 2022
b4f7db4
Merge remote-tracking branch 'xamarin/main' into Defines-SceneKit
Mar 24, 2022
f907ab8
consolidate the usings to one
Mar 28, 2022
4252aed
add NoMacCatalyst
Mar 29, 2022
c48dcfb
Merge branch 'main' into Defines-SceneKit
mandel-macaque Mar 30, 2022
fa13064
Merge branch 'main' into Defines-SceneKit
mandel-macaque Mar 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions src/scenekit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,19 @@
using GameplayKit;
#endif

#if MONOMAC || WATCH || __MACCATALYST__
using EAGLContext = System.Object;
#endif

#if MONOMAC
using AppKit;

using GLContext = global::OpenGL.CGLContext;
#else
using UIKit;
using CAOpenGLLayer = System.Object;
using NSOpenGLContext = System.Object;
using NSOpenGLPixelFormat = System.Object;

#if HAS_OPENGLES
using OpenGLES;
Expand Down Expand Up @@ -1219,8 +1226,7 @@ interface SCNHitTestResult {
CGPoint GetTextureCoordinatesWithMappingChannel (nint channel);
}

#if MONOMAC
[iOS (8,0)]
Comment on lines -1222 to -1223

@mandel-macaque mandel-macaque Mar 25, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was that IOS attr doing there?!?!

[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 14, message: "Please use Metal instead of OpenGL API.")]
[NoMacCatalyst]
[BaseType (typeof (CAOpenGLLayer))]
Expand All @@ -1229,7 +1235,6 @@ interface SCNLayer : SCNSceneRenderer, SCNTechniqueSupport {
// [Export ("scene", ArgumentSemantic.Retain)]
// SCNScene Scene { get; set; }
}
#endif

[Watch (3,0)]
[iOS (8,0)]
Expand Down Expand Up @@ -1494,7 +1499,7 @@ interface SCNLightType {
NSString Area { get; }
}

#if MONOMAC
[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 10)]
[Static]
interface SCNLightAttribute {
Expand All @@ -1519,7 +1524,6 @@ interface SCNLightAttribute {
[Field ("SCNLightShadowFarClippingKey")]
NSString ShadowFarClippingKey { get; }
}
#endif

[Watch (3,0)]
[iOS (8,0)]
Expand Down Expand Up @@ -1784,15 +1788,13 @@ interface SCNSceneLoadingOptions {
[Export ("SCNSceneSourceLoading.OptionPreserveOriginalTopology")]
bool PreserveOriginalTopology { get; set; }

#if !TVOS && !WATCH
// note: generator's StrongDictionary does not support No* attributes yet

@rolfbjarne rolfbjarne Mar 28, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems important: does the generator's StrongDictionary support No* attributes now? If not, we can't remove the #if conditions (and if it does, then you can remove the comment).

@tj-devel709 tj-devel709 Mar 29, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chamons would you happen to know this one?

@chamons chamons Mar 29, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go check the generated code and see what it generates before and after your change.

[NoTV]
[NoWatch]
float ConvertUnitsToMeters { get; set; } /* 'floating value encapsulated in a NSNumber' probably a float since it's a graphics framework */
[NoTV]
[NoWatch]
bool ConvertToYUp { get; set; }
#endif

[Internal, Export ("SCNSceneSourceLoading.AnimationImportPolicyKey")]
NSString _AnimationImportPolicyKey { get; set; }
Expand Down Expand Up @@ -2498,35 +2500,28 @@ interface SCNProgram : NSCopying, NSSecureCoding {
[BaseType (typeof (NSObject))]
[Model, Protocol]
interface SCNProgramDelegate {
#if MONOMAC
#if XAMCORE_3_0

@chamons chamons Mar 25, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe removing XAMCORE_3_0 is a safe transformation.

@mandel-macaque mandel-macaque Mar 25, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have XAMCORE_3_0, right? Since NET == XAMCORE_4_0, The if was just around a NoiOS, so it gives no harm since the method was not there.

@tj-devel709 tj-devel709 Mar 25, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the #if XAMCORE_3_0's because I believe the compiler would not let me add another [NoiOS] outside of it (since it had duplicate attributes) and when I didn't add it, xtro would complain that things were being set for iOS but were not really there. I could be misremembering the above though.
I figured that if I have to add a #if !XAMCORE_3_0 to add the [NoiOS] there as well and the fact that it was already inside a #if MONOMAC and not being able to be called for iOS anyways, that it would maybe be okay to remove.

Any thoughts on this? @mandel-macaque @chamons

@rolfbjarne rolfbjarne Mar 28, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's some convoluted code... in general removing XAMCORE_3_0 isn't a safe transformation, but in this particular case, the code inside the XAMCORE_3_0 condition didn't do anything, because it was also inside a MONOMAC condition, thus the XAMCORE_3_0 condition is safe to remove.


[Unavailable (PlatformName.iOS)]
#endif
[NoTV][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 10)]
[Export ("program:bindValueForSymbol:atLocation:programID:renderer:")]
bool BindValue (SCNProgram program, string symbol, uint /* unsigned int */ location, uint /* unsigned int */ programID, SCNRenderer renderer);

#if XAMCORE_3_0
[Unavailable (PlatformName.iOS)]
#endif
[NoTV][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 10)]
[Export ("program:unbindValueForSymbol:atLocation:programID:renderer:")]
void UnbindValue (SCNProgram program, string symbol, uint /* unsigned int */ location, uint /* unsigned int */ programID, SCNRenderer renderer);
#endif

[Export ("program:handleError:")]
void HandleError (SCNProgram program, NSError error);

#if MONOMAC
#if XAMCORE_3_0
[NoiOS]
[NoTV, NoWatch]
#endif
[Deprecated (PlatformName.MacOSX, 10, 10, message: "Use the SCNProgram's Opaque property instead.")]
[NoMacCatalyst]
[Export ("programIsOpaque:")]
bool IsProgramOpaque (SCNProgram program);
#endif
}

[Watch (3,0)]
Expand Down Expand Up @@ -3058,15 +3053,14 @@ interface SCNSceneRenderer {
[Export ("context")]
IntPtr Context { get; }

#if MONOMAC
#if NET
[Abstract]
#endif
[NoTV][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 10)]
[NoiOS]
[Export ("currentTime")]
double CurrentTime { get; set; }
#endif

[Abstract]
[Export ("hitTest:options:")]
Expand Down Expand Up @@ -3322,11 +3316,10 @@ interface SCNText {
[Export ("containerFrame")]
CGRect ContainerFrame { get; set; }

#if MONOMAC
// removed in iOS8 beta 5 - but it was already existing in 10.8 ?
[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Export ("textSize")]
CGSize TextSize { get; }
#endif

[Export ("truncationMode", ArgumentSemantic.Copy)]
string TruncationMode { get; set; }
Expand Down Expand Up @@ -3489,31 +3482,31 @@ interface SCNView : SCNSceneRenderer, SCNTechniqueSupport {
// [Export ("scene", ArgumentSemantic.Retain)]
// SCNScene Scene { get; set; }

#if MONOMAC
[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Export ("backgroundColor", ArgumentSemantic.Copy)]
NSColor BackgroundColor { get; set; }
#endif

[Export ("allowsCameraControl")]
bool AllowsCameraControl { get; set; }

#if MONOMAC
[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 14, message: "Please use Metal instead of OpenGL API.")]
[Export ("openGLContext", ArgumentSemantic.Retain)]
[NullAllowed]
NSOpenGLContext OpenGLContext { get; set; }

[NoiOS][NoTV][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.MacOSX, 10, 14, message: "Please use Metal instead of OpenGL API.")]
[Export ("pixelFormat", ArgumentSemantic.Retain)]
[NullAllowed]
NSOpenGLPixelFormat PixelFormat { get; set; }
#elif !WATCH && !__MACCATALYST__

[NoMac][NoWatch][NoMacCatalyst]
[Deprecated (PlatformName.iOS, 12, 0, message: "Please use Metal instead of OpenGL API.")]
[Deprecated (PlatformName.TvOS, 12, 0, message: "Please use Metal instead of OpenGL API.")]
[Export ("eaglContext", ArgumentSemantic.Retain)]
[NullAllowed]
EAGLContext EAGLContext { get; set; }
#endif

#if !WATCH
[iOS (9,0)][Mac (10,11)]
Expand Down