-
Notifications
You must be signed in to change notification settings - Fork 606
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
Introduce android-activity backend #3807
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall good to me. I'd change the listener API, but we can discuss this over API review. I really like how this otherwise keeps changes minimal to the example.
# FIXME: we shoudln't depend on slint, but the other way around | ||
slint = { workspace = true, features = ["compat-1-2"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, why does the backend depend on Slint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought at first that the backend would be using public API only and so that would simplify things with regards to publishing it with a different version and a name like slint-android 0.3.0
or something. But I guess this is futile.
Do you want me to go ahead and use the internal instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that makes sense yeah. Either that or remove the FIXME :)
fn android_main(app: i_slint_backend_android_activity::AndroidApp) { | ||
slint::platform::set_platform(Box::new( | ||
i_slint_backend_android_activity::AndroidPlatform::new_with_event_listener(app, |event| { | ||
eprintln!("Got event: {event:?}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to remove that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just as the example. Do you think it's bad to have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an example, I think it's weird yes :)
/// // ... your slint application ... | ||
/// } | ||
/// ``` | ||
pub fn new_with_event_listener( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it would make sense to make this not a "listener" but an event filter. So a function that we say will be called before we process the event, and the provider can choose to "consume" the event (i.e. prevent it from reaching Slint).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to call filter, but filter doesn't really make more sense.
The event that are there cannot really be filtering without introducing bugs.
What would be an example of an use case of filtering?
This also doesn't include the input event that are handled by Slint only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main example I can think of are new events that we don't handle. Such as the save/restore for example.
This is a backend that implement a slint::platform for android-activity
This also adapt the
todo
example so it can be run on android.In order to run it on android, the
#wasm#
lines in the todo/Cargo.toml need to be removed, and thetodo example can be run with cargo-apk:
cargo apk run -p todo --target aarch64-linux-android --lib
(and the proper env variable needs to be defined for the NDK and the SDK)This work is sponsored by NLnet
Part of #46