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

Dart bindings #166

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Dart bindings #166

wants to merge 10 commits into from

Conversation

Dampfwalze
Copy link
Contributor

@Dampfwalze Dampfwalze commented Aug 14, 2024

Dart bindings on top of driver_ipc using flutter_rust_bridge.

It uses an experimental feature, called native_assets, to build and link with the rust side. native_assets introduces a build.dart file, similar to build.rs.

Dart bindings can be regenerated using cargo make generate.

Unit tests can be run using cargo make test.

Hack

For now it relies on a hack, because flutter_rust_bridge does not support linking using native_assets yet. native_assets uses @Native annotations as follows:

@Native<Int32 Function(Int32, Int32)>(assetId: "package:pkg/example")
external int add(int a, int b);

The assetId is used to identify the native library, this function should link to. flutter_rust_bridge still uses the old way using DynamicLibrary, where you have to specify the full path to the native library. flutter_rust_bridge can usually resolve the actual path. But not when native_assets is used, because there is no way, to get the actual path to the asset using native_assets. This is deliberately abstracted away.

The hack works as follows:

Future<void> init() async {
// There is currently no way to locate a native asset in dart.
// HACK: Force dart to load the dll into the process
frb_get_rust_content_hash();
await RustLib.init(
externalLibrary: ExternalLibrary.process(iKnowHowToUseIt: true));
}
// HACK: Only used to force dart to load the dll into the process
@Native<Int32 Function()>(assetId: "package:vdd/vdd_lib")
// ignore: non_constant_identifier_names
external int frb_get_rust_content_hash();

When calling frb_get_rust_content_hash, which is linked using @Native, dart will load the complete dll into the process. Now we can tell flutter_rust_bridge to load its functions from the process. frb_get_rust_content_hash is just a function that is always there in flutter_rust_bridge. It returns a constant hash value to check the version compatibility.

But this can eventually be removed, when flutter_rust_bridge transitions to @Native.

@Dampfwalze
Copy link
Contributor Author

I noticed that the driver notifies all clients of changes, except the client that requested that change. I think this should not be the case, because this way, the client is at risk of getting out of sync, without knowing. Also this is something anyone must be aware of and introduces manual labor, since the client application now needs to keep track of its own changes. Even worse, the DriverClient::refresh_state method is implemented in a way where it only reads the changes sent by the change events, since we have it anyways (and we assume it is always correct).

I think it is better when we can assume that the change events always reflect the real state of the driver and you are never at risk of getting out of sync.

In fact, this fits perfectly with the state management patterns in Flutter. You often have some sort of stream that sends state changes to the UI and the UI rebuilds around the current state. When a user interaction (or else) changes the state, the Ui will not change directly, but it will rebuild in response to the stream sending a new state.

let command = match val {
// ignore if this value was sent for the current client (current client doesn't need notification)
Ok((client_id, _)) if client_id == id => continue,

@MolotovCherry
Copy link
Owner

I noticed that the driver notifies all clients of changes, except the client that requested that change. I think this should not be the case, because this way, the client is at risk of getting out of sync, without knowing. ...

I'll add a bug report and address this. Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants