rmawatson / flutter_isolate

Launch an isolate that can use flutter plugins.
MIT License
262 stars 80 forks source link

Investigate whether this plugin is still required #128

Open nmfisher opened 1 year ago

nmfisher commented 1 year ago

Per https://github.com/flutter/flutter/issues/13937#issuecomment-1258766459, Flutter now seems to have support for running plugin code in background isolates, which would render this plugin redundant.

This is apparently merged into Flutter master (not yet stable as of 12 November 2022) - I don't have much free time so would appreciate someone investigating further to see if this is correct.

rlueders commented 1 year ago

I have been using your plugin to call Flutter UI functions from an isolate. I will check the sample in the link you provided to see if it offers a solution.

rlueders commented 1 year ago

It appears that changes from 13937 do not allow for running dart.ui code from a standard Isolate. Consequently, this plugin does indeed provide additional capability outside of standard flutter isolates and is not completely redundant.

After registering a background isolate with a RootIsolateToken as described in 13937 sample code, the isolate will still throw a "UI Actions are only available on root isolate" exception any time that dart.ui code is called from a standard isolate. No special handling is required when calling dart.ui code from a FlutterIsolate (this plugin).

I tried enabling the new impeller render on IOS to see if there was any difference in behavior, but the same exception is thrown for any calls to dart.ui code from a standard Isolate. Surprisingly, the calls to dart.ui.picture.toImage() (with impeller enabled) from FlutterIsolate (this plugin) result in "Exception: operation failed" exception. As such, this plugin is currently the only way to run dart.ui code from an isolate, but it looks like that might be breaking with the new impeller renderer unless it is fixed before the stable release of impeller.

nmfisher commented 1 year ago

Thank you @rlueders, this is excellent. Let me review further and see what's needed for Impeller.

rlueders commented 1 year ago

Thank you @rlueders, this is excellent. Let me review further and see what's needed for Impeller.

Did you ever get a chance to look into this? looks like the same behavior on the stable release of Flutter 3.7.

nmfisher commented 1 year ago

@rlueders I only recently upgraded to Flutter 3.7 and haven't had a chance to test out plugins/platform channels yet. However I did recently review the release documentation again, and all signs point to this plugin being mostly redundant.

The primary driver for this plugin was allowing invocation of general plugin code on a non-main isolate. From reading the documents (and this is what I need to test), this is now available in Flutter 3.7 (and possibly avoiding all of the issues with plugin reinitialization/Android Activity/etc that using flutter_isolate causes).

The fact that you could also call dart.ui method on a background isolate was probably incidental to the main purpose. That's not to downplay the use-case, just pointing out that it wasn't the main reason for the existence of this plugin.

The question is whether or not this plugin should continue to support calling dart.ui methods on a background isolate. As you point out, this would require some additional work to maintain compatibility with Impeller. My gut feeling is that there must be a good reason why the Flutter engine does not allow dart.ui methods on background isolate, and therefore flutter_isolate is not appropriate for this. But that's also something that requires further investigation.

This was mostly an unedited/unfiltered brain-dump so forgive any poor phrasing.

EDIT: just leaving a link to https://github.com/flutter/flutter/issues/10647 that @rlueders has also participated in. Looks like the Flutter team are aware of the restriction on dart.ui methods on background isolates but it is not a priority for them to remove these and have asked for a community submission to address it.

rlueders commented 1 year ago

Here are a few other related issues requesting dart.ui from isolates:

The reason the Flutter engine does not allow dart.ui from background isolate is discussed in 13343 by @chinmaygarde where he talks about dart.ui having some unsafe calls related to frame scheduling. The majority of the function in dart.ui are safe to call from a background isolates. His proposal to "Design a dart:ui lite variant of bindings that are safely accessible by secondary isolates." makes sense, but since this has been open for 5 years, I doubt it will be prioritized any time soon.

jamesncl commented 1 year ago

Also standard isolates still do not allow access the root bundle, so you can't load asset files. This bit me when attempting to remove flutter_isolate from my project, because I was trying to initialise a timezone package using a timezone database file in my assets. So for me, this package is still useful because it somehow enables root bundle access from it's isolates. This limitation of standard isolates is mentioned in a comment in the official isolate example code:

//  ... spawned isolates do not have access to the root bundle

However I do think the docs need updating. https://pub.dev/packages/isolate_handler currently says the following which is no longer true for 3.7 +

Effortless isolates abstraction layer with support for inter-isolate communication. These isolates (unlike the standard ones in Flutter) can call platform plugins.

nmfisher commented 1 year ago

That's a good point - annoyingly, I think flutter forces all assets loading to occur on the main thread (flutter_isolate circumvents this by spawning a new FlutterEngine which I assume itself starts a new thread(s)).

You're also right that a documentation update is probably needed.

greensopinion commented 1 year ago

It appears that changes from 13937 do not allow for running dart.ui code from a standard Isolate. Consequently, this plugin does indeed provide additional capability outside of standard flutter isolates and is not completely redundant.

I'm interested in this capability to improve the frame rate of vector_map_tiles which renders map tiles from vector data. Currently the plugin suffers from jank.

I tried enabling the new impeller render on IOS to see if there was any difference in behavior, but the same exception is thrown for any calls to dart.ui code from a standard Isolate. Surprisingly, the calls to dart.ui.picture.toImage() (with impeller enabled) from FlutterIsolate (this plugin) result in "Exception: operation failed" exception. As such, this plugin is currently the only way to run dart.ui code from an isolate, but it looks like that might be breaking with the new impeller renderer unless it is fixed before the stable release of impeller.

Has anyone raised this issue with the Flutter team? They may not be aware of this use-case.

rlueders commented 1 year ago

I'm interested in this capability to improve the frame rate of vector_map_tiles which renders map tiles from vector data. Currently the plugin suffers from jank.

Has anyone raised this issue with the Flutter team? They may not be aware of this use-case.

@greensopinion I don't know exactly what you are trying to do with vector_map_tiles, but I would say the flutter_isolates has worked great for my app enabling significant performance improvements. That is, as long as I don't enable impeller. I have not created an issue in the Flutter repository for this particular problem. All the issues I have have ever opened with Flutter never get fixed, rather they sit in P4 priority for years. Also, There are however five issues, all requesting the use of dart.UI from regular isolates, that were opened by other users. Resolving any one of those issues would eliminate this issue (128) and eliminate the need for this flutter_isolate plugin completely. Unfortunately, the Flutter team has shown no interest in fixing these and they were all opened multiple years ago. I gather that the only way to influence the Flutter team to prioritize these, is to thumbs up the first comment on the issue.

greensopinion commented 1 year ago

Thanks for the insights, I appreciate it.

From what I can gather, the main difference between those issues and this case is that here we have an existing capability that is no longer working, whereas the others are for adding a capability that doesn't exist today.

rlueders commented 1 year ago

FYI this issue (calling dart.ui functions from a Flutter Isolate) was resolved in one of the recent Flutter releases.

nmfisher commented 1 year ago

Thanks @rlueders - I guess that's one more argument in favour of deprecating this plugin.

greensopinion commented 1 year ago

this issue (calling dart.ui functions from a Flutter Isolate) was resolved in one of the recent Flutter releases.

Any idea where we can read about this? I haven't found it.

rlueders commented 1 year ago

No, it is an argument in favor of keeping the plugin. I can see how my previous reply was confusing. Let me rephrase. Calling dart.ui from a standard platform Isolate has never worked an still doesn't work. However, calling dart.ui from a Flutter Isolate (this plugin) now works when using the latest version of Flutter/Dart. It was actually fixed in this dart change.

nmfisher commented 1 year ago

Understood, thanks for clarifying.