Android Updates #2277
Android Updates #2277
Conversation
…re controls implemented but still incomplete. The only change outside of the Android projects for this commit is defining the Eto.Forms.ILayout interface, and copying definition of Eto.Forms.Layout.Update() to it. This allows StackLayout to implement ILayout so Update() becomes accessible, whereas previously there was no Update() because StackLayout does not actually derive from Layout. As this is only exposing a *new* and previously unused/unavailable method, this change will not affect any other platforms or existing code.
… to resolve build check issues.
|
Added the missing XML comments to fix the build. |
|
Hey @allsorts46, this is awesome! Thanks for taking the time to put this PR together. I'm certainly willing to merge any changes specific to Android directly without much fuss. The Layout/StackLayout changes are a little confusing though and it's hard to tell what scenarios those would be useful for. Ideally they could go in a separate PR along with unit tests or at least an example of their usage. |
|
Hi @cwensley, thanks for checking it out. The additions in Layout.cs and adding ILayout to StackLayout in StackLayout.cs are just so that StackLayout can provide an Update() method like all other layout classes do. Since StackLayout is just a wrapper around a TableLayout, it just passes the Update() call onwards to the inner table. The advantage/use case is just any case where you might have previously wanted to cast an unknown control as a 'Layout' instance. Usually this would have excluded StackLayout, but now you can use ILayout instead, which will include it. I imagine this is quite a rare case, but I'm also sure that it doesn't have any negative consequences whether you use it or not. Overriding of Remove() and RemoveAll() are bug fixes related to the reason above - as StackLayout is only a wrapper of a table the controls actually need to be removed from the inner table, which previously was not done. The changes in GetVerticalAlign() and GetHorizontalAlign() are to reduce the unnecessary nesting of tables. Nested tables in tables are an absolute nightmare to get to behave correctly in Android, and not great for performance reasons on any platform. If the StackLayoutItem is set to be Stretch, the purpose of the nested table (to align left, right or center) no longer exists. Ideally, I would love for StackLayout to stop being a facade and actually be a first-class layout implementation itself, however I have no idea how I could do this only for selected platforms without breaking existing code on other platforms, so haven't touched this. It's something I wanted to ask you about. The ILayout part of the changes at least are necessary for the Android layouts to work properly right now - you can see ILayout.Update() is invoked in AndroidControl.cs, Visible setter. The stretch/no nested optimisation is also necessary for StackLayout to work on Android at the moment, because nested tables in tables do not play well at all. I could remove this code from the PR for now, but it means StackLayout will be unusable on Android until there's a much better and more complex implementation of TableLayout. The fix to Remove(), RemoveAll() is not strictly necessary and could be a separate PR but I thought it was appropriate to include. If there's any particular concerns you had or something I can do to prove that these changes do not affect any existing code, please let me know. |
|
I had a look on the eto test program, the layout behavior of TabletLayout is quite different from other platforms, is there any plans to synchronize the behavior. |
|
TableLayout has definitely been the greatest challenge out of all the Android work I've done, and you're correct there are many cases where it does not behave the same. I've sunk hours and hours into it just to get it working for the particular cases that I use it for. This PR is absolutely not a 'finished' Android platform implementation in any sense, but a starting point. It is my goal to reach parity with the other platforms for all supported controls, but there is a lot of work left to do. I felt it was worth submitting the PR now anyway because right now Android support basically doesn't exist at all, and if I waited until it was 'finished' before contributing, that might well end up being never. If anyone else is able to help work on this that's definitely encouraged and appreciated! But to just answer the question, yes it is the plan to get all controls up to the same standard as other platforms. |
I wanted to compile this to test a project of mine, but ran into issues. I have both Mono and Xamarin installed, but despite that, the compiler still threw Also, if I understand the monoAndroid versioning correctly, this was bumped to Android 9.0, will it still be possible to use it with older Android versions (same question for the eventual use with .net6.0-android)? They may be EOL now, but there are still quite a few devices running on pre-Android 9. |
I've actually already done the retargeting on my local branch, all works fine. I guess it makes sense for me to push onto this PR branch, as the PR has not been accepted yet anyway.
I believe you do have to set the target Android SDK to a pretty recent one (can't recall exactly, may be Android 10 or more), but you can still independently set the minimum Android version to much lower, as long as you then do runtime checks to test which features are available before using them. My own app is building with Android 12 SDK but still runs correctly on an Android 5 device. @cwensley Other than finding the time for it, is there anything specific blocking this PR that I could work on? I responded to your questions above but if you still have concerns let me know, I can make changes as needed. |
|
Hey @allsorts46, sorry for not responding sooner. I would be happy to merge this PR, however I would like I may not be opposed to adding it to the public API possibly in the future, but that will require work to provide test cases and scenarios where it would be used outside of Android. |
# Conflicts: # src/Eto.Android/Eto.Android.csproj
|
Thanks @cwensley for getting back to me. I've just committed to remove the majority of the few changes that were made to StackLayout - keeping this in my private branch instead for now. The optimization relating to flattening the table hierarchy when using the 'Stretch' alignments could be classed as specific Android. Final decision is yours of course, but I'd like to try to make the case for the I don't believe this is "a detail of how the Android platform works currently". My argument is that To be truly consistent, we'd actually add Finally I'd be happy to provide test cases as you mentioned but honestly cannot think of any tests to perform. There should not be any way that adding the interface could change behaviour of existing code, unless a user was doing something very, very specific like using reflection to count the number of interfaces implemented by @Miepee I also retargeted the project and the test project to target 'net6.0-android'. Regarding your concerns about supporting older Android versions, I've just tested running the test project on my Android 13 phone and an Android 7.1 virtual device and it runs on both. According to stats from 2 years ago this would include almost 75% of Android devices, and I think it should run okay on 6.0 as well which likely means >90% these days. One final concern - since the ToolStripDropDownItem changes have been merged, we also need an Android implementation. I have one, but due to this branch being made before those changes were merged, the corresponding Eto controls do not exist. Should I merge 'develop' into this branch, then commit the ToolStripDropDownItem implementation on top? Or leave it out of this branch and add it later? |
|
I just tried compiling the Eto.Test.Android project and run it on my phone. It installs fine, but crashes at runtime. There only is one entry in the assemblies folder. which is set as STORE Here's also a backtrace it generated, but not sure how useful it is: Something that may also be relevant, is this warning when trying to compile: |
Thanks for trying it. I'll try to look into it, just a few questions:
May be some missing properties in the Eto.Test.Android project. It was originally also an old-style monodroid project but I hastily converted it to the new SDK format just to be able to test it. I copied the basics from my actual work project (which was already an SDK-style, and has been tested on many devices), but didn't copy everything. Ran it on my phone and one virtual device and it seemed okay, but didn't get tested beyond that. I'll try a full clean rebuild on a new physical device that I haven't used so far. |
|
1-3: no. Also, you may wanna remove the commented out code from |
Ah, okay. I've only ever been working on this in Visual Studio on Windows. I have a Linux VM but not sure how easy it would be to connect my physical phone to ADB within that. I guess I can try using a |
|
Just to confirm, yes, buliding via |
|
That's a good confirmation to have. Also, is it known/expected that the test application crashes after clicking the about button? EDIT: forgot that this PR is not a complete implementation for Android, so this probably is expected. |
|
I think this is due to 'fast deployment', which I guess VS knows how to handle but is not suitable for publishing. It's enabled only for debug builds so you might find that a release build works for you. Anyway you can disable it by adding this property into the
I haven't committed it because I'd like to find a way to continue using fast deployment in VS (it's much faster) but force it to be disabled with dotnet publish. I can confirm this solves the problem for me on windows though - with this property added I can build with dotnet publish and the app starts up successfully.
Yes, many controls are unimplemented and |
|
Hey @allsorts46 I've just added a new |
Thanks @cwensley for the suggestion, I tried tonight to merge your commit adding UpdateLayout() and use that instead of ILayout, but unfortunately it can't quite replace the ILayout.Update() on StackLayout. The reason is that Control.UpdateLayout() is not virtual, so it can't be overridden in StackLayout. The implementation of Control.UpdateLayout() just calls Control.IHandler.UpdateLayout(), but here we come back to the problem that StackLayout isn't really a layout at all, it's a Panel, and doesn't have its own implementation of IHandler. So to keep the same functionality I have three choices:
Options 2 and 3 involve making more significant changes to non-Android parts of Eto than the ILayout addition, so I assume you wouldn't be in favour of that. Option 1 would work, but feels very wrong putting StackLayout-specific code in the handler for Panel, and it relies accessing internals of StackLayout which really only StackLayout should have knowledge of. Also just to point out, the abstract Layout class itself inherits from Control, so we now have Layout.Update() and Layout.UpdateLayout(), with separate implementations, which is quite confusing. |
|
Hey @ allsorts46, I'm sorry but I just don't see the point of adding this specific hack to StackLayout to chain an update to its inner TableLayout. What happens when a user creates a TableLayout inside of their own Panel? What about a TableLayout inside a Panel inside another TableLayout? etc. All of this update logic should be in the Android platform, so having a specific thing for StackLayout should not ever be necessary. If it is, then other things the developers could do would not work. I don't think you'd need have to have any code checking for a StackLayout specifically, that makes no sense as you'd have to do it for a simple Panel as well and chain to the entire child tree from what it looks like.
I agree, |
…dio started raising an error about 'invalid XML comment'.
|
Short version: Okay, removed ILayout. This branch now contains no changes outside of the Android tree. Users may find issues using TableLayout and StackLayout. Far from a complete Android platform, but hopefully much better than it was and usable by someone. Long version: Let me quote from the Eto.Forms.Layout class, part of the existing Eto.Forms public API: As the remarks on the Update() method say, "All layouts should theoretically work without having to manually update them, but in certain cases this may be necessary to be called.". I'd like to be able to contribute in one go a fully-functional Android platform, but unfortunately it's not feasible. TableLayout, and nested TableLayouts in particular, have been a major source of problems during my work on this. They should 'just work' in all scenarios, but sadly they don't and I've already sunk many, many hours fighting the Android table layout logic and can't afford to spend more right now. Another unfortunate fact is that what I'm able to contribute is limited and directed by the needs of my own project. I need a lot of StackLayouts. Don't know if you have any actual metrics on it, but I'd be willing to bet StackLayout is one of the most used of the layout types. The fact that StackLayout is actually just a wrapper around nested TableLayouts has caused a lot of difficulties on Android, because of the above. It also causes unnecessarily poor performance on WinForms, not sure about other platforms.
This 'specific hack' is exactly what the above quoted code, a current part of the Eto.Forms public API, already does, and if StackLayout was derived from Layout, it would do that too. Adding ILayout was just so that, in the places where it shouldn't be necessary but currently unfortunately is, I can write:
instead of having to manually check for layouts which may or may not be Layouts. Adding the interface would provably have no negative effects on existing users of Eto.Forms, and is not really adding anything new anyway, just exposing access to what is already part of the Eto.Forms API, Layout.Update(), in a common way. I feel that StackLayout, for not just the Android issues but for the sake of better performance overall, could be better implemented as a first-class layout control in its own right, with its own handler and own native per-platform implementations. As this would involve significant changes to the core of Eto.Forms, I plan to instead 'branch' StackLayout as a new separate control in an external library project and maintain this separately in place of the existing StackLayout. I'll happily share this control and any other related layouts if anybody wants them. |
|
Hey @allsorts46, thanks for updating the PR. I'm sorry about the complications regarding StackLayout, but if StackLayout doesn't work then using a Panel with a child TableLayout wouldn't work either.. which is certainly a case that should "just work". Maybe if we can have some test cases set up that show the problem we can get that issue resolved. The problem I see with the I really appreciate the work you've done on this. Hopefully we can get any of these major issues resolved so it can be more useful out of the box. |
Hello! We've talked briefly before about the work I've been doing on the Android platform for Eto. I have finally found the time to review all my changes and split them up into blocks that might make reasonable pull requests.
I still have around a further ~150 files with changes currently in my working directory sitting on top of this branch so there is a lot more to come, but those changes introduce new ideas or make changes that potentially affect the other Eto platforms, so none of that is included for now.
This PR only affects files under Eto.Android, bringing that project to an up-to-date building state with the most recent Eto version. The only exception is, as noted in my commit, adding an ILayout interface for StackLayout to implement so it can pretend to be a 'real' Layout-derived class and offer an Update() method.
Please don't get expectations too high; the Android platform is still very incomplete and there is a lot more work to do. The aim of this is just to bring it back up to date with the current Eto APIs, building and working, with the aim of keeping it maintained and 'current' from now on.
The driving force behind this work was to be able to port our existing Winforms project to run on Android. Our project uses a lot of custom-drawn controls, so most of the biggest changes are around Graphics, Drawable, and the various *Layout classes, as well as things like Toolbar and ContextMenu and the handling of forms and dialogs. Many controls have been implemented on a very basic level just to get them usable, for example NumericStepper is basically just a text box and doesn't feature the 'stepper'. Splitter does support splitting horizontally or vertically, but the divider is fixed at 50%. Some more advanced controls like TabControl and GridView are not implemented at all.
The Android version of the Eto.Test project does build and you can browse and run the various test sections - some work and some don't, due to the issues mentioned above. Working through these test sections one by one and getting them fully working is something I will be spending time on.
The project is unfortunately still an older style Xamarin/MonoDroid project, though I plan to retarget to net6.0-android TFM very soon. I'm currently in the middle of migrating the rest of our project to net6.0.
I realise this is a large PR, but I'm hoping that because it exclusively affects the Android platform, which was previously not even in a working state, it should not be too much work for you to review. These changes are required as a common base for the many other changes I'd like to share - once this is accepted I can then submit separate PRs for each of the other changes and use that as a place for discussion on how to implement missing support for the other platforms so they can be accepted. I'll definitely need help with these as I do not have a Mac build environment or know anything at all about the Mac UI frameworks. I am able to build and test Winforms, WPF and GTK and Android.
Of course please let me know any comments, changes, etc. I'm happy to finally start contributing some of this back to the project, after having made use of it for several years!