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

[Mono.Android] Fix missing enum issues that cause BG8800 warnings. #8707

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Feb 7, 2024

Fixes: #8703

In #8703, we do not generate HardwareBuffer.create because we attempted to map the usage parameter to an enum, but no enum was created as the values are long instead of int.

Manually create a long HardwareBufferUsage enum. However, generator does not support long enums, and generates marshalling code using an int. Thus, we need to manually bind create and getUsage so we can fix the int machinery to long.

While we're at it, audit all of the BG8800 warnings that are caused by improper enumification and fix them:

warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.Create (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware.Hardw...
warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.IsSupported (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware....
warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context'.
warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context.BindIsolatedService (Android.Content.Intent, Android.App.Bind, java.lang.String, java.util.concurrent.Executor, ...
warning BG8800: Unknown parameter type 'Android.Graphics.ImageDecoderAllocatorType' for member 'Android.Graphics.ImageDecoder.SetAllocator (Android.Graphics.ImageDecoderAllocatorType)'.
warning BG8800: Unknown parameter type 'Android.Net.WpsFailureReason' for member 'Android.Net.Wifi.WifiManager.WpsCallback.OnFailed (Android.Net.WpsFailureReason)'.
warning BG8800: Unknown parameter type 'Android.OS.DeviceTemperatureSource' for member 'Android.OS.HardwarePropertiesManager.GetDeviceTemperatures (Android.OS.DeviceTemperatureType, Android.OS.DeviceTempera...
warning BG8800: Unknown parameter type 'Android.Telephony.Mbms.DownloadStatus' for member 'Android.Telephony.Mbms.DownloadStatusListener.OnStatusUpdated (Android.Telephony.Mbms.DownloadRequest, Android.Tel...
warning BG8800: Unknown parameter type 'Android.Telephony.StreamingMethod' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamMethodUpdated (Android.Telephony.StreamingMethod)'.
warning BG8800: Unknown parameter type 'Android.Telephony.StreamingState' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamStateUpdated (Android.Telephony.StreamingState, Android.Telepho...
warning BG8800: Unknown parameter type 'Android.Icu.Text.CollatorReorderCodes' for member 'Android.Icu.Text.Collator.GetEquivalentReorderCodes (Android.Icu.Text.CollatorReorderCodes)'.
warning BG8800: Unknown parameter type 'params Android.Icu.Text.CollatorReorderCodes[]' for member 'Android.Icu.Text.Collator.SetReorderCodes (params Android.Icu.Text.CollatorReorderCodes[])'.

This results in new API being surfaced that was previously not being bound, requiring updates to PublicAPI.Unshipped.txt.

One of these new APIs WpsCallback.OnFailed(int) is an abstract method on an existing type. Although this is a breaking change, this type previously could not have been implemented as the Java abstract method would not have been implementable. Thus, add it as an "acceptable breakage".

@Steph55
Copy link

Steph55 commented Feb 8, 2024

Thank you very much! I also have problem finding another function, maybe I should make another issue for it? Anyhow, here it is:

I cannot find the Xamarin's function for androidx.opengl.EGLExt.glEGLImageTargetTexture2DOES(). This function is not well documented, but it appears in one Google Android sample in the module https://github.com/android/renderscript-samples/blob/main/RenderScriptMigrationSample/app/src/main/java/com/android/example/rsmigration/GLSLImageProcessor.kt at line 627. This is the module that I am trying to port to Xamarin.

Please note that this function is not the same as GLES11Ext.GlEGLImageTargetTexture2DOES() since the last parameter is different. I need the first function in order to use a EGLImageKHR as a Texture2D.

@jpobst
Copy link
Contributor Author

jpobst commented Feb 8, 2024

It looks like androidx.opengl.EGLExt.glEGLImageTargetTexture2DOES() is bound in an AndroidX package, which is this repository:
https://github.com/xamarin/AndroidX

The artifact is androidx.graphics:graphics-core:
https://developer.android.com/jetpack/androidx/releases/graphics

However, it does not have a stable release yet, and we do not generally bind pre-release packages:
https://maven.google.com/web/index.html?#androidx.graphics:graphics-core

@jpobst jpobst marked this pull request as ready for review February 8, 2024 18:18
@jpobst jpobst requested a review from jonpryor as a code owner February 8, 2024 18:18
@Steph55
Copy link

Steph55 commented Feb 9, 2024

The new EGLExt functions are even referenced in the official Android documentation like here: https://developer.android.com/guide/topics/renderscript/migrate/migrate-gles. One function described in this page is not in Xamarin: androidx.opengl.EGLExt.eglCreateImageFromHardwareBuffer(). Would you consider including them in Xamarin, in their current beta version?

@jpobst
Copy link
Contributor Author

jpobst commented Feb 9, 2024

If you need this AndroidX library before Google finishes it, your best bet is to create your own binding of it:
https://learn.microsoft.com/en-us/xamarin/android/platform/binding-java-library/
https://learn.microsoft.com/en-us/dotnet/maui/migration/android-binding-projects

@Steph55
Copy link

Steph55 commented Feb 9, 2024

Thank you for your suggestion, but am I to use the entire Android JAR to achieve this?

@jpobst
Copy link
Contributor Author

jpobst commented Feb 9, 2024

You would need to bind the .aar from Google's Maven repository:
https://maven.google.com/web/index.html?#androidx.graphics:graphics-core:1.0.0-beta01

@jonpryor
Copy link
Member

@jpobst: next (last?) "meta-question": given that we can't apply these changes to .NET 8 / API-34, should we wrap all these changes within #if ANDROID_35, to explicitly require that they only apply to API-35+ bindings?

That feels like the safer course of action, and would mean that if we accidentally applied this change to the .NET 8 release branch, we still would not alter API.

@jpobst
Copy link
Contributor Author

jpobst commented Feb 10, 2024

I considered it, but I feel like we are reasonably covered from that scenario with the fact that PublicAPI.Unshipped.txt has changed so much that it would definitely cause merge conflicts if we tried to backport this (intentionally or not):

But if we don't feel confident enough about that, I can add the #ifs.

@Steph55
Copy link

Steph55 commented Feb 13, 2024

Hi Jonathan!

Thanks a lot again for the information. It wasn't easy to get it working, I had to include all the biggest JAR files from the Android Studio install folders as external dependencies!

However, I have another question. How can I read the following key from the CaptureRequest keys SENSOR_DYNAMIC_BLACK_LEVEL ? This one and some others seem to be missing.

@Steph55
Copy link

Steph55 commented Feb 14, 2024

Sorry, I found the answer myself, I have to use TotalCaptureResult.Get(CaptureResult.SensorDynamicBlackLevel);

namespace Android.Hardware;

[global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android26.0")]
public enum HardwareBufferUsage : long
Copy link
Member

Choose a reason for hiding this comment

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

@jpobst: The values of these enum members look rather "flags-like"; should this be a [Flags] enum?

@jonpryor
Copy link
Member

Draft commit message:

Fixes: https://github.com/xamarin/xamarin-android/issues/8703

xamarin/xamarin-android#8730 reported that we did not bind
[`HardwareBuffer.create(int width, int height, int format, int layers, long usage)`][0].

Indeed, in the build logs for `src/Mono.Android`, there is a BG8800
warning about it!

	obj/Debug/net9.0/android-34/mcw/api-34.xml(32327,10): warning BG8800:
	  Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member
	  'Android.Hardware.HardwareBuffer.Create (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware.HardwareBufferUsage)'

The BG8800 was generated because we attempted to map `long usage` to
an `enum`, but no enum was created as the values are of type `long`,
not `int`.

Manually create a `long`-based `HardwareBufferUsage` enum:

	[Flags]
	/* partial */ enum HardwareBufferUsage : long {
	    None = 0,
	    UsageComposerOverlay = 0x800,
	    // …
	}

However, `generator` does not support `long` enums, and generates
marshalling code using an `int`.  Thus, we need to manually bind
`HardwareBuffer.create()` and [`HardwareBuffer.getUsage()`][1] so we
can replace the `int` machinery with `long`.

While we're at it, audit all of the `BG8800` warnings that are caused
by improper enumification and fix them:

	warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.Create (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware.Hardw...
	warning BG8800: Unknown parameter type 'Android.Hardware.HardwareBufferUsage' for member 'Android.Hardware.HardwareBuffer.IsSupported (int, int, Android.Hardware.HardwareBufferFormat, int, Android.Hardware....
	warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context'.
	warning BG8800: Unknown parameter type 'Android.App.Bind' for member 'Android.Content.Context.BindIsolatedService (Android.Content.Intent, Android.App.Bind, java.lang.String, java.util.concurrent.Executor, ...
	warning BG8800: Unknown parameter type 'Android.Graphics.ImageDecoderAllocatorType' for member 'Android.Graphics.ImageDecoder.SetAllocator (Android.Graphics.ImageDecoderAllocatorType)'.
	warning BG8800: Unknown parameter type 'Android.Net.WpsFailureReason' for member 'Android.Net.Wifi.WifiManager.WpsCallback.OnFailed (Android.Net.WpsFailureReason)'.
	warning BG8800: Unknown parameter type 'Android.OS.DeviceTemperatureSource' for member 'Android.OS.HardwarePropertiesManager.GetDeviceTemperatures (Android.OS.DeviceTemperatureType, Android.OS.DeviceTempera...
	warning BG8800: Unknown parameter type 'Android.Telephony.Mbms.DownloadStatus' for member 'Android.Telephony.Mbms.DownloadStatusListener.OnStatusUpdated (Android.Telephony.Mbms.DownloadRequest, Android.Tel...
	warning BG8800: Unknown parameter type 'Android.Telephony.StreamingMethod' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamMethodUpdated (Android.Telephony.StreamingMethod)'.
	warning BG8800: Unknown parameter type 'Android.Telephony.StreamingState' for member 'Android.Telephony.Mbms.StreamingServiceCallback.OnStreamStateUpdated (Android.Telephony.StreamingState, Android.Telepho...
	warning BG8800: Unknown parameter type 'Android.Icu.Text.CollatorReorderCodes' for member 'Android.Icu.Text.Collator.GetEquivalentReorderCodes (Android.Icu.Text.CollatorReorderCodes)'.
	warning BG8800: Unknown parameter type 'params Android.Icu.Text.CollatorReorderCodes[]' for member 'Android.Icu.Text.Collator.SetReorderCodes (params Android.Icu.Text.CollatorReorderCodes[])'.

This results in new API being surfaced that was previously not being
bound, requiring updates to
`src/Mono.Android/PublicAPI/API-34/PublicAPI.Unshipped.txt`.

One of these new APIs is [`WpsCallback.OnFailed(WpsFailureReason)`][2],
which is a *new* `abstract` method on an existing non-`abstract` type.
Although this is a breaking change, this type previously could not have
been inherited from as the Java-side `abstract` method would not have
been implemented; consider this C# code:

	// This C# code compiles, no warnings or errors:
	class MyCallback : Android.Net.Wifi.WifiManager.WpsCallback {
	    public override void OnStarted(string? pin) {}
	    public override void OnSucceeded() {}
	}

However, the `.csproj` containing `MyCallback` will fail to build:

	obj/Debug/net8.0-android/android/src/crc64475861335642e0f6/MyCallback.java(4,8):
	  javac error JAVAC0000:
	  error: MyCallback is not abstract and does not override abstract method onFailed(int) in WpsCallback

Thus, add it as an "acceptable breakage".

[0]: https://developer.android.com/reference/android/hardware/HardwareBuffer?hl=en#create(int,%20int,%20int,%20int,%20long)
[1]: https://developer.android.com/reference/android/hardware/HardwareBuffer?hl=en#getUsage()
[2]: https://developer.android.com/reference/android/net/wifi/WifiManager.WpsCallback?hl=en#onFailed(int)

@jonpryor
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor jonpryor merged commit c20d51f into main Mar 13, 2024
63 checks passed
@jonpryor jonpryor deleted the enum-fixes branch March 13, 2024 00:19
grendello added a commit that referenced this pull request Mar 13, 2024
* main:
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
grendello added a commit that referenced this pull request Mar 13, 2024
* main:
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
grendello added a commit that referenced this pull request Mar 15, 2024
* main:
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
  [Mono.Android] is now "trimming safe" (#8778)
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
grendello added a commit that referenced this pull request Mar 15, 2024
* main:
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
  [Mono.Android] is now "trimming safe" (#8778)
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
grendello added a commit that referenced this pull request Mar 20, 2024
* main:
  LEGO: Merge pull request 8818
  Bump to dotnet/installer@b40c44502d 9.0.100-preview.3.24165.20 (#8817)
  Bump com.android.tools:r8 from 8.2.47 to 8.3.37 (#8816)
  [Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795)
  Localized file check-in by OneLocBuild Task (#8815)
  [Xamarin.Android.Build.Tasks] Make all assemblies RID-specific (#8478)
  Localized file check-in by OneLocBuild Task (#8813)
  [Xamarin.Android.Build.Tasks] %(AndroidAsset.AssetPack) Support (#8631)
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
  [Mono.Android] is now "trimming safe" (#8778)
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
  [templates] Remove redundant "template" from display name. (#8773)
grendello added a commit that referenced this pull request Mar 20, 2024
* main:
  LEGO: Merge pull request 8818
  Bump to dotnet/installer@b40c44502d 9.0.100-preview.3.24165.20 (#8817)
  Bump com.android.tools:r8 from 8.2.47 to 8.3.37 (#8816)
  [Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795)
  Localized file check-in by OneLocBuild Task (#8815)
  [Xamarin.Android.Build.Tasks] Make all assemblies RID-specific (#8478)
  Localized file check-in by OneLocBuild Task (#8813)
  [Xamarin.Android.Build.Tasks] %(AndroidAsset.AssetPack) Support (#8631)
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
  [Mono.Android] is now "trimming safe" (#8778)
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
grendello added a commit that referenced this pull request Mar 20, 2024
* main: (99 commits)
  LEGO: Merge pull request 8818
  Bump to dotnet/installer@b40c44502d 9.0.100-preview.3.24165.20 (#8817)
  Bump com.android.tools:r8 from 8.2.47 to 8.3.37 (#8816)
  [Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795)
  Localized file check-in by OneLocBuild Task (#8815)
  [Xamarin.Android.Build.Tasks] Make all assemblies RID-specific (#8478)
  Localized file check-in by OneLocBuild Task (#8813)
  [Xamarin.Android.Build.Tasks] %(AndroidAsset.AssetPack) Support (#8631)
  [runtime] Remove the last vestiges of desktop builds (#8810)
  [ci] Don't auto-retry APK test suites. (#8811)
  [Microsoft.Android.Templates] Update EN l10n template strings (#8808)
  Bump to xamarin/Java.Interop/main@651de42 (#8809)
  [Mono.Android] is now "trimming safe" (#8778)
  [Mono.Android] Fix missing enum issues that cause BG8800 warnings. (#8707)
  Bump external/Java.Interop from `3436a30` to `5bca8ad` (#8803)
  Bump to xamarin/monodroid@77124dc1 (#8804)
  Bump to dotnet/installer@e911f5c82c 9.0.100-preview.3.24161.2 (#8802)
  Bump to xamarin/Java.Interop/main@3436a30 (#8799)
  [templates] Remove redundant "template" from display name. (#8773)
  Bump to xamarin/Java.Interop/main@a7e09b7 (#8793)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing method HardwareBuffer.Create(...)
3 participants