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

Use deferred renderer on Android #5337

Merged
merged 5 commits into from
Mar 19, 2021
Merged

Use deferred renderer on Android #5337

merged 5 commits into from
Mar 19, 2021

Conversation

jp2masa
Copy link
Contributor

@jp2masa jp2masa commented Jan 23, 2021

What does the pull request do?

  • Use deferred renderer on Android.
  • Implemented render timer based on Choreographer.
  • Removed apparently useless ActivityTracker.
  • Removed useless EGL classes for Android that I added in the last PR, as there's a common implementation that works perfectly but I hadn't seen it.
  • Handle surface changes on OpenGL.

What is the current behavior?

Only immediate renderer works on Android.

What is the updated/expected behavior with this PR?

It is now possible to use deferred renderer on Android, and it's set as default. Performance is much better now. Unfortunately, OnPause/OnResume isn't working, it shows a black screen, and I don't know if it was like that before, I'll take a look at that soon.

Checklist

@jp2masa
Copy link
Contributor Author

jp2masa commented Jan 24, 2021

The OnPause/OnResume problem is because the OpenGL surface has to be marked as corrupted when surface changes, the fix is simple so I will add it to this PR (also need to revert the deleted EGL classes as they are now needed).

@jp2masa jp2masa force-pushed the android branch 2 times, most recently from eabb44f to 26681bb Compare January 24, 2021 06:30
@grokys
Copy link
Member

grokys commented Jan 25, 2021

@jp2masa slightly off topic, but I wonder if #5070 improves performance in Android noticeably? If you're working on Android at the moment I'd be interested to know (or if it causes problems).

@jp2masa
Copy link
Contributor Author

jp2masa commented Jan 26, 2021

It doesn't seem to make much difference, I believe the problem isn't related to styles (switching styles isn't much slower than Windows as well for example). I actually profiled it a few days ago and I think the problem is that it spends more than 1 min for syscalls when loading the app, probably related to dynamic loading and the native part in general.

@Gillibald
Copy link
Contributor

Is this with a release build without debugger etc?

@jp2masa
Copy link
Contributor Author

jp2masa commented Jan 26, 2021

For profiling it's required to have debugging enabled, so it's slower, but I already tried disabling it and enabling AOT and although it's faster, it still takes more than 30s to load IIRC.

@Gillibald
Copy link
Contributor

I wonder how well Xamarin.Forms and Uno perform in that area. We definitely need some benchmarks for startup etc.

View.Content = _content;
SetContentView(View);
TakeKeyEvents(true);
base.OnCreate(savedInstanceState);
}

protected override void OnResume()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will require an Avalonia-based Activity which prevents AvaloniaView from being used for embedding scenarios

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to replace this with manual Tick subscription count. However, I noticed it isn't enough to stop the timer as the animation clock (Avalonia.Animation.RenderLoopClock) never stops, so the render loop doesn't stop, i.e. _items.Count never reaches 0 here:

if (_items.Count == 0)
{
Timer.Tick -= TimerTick;
}

Is this a bug? Or am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a known issue. For now I'd suggest to manually cast IRenderTimer to a concrete type and manually notify it about the subscription count

@ili
Copy link
Contributor

ili commented Mar 19, 2021

@jp2masa @kekekeks what's about this PR?
It shows significant prefomance application response time improvement.

@jmacato jmacato enabled auto-merge March 19, 2021 13:37
@jmacato jmacato merged commit 3b0f4af into AvaloniaUI:master Mar 19, 2021
@jp2masa jp2masa deleted the android branch March 19, 2021 17:12
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.

7 participants