Android: make `slint::android` public #4616

ogoffart · 2024-02-14T17:01:30Z

Add a backend-android-native-acticity feature that enables the slint::android_* re-exports.

This doesn't expose the game-activity feature because it is harder to use and is not well tested, and I don't think bring much value at this point.

Edit: feature renamed to backend-android-activity-05 and added android module

ogoffart · 2024-02-14T17:03:46Z

api/rs/slint/Cargo.toml

@@ -157,6 +157,9 @@ backend-linuxkms = ["i-slint-backend-selector/backend-linuxkms", "std"]
 ## windowing system. (Experimental)
 backend-linuxkms-noseat = ["i-slint-backend-selector/backend-linuxkms-noseat", "std"]

+## Use the backend based on the [android-activity](https://docs.rs/android-activity) crate. (Using it's native activity feature)
+backend-android-native-activity = ["i-slint-backend-android-activity/native-activity"]


This is the name of the feature.
Other option include:

backend-android-activity-native since it is using the native feature of the android-activity backend.

backend-android-activity-0-5 of some variation of this to encode the version number of the android activity crate

Would it perhaps make sense to synchronize a prefix of the feature name with the rust module?

Like we do for the software renderer and femtovg.

Like backend-android-activity-native and slint::backends::android_activity::*.

(This is a case of precedence, whatever we do here we can (and perhaps should) also do for other backends)

Yes, i agree about the common prefix.
So I'll rename it backend-android-activity-native
Or should it be backend-android-activity-native-0-5 or backend-android-activity-native-05 or backend-android-activity-05-native ?

If I understand the intention of supporting multiple versions simultaneously correctly, then I'd go for backend-android-activity-0-5 and say that this convenience feature locks in the common case. If you prefer to include the feature name then I'd go for the long way backend-android-activity-0-5-native-activity.

(This is where a separate crate would scale better)

ogoffart · 2024-02-14T17:06:50Z

api/rs/slint/lib.rs

+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+mod android;
+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+pub use android::*;


I've put everything inline as that's what we usually do. (as opposed to a public android module)

But the alternative would be to have slint::android::init and re-export under slint::android::android_activity.

The advantage is that we could put more documentation in the module docs instead of in the android_init function.

But given that there is not much API surface, I think it's fine.

I'd prefer a module over inlining the functions. The docs argument is good (see fake mcu module...), and I have the sinking feeling that we might have more functionality specific to Android in the future.

I'm surprised by your position as you've always been defending flat namespace in the past :-)

Then you you suggest that the user code look like:

fn android_main(app: slint::android::android_activity::AndroidApp) { slint::android_init(app).unwrap();

Ok, slint::android::android_activity::AndroidApp could be imported. I think if it is a module, i can re-export it as slint::android::AndroidApp.

I have the sinking feeling that we might have more functionality specific to Android in the future.

That's possible although they may not belong there. Could just be extra function on slint::Window or so. (Could also be extension trait, but the fact you need to import them is not great IMHO)

Regarding the name of the module, I think android is simple and obvious.
I don't think slint::android_activity is a good name for the module because it is confusing with the crate name (and then it would be slint::android_activity::android_activity to access the android_activity crate?)
And i'm not expecting other android backends
Maybe having slint::backends::android is fine but do we expect to have more API for backends? I guess we want to export winit at some point.

An alternative to a module is also to make it a different crate:
slint-android or something like that.

Hah, did I defend that? I thought I was the one who proposed to put the BorrowedOpenGLTextureBuilder into a gpu module.:)

I agree that we don't need android_activity in the name, since that'll be the only support crate to use.

Yes, I think that perhaps slint::backends::android (which contains all types needed without an android_activity sub-module) looks better next to slint::backends::winit, as opposed to slint::winit, which I'm unsure about how it should look like. Purely by name it sounds like a straight re-export of winit, but it wouldn't be. So either backends::winit (which has the types from our crate) or slint::backend_winit looks better to me.

api/rs/slint/android.rs

examples/todo/rust/main.rs

Add a `backend-android-native-acticity` feature that enables the `slint::android_*` re-exports.

tronical

I think this is a good idea and the right time. Comments inline about the API/feature name.

The main open question to me is how we decide to handle the semver divergence - as whatever we do here we should also apply to winit and skia crates that we can expose / re-export.

My guts feeling is that it would be worse for us to support multiple android-activity/winit major versions. Maaaybe the surface with Android is little (winit is certainly subject to many more changes and the upcoming trait improvements are a good example). So we could treat them differently, but that doesn't feel great.

At the moment my preference would be to have a public slint-backend-android-activity-0-5 crate. It makes the search on crates.io worse, but I'd think people would find the dependency choice through our docs instead anyway.

We could say that once such a "public" dependency of ours hits v1.0, we could re-export it.

tronical · 2024-02-15T08:58:29Z

api/rs/slint/android.rs

+/// cargo apk run --target aarch64-linux-android --lib
+/// ```
+///
+/// Please ensure that you have the Android NDK and SDK installed and properly set up in your development environment for the above command to work as expected.


"Please ensure" without a link to docs that explains how to ensure it is not good IMO. Think of the target audience of an existing Rust dev who wants to expand to Android. The better our docs, the smoother the experience, the faster they're up and running, and the better it reflects on Slint.

(I think this might be out of scope for this PR, but I'll keep whining about it :-)

I agree. I can try to find a link that help with the setup.

tronical · 2024-02-15T08:59:51Z

api/rs/slint/lib.rs

+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+mod android;
+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+pub use android::*;


I'd prefer a module over inlining the functions. The docs argument is good (see fake mcu module...), and I have the sinking feeling that we might have more functionality specific to Android in the future.

tronical · 2024-02-15T09:04:05Z

api/rs/slint/Cargo.toml

@@ -157,6 +157,9 @@ backend-linuxkms = ["i-slint-backend-selector/backend-linuxkms", "std"]
 ## windowing system. (Experimental)
 backend-linuxkms-noseat = ["i-slint-backend-selector/backend-linuxkms-noseat", "std"]

+## Use the backend based on the [android-activity](https://docs.rs/android-activity) crate. (Using it's native activity feature)
+backend-android-native-activity = ["i-slint-backend-android-activity/native-activity"]


Would it perhaps make sense to synchronize a prefix of the feature name with the rust module?

Like we do for the software renderer and femtovg.

Like backend-android-activity-native and slint::backends::android_activity::*.

(This is a case of precedence, whatever we do here we can (and perhaps should) also do for other backends)

ogoffart

My guts feeling is that it would be worse for us to support multiple android-activity/winit major versions. [...]
At the moment my preference would be to have a public slint-backend-android-activity-0-5 crate.

Having it in the same crate (different modules) makes it easier to to share code between implementations if we maintain two different implementation in parallel using macro or other tricks.

The question is whether we can stop supporting android-activity 0.5 by not releasing slint-android-activity-05 from a given version?

Imagine i'm doing a rust widget library that uses slint="1.5"
If Slint 1.6 no longer release the android crate that some people depend on, i think it would be a semver breaking change to do slint="1.6" in the widget library because if someone uses the android crate, it would fail to compile.

So the approach you suggest is to rename i-slint-backend-android-activity to slint-backend-android-activity-0-5 and hide the backend, but expose some init function?
Then thje main would look like:

fn android_main(app: slint_backend_android_activity_0_5::android_activity::AndroidApp) {
    slint_backend_android_activity_0_5::init(app).unwrap();

granted you don't reach into that namespace often, it's fairly bad.

ogoffart · 2024-02-15T10:15:18Z

api/rs/slint/Cargo.toml

@@ -157,6 +157,9 @@ backend-linuxkms = ["i-slint-backend-selector/backend-linuxkms", "std"]
 ## windowing system. (Experimental)
 backend-linuxkms-noseat = ["i-slint-backend-selector/backend-linuxkms-noseat", "std"]

+## Use the backend based on the [android-activity](https://docs.rs/android-activity) crate. (Using it's native activity feature)
+backend-android-native-activity = ["i-slint-backend-android-activity/native-activity"]


Yes, i agree about the common prefix.
So I'll rename it backend-android-activity-native
Or should it be backend-android-activity-native-0-5 or backend-android-activity-native-05 or backend-android-activity-05-native ?

ogoffart · 2024-02-15T10:16:17Z

api/rs/slint/android.rs

+/// cargo apk run --target aarch64-linux-android --lib
+/// ```
+///
+/// Please ensure that you have the Android NDK and SDK installed and properly set up in your development environment for the above command to work as expected.


I agree. I can try to find a link that help with the setup.

ogoffart · 2024-02-15T10:35:36Z

api/rs/slint/lib.rs

+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+mod android;
+#[cfg(any(doc, all(target_os = "android", feature = "backend-android-native-activity")))]
+pub use android::*;


I'm surprised by your position as you've always been defending flat namespace in the past :-)

Then you you suggest that the user code look like:

fn android_main(app: slint::android::android_activity::AndroidApp) { slint::android_init(app).unwrap();

Ok, slint::android::android_activity::AndroidApp could be imported. I think if it is a module, i can re-export it as slint::android::AndroidApp.

I have the sinking feeling that we might have more functionality specific to Android in the future.

That's possible although they may not belong there. Could just be extra function on slint::Window or so. (Could also be extension trait, but the fact you need to import them is not great IMHO)

Regarding the name of the module, I think android is simple and obvious.
I don't think slint::android_activity is a good name for the module because it is confusing with the crate name (and then it would be slint::android_activity::android_activity to access the android_activity crate?)
And i'm not expecting other android backends
Maybe having slint::backends::android is fine but do we expect to have more API for backends? I guess we want to export winit at some point.

An alternative to a module is also to make it a different crate:
slint-android or something like that.

tronical · 2024-02-15T12:27:18Z

My guts feeling is that it would be worse for us to support multiple android-activity/winit major versions. [...]
At the moment my preference would be to have a public slint-backend-android-activity-0-5 crate.

Having it in the same crate (different modules) makes it easier to to share code between implementations if we maintain two different implementation in parallel using macro or other tricks.

The question is whether we can stop supporting android-activity 0.5 by not releasing slint-android-activity-05 from a given version?

IMHO yes, we should stop supporting 0.5 once we switch to 0.6. I mean, we couuuuldd do it probably for android-activity, but IMHO the cost outweighs the benefits for candidates like winit or skia-safe. So we could make an exception, but I'm first looking for agreement on the common case.

Imagine i'm doing a rust widget library that uses slint="1.5" If Slint 1.6 no longer release the android crate that some people depend on, i think it would be a semver breaking change to do slint="1.6" in the widget library because if someone uses the android crate, it would fail to compile.

Why would it be breaking? Suppose the rust widget library does not re-export anything from android-activity, then wouldn't the app use 0.6 (from slint 1.6) and whatever version it accesses directly?

If android-activity uses links = "..." then I can see that it would be breaking, but what else could break?

So the approach you suggest is to rename i-slint-backend-android-activity to slint-backend-android-activity-0-5 and hide the backend, but expose some init function? Then thje main would look like:
fn android_main(app: slint_backend_android_activity_0_5::android_activity::AndroidApp) {
    slint_backend_android_activity_0_5::init(app).unwrap();
granted you don't reach into that namespace often, it's fairly bad.

Yes, that's what I'm suggesting. And if android-activity commits to v1.0, then I'd say we should add the feature to the slint crate itself and re-export it.

Differently put:

A pre v1.0 dependency XXX gets their own slint-whatever-XXX-0_ZZ crate, where ZZ follows the latest version (that we want to support).
Once XXX dependency commits to v1.0, we make it easier to access via a slint crate feature and a re-exported module.

It's still not ideal though, as this falls short when say slint-backend-winit also wants to use glutin API in the public API.

Orthogonal: I really like it when a public dependency is re-exported as a whole, for easier use. So slint-backend-android-activity-0-5 re-exports all of its android_activity crate, so that apps using this slint-backend-android-activity-0-5 crate don't need to also depend on android-activity and keep it in sync.

Do I understand your proposal correctly that you'd like to only make XXX dependencies public in the slint API if we also commit to supporting all released versions in parallel until slint does a semver bump?

tronical · 2024-02-15T13:09:56Z

A few thoughts later, I think we can separate the winit side angle I introduced to the discussion. I'm fine with with a feature in slint, which exposes new API, and us supporting all future android-activity versions until our next semver break.

ogoffart · 2024-02-15T17:24:31Z

Why would it be breaking? Suppose the rust widget library does not re-export anything from
android-activity, then wouldn't the app use 0.6 (from slint 1.6) and whatever version it
accesses directly?

Right, it might not be

Because the users of the widget library might have this in their cagto.toml

sint-backend-android-activity = "1.5" # depends on "i-sliny-core=1.5.x"
slint = "1.5" # even when 1.6 is released, the resolver can't pick it because of i-slint-core
widgets-lib = "1.2" # when a new version depends on Slint 1.6, the resolver can't pick it

I think the resolver is smart enough to figure it. so that may work, even if confusing

Orthogonal: I really like it when a public dependency is re-exported as a whole, for easier
use.

Yes, I like it too.
But we also can't prevent user to import it directly or mixing with a 3rd party crate that use
also android-activity (eg, ndk)

Do I understand your proposal correctly that you'd like to only make xxx dependencies public
in the slint API if we also commit to supporting all released versions in parallel until slint
does a semver bump?

Yes, that's what I'm suggesting.
I think from an API point of view it is better for our users
But I agree that committing to maintain the crate for all the life time of major version is a
burden. And we may regret this.

Once XXX dependency commits to v1.0, we make it easier to access via a slint crate feature
and a re-exported module

Note that 1.0 doesn't say how often they are going to make semver breaks.
But yeah, in principle it is a stronger sign.

A few thoughts later, I think we can separate the winit side angle I introduced to the discussion

Right, the winit is another topic and we might pick a different decision when it comes to it.

I'm fine with with a feature in slint, which exposes new API, and us supporting all future android-activity versions until our next semver break.

So an android module then?
and backend-android-activity-05 feature.

tronical · 2024-02-16T01:06:11Z

So an android module then?
and backend-android-activity-05 feature.

Sounds good to me.

api/rs/slint/android.rs

Co-authored-by: Simon Hausmann <simon.hausmann@slint.dev>

ogoffart requested a review from tronical February 14, 2024 17:01

ogoffart commented Feb 14, 2024

View reviewed changes

github-actions bot reviewed Feb 14, 2024

View reviewed changes

api/rs/slint/android.rs Outdated Show resolved Hide resolved

examples/todo/rust/main.rs Outdated Show resolved Hide resolved

ogoffart force-pushed the olivier/android branch from 7c5d244 to 12124c6 Compare February 15, 2024 08:34

Android: make slint::android_init public

5c762f9

Add a `backend-android-native-acticity` feature that enables the `slint::android_*` re-exports.

ogoffart force-pushed the olivier/android branch from 12124c6 to 5c762f9 Compare February 15, 2024 08:51

tronical reviewed Feb 15, 2024

View reviewed changes

ogoffart commented Feb 15, 2024

View reviewed changes

Rename the feature and put the android function into a module

faa4b31

ogoffart changed the title ~~Android: make slint::android_init public~~ Android: make slint::android public Feb 20, 2024

tronical approved these changes Feb 20, 2024

View reviewed changes

api/rs/slint/android.rs Outdated Show resolved Hide resolved

api/rs/slint/android.rs Outdated Show resolved Hide resolved

Apply suggestions from code review

bda4ee2

Co-authored-by: Simon Hausmann <simon.hausmann@slint.dev>

ogoffart merged commit ddebd64 into master Feb 20, 2024
35 checks passed

ogoffart deleted the olivier/android branch February 20, 2024 14:00

ogoffart added the a:platform-android Android platform integration (mO,bS) label Mar 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android: make `slint::android` public #4616

Android: make `slint::android` public #4616

ogoffart commented Feb 14, 2024 •

edited

Loading

ogoffart Feb 14, 2024

tronical Feb 15, 2024

ogoffart Feb 15, 2024

tronical Feb 15, 2024

ogoffart Feb 14, 2024

tronical Feb 15, 2024

ogoffart Feb 15, 2024

tronical Feb 15, 2024

tronical left a comment

tronical Feb 15, 2024

ogoffart Feb 15, 2024

tronical Feb 15, 2024

tronical Feb 15, 2024

ogoffart left a comment

ogoffart Feb 15, 2024

ogoffart Feb 15, 2024

ogoffart Feb 15, 2024

tronical commented Feb 15, 2024

tronical commented Feb 15, 2024

ogoffart commented Feb 15, 2024

tronical commented Feb 16, 2024

Android: make slint::android public #4616

Android: make slint::android public #4616

Conversation

ogoffart commented Feb 14, 2024 • edited Loading

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

tronical 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

ogoffart 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

tronical commented Feb 15, 2024

tronical commented Feb 15, 2024

ogoffart commented Feb 15, 2024

tronical commented Feb 16, 2024

Android: make `slint::android` public #4616

Android: make `slint::android` public #4616

ogoffart commented Feb 14, 2024 •

edited

Loading