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.

Sign up for GitHub

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

perf(x11): use a Timer instead of a DispatcherTimer for render callbacks #19054

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+92 −47
Open

perf(x11): use a Timer instead of a DispatcherTimer for render callbacks #19054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
32885bd
chore(x11): use a Timer instead of a DispatcherTimer for render callb…
ramezgerges Dec 10, 2024
798ffb8
chore: rework fix
ramezgerges Dec 10, 2024
17a70a9
chore: formatting
ramezgerges Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/Uno.UI.Runtime.Skia.X11/X11ApplicationHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace Uno.WinUI.Runtime.Skia.X11;
public partial class X11ApplicationHost : SkiaHost, ISkiaApplicationHost, IDisposable
{
[ThreadStatic] private static bool _isDispatcherThread;
[ThreadStatic] private static int _renderFrameRate;
private readonly EventLoop _eventLoop;

private readonly Func<Application> _appBuilder;
Expand Down Expand Up @@ -75,19 +74,24 @@ public X11ApplicationHost(Func<Application> appBuilder, int renderFrameRate = 60
{
_appBuilder = appBuilder;

if (RenderFrameRate != default && renderFrameRate != RenderFrameRate)
{
throw new InvalidOperationException($"X11's render frame rate should only be set once.");
}
RenderFrameRate = renderFrameRate;

_eventLoop = new EventLoop();
_eventLoop.Schedule(() => { Thread.CurrentThread.Name = "Uno Event Loop"; }, UI.Dispatching.NativeDispatcherPriority.Normal);

_eventLoop.Schedule(() =>
{
_isDispatcherThread = true;
_renderFrameRate = renderFrameRate;
}, UI.Dispatching.NativeDispatcherPriority.Normal);
CoreDispatcher.DispatchOverride = _eventLoop.Schedule;
CoreDispatcher.HasThreadAccessOverride = () => _isDispatcherThread;
}

internal static int RenderFrameRate => _renderFrameRate;
internal static int RenderFrameRate { get; private set; }

[LibraryImport("libc", StringMarshallingCustomType = typeof(AnsiStringMarshaller))]
private static partial void setlocale(int type, string s);
Expand Down
121 changes: 82 additions & 39 deletions src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,28 @@ internal partial class X11XamlRootHost : IXamlRootHost
private readonly ApplicationView _applicationView;
private readonly X11WindowWrapper _wrapper;
private readonly Window _window;
private readonly CompositeDisposable _disposables = new();
private readonly XamlRoot _xamlRoot;

private int _synchronizedShutDownTopWindowIdleCounter;

private X11Window? _x11Window;
private X11Window? _x11TopWindow;
private IX11Renderer? _renderer;

private bool _renderDirty = true;
private readonly DispatcherTimer _renderTimer;
private static readonly Stopwatch _stopwatch = Stopwatch.StartNew();

private readonly DispatcherTimer _renderTimer = new DispatcherTimer();
private long _lastRenderTime;
private int _renderScheduled;

private readonly DispatcherTimer _configureTimer = new DispatcherTimer();
private long _lastConfigureTime;
private int _configureScheduled;

public X11Window RootX11Window => _x11Window!.Value;
public X11Window TopX11Window => _x11TopWindow!.Value;

public X11XamlRootHost(X11WindowWrapper wrapper, Window winUIWindow, XamlRoot xamlRoot, Action configureCallback, Action closingCallback, Action<bool> focusCallback, Action<bool> visibilityCallback)
{
_xamlRoot = xamlRoot;
_wrapper = wrapper;
_window = winUIWindow;

Expand All @@ -99,31 +103,28 @@ public X11XamlRootHost(X11WindowWrapper wrapper, Window winUIWindow, XamlRoot xa
_visibilityCallback = visibilityCallback;
_configureCallback = configureCallback;

_renderTimer = new DispatcherTimer();
_renderTimer.Interval = TimeSpan.FromSeconds(1.0 / X11ApplicationHost.RenderFrameRate); // we're on the UI thread
_renderTimer.Tick += (sender, o) =>
_closed = new TaskCompletionSource();
Closed = _closed.Task;

_renderTimer.Tick += (_, _) =>
{
if (Interlocked.Exchange(ref _needsConfigureCallback, 0) == 1)
{
_configureCallback();
}
_renderTimer.Stop();
_renderScheduled = 0;
_renderer?.Render();
};

if (_renderDirty)
{
_renderDirty = false;
_renderer?.Render();
}
_configureTimer.Tick += (_, _) =>
{
_configureTimer.Stop();
_configureScheduled = 0;
_configureCallback();
};
_renderTimer.Start();

_applicationView = ApplicationView.GetForWindowId(winUIWindow.AppWindow.Id);
_applicationView.PropertyChanged += OnApplicationViewPropertyChanged;
CoreApplication.GetCurrentView().TitleBar.ExtendViewIntoTitleBarChanged += UpdateWindowPropertiesFromCoreApplication;
winUIWindow.AppWindow.TitleBar.ExtendsContentIntoTitleBarChanged += ExtendContentIntoTitleBar;

_closed = new TaskCompletionSource();
Closed = _closed.Task;

Initialize();

// Note: the timing of XamlRootMap.Register is very fragile. It needs to be early enough
Expand All @@ -132,6 +133,15 @@ public X11XamlRootHost(X11WindowWrapper wrapper, Window winUIWindow, XamlRoot xa
_windowToHost[winUIWindow] = this;
X11Manager.XamlRootMap.Register(xamlRoot, this);

UpdateWindowPropertiesFromPackage();
OnApplicationViewPropertyChanged(this, new PropertyChangedEventArgs(null));

// only start listening to events after we're done setting everything up
InitializeX11EventsThread();

var windowBackgroundDisposable = _window.RegisterBackgroundChangedEvent((_, _) => UpdateRendererBackground());
UpdateRendererBackground();

Closed.ContinueWith(_ =>
{
using (X11Helper.XLock(RootX11Window.Display))
Expand All @@ -141,16 +151,9 @@ public X11XamlRootHost(X11WindowWrapper wrapper, Window winUIWindow, XamlRoot xa
_applicationView.PropertyChanged -= OnApplicationViewPropertyChanged;
CoreApplication.GetCurrentView().TitleBar.ExtendViewIntoTitleBarChanged -= UpdateWindowPropertiesFromCoreApplication;
winUIWindow.AppWindow.TitleBar.ExtendsContentIntoTitleBarChanged -= ExtendContentIntoTitleBar;
windowBackgroundDisposable.Dispose();
}
});

UpdateWindowPropertiesFromPackage();
OnApplicationViewPropertyChanged(this, new PropertyChangedEventArgs(null));

// only start listening to events after we're done setting everything up
InitializeX11EventsThread();

RegisterForBackgroundColor();
}

public static X11XamlRootHost? GetHostFromWindow(Window window)
Expand Down Expand Up @@ -444,7 +447,7 @@ private unsafe static X11Window CreateGLXWindow(IntPtr display, int screen, Size
}

IntPtr context = GlxInterface.glXCreateNewContext(display, bestFbc, GlxConsts.GLX_RGBA_TYPE, IntPtr.Zero, /* True */ 1);
var _1 = XLib.XSync(display, false);
_ = XLib.XSync(display, false);

XSetWindowAttributes attribs = default;
attribs.border_pixel = XLib.XBlackPixel(display, screen);
Expand Down Expand Up @@ -534,10 +537,57 @@ private bool IsOpenGLSupported(IntPtr display)
}
}

void IXamlRootHost.InvalidateRender() => _renderDirty = true;

UIElement? IXamlRootHost.RootElement => _window.RootElement;

// running XConfigureEvent and rendering callbacks on a timer is necessary to avoid freezing in certain situations
// where we get spammed with these events. Most notably, when resizing a window by dragging an edge, we'll get spammed
// with XConfigureEvents.
void IXamlRootHost.InvalidateRender()
{
if (Interlocked.Exchange(ref _renderScheduled, 1) == 0)
{
// Don't use ticks, which seem to mess things up for some reason
var now = _stopwatch.ElapsedMilliseconds;
var delta = now - Interlocked.Exchange(ref _lastRenderTime, now);
if (delta > TimeSpan.FromSeconds(1.0 / X11ApplicationHost.RenderFrameRate).TotalMilliseconds)
{
QueueAction(this, () =>
{
_renderScheduled = 0;
_renderer?.Render();
});
}
else
{
_renderTimer.Interval = TimeSpan.FromTicks(delta);
_renderTimer.Start();
Comment on lines +562 to +563
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, we're setting up the interval to be the time since the last update? Aren't we trying to render immediately, or at least at the FPS interval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're rendering immediately if the last frame was more than 1/60 secs ago. If the last frame is within the last 1/60 secs, we wait the remainder of this 1/60 secs "time slice" before rendering again, basically capping the frame rate at 60.

}
}
}

private void RaiseConfigureCallback()
{
if (Interlocked.Exchange(ref _configureScheduled, 1) == 0)
{
// Don't use ticks, which seem to mess things up for some reason
var now = _stopwatch.ElapsedMilliseconds;
var delta = now - Interlocked.Exchange(ref _lastConfigureTime, now);
if (delta > TimeSpan.FromSeconds(1.0 / X11ApplicationHost.RenderFrameRate).TotalMilliseconds)
{
QueueAction(this, () =>
{
_configureScheduled = 0;
_configureCallback();
});
}
else
{
_configureTimer.Interval = TimeSpan.FromTicks(delta);
_configureTimer.Start();
}
}
}

public unsafe void AttachSubWindow(IntPtr window)
{
using var lockDisposable = X11Helper.XLock(RootX11Window.Display);
Expand Down Expand Up @@ -593,13 +643,6 @@ private void SynchronizedShutDown(X11Window x11Window)
}
}

private void RegisterForBackgroundColor()
{
UpdateRendererBackground();

_disposables.Add(_window.RegisterBackgroundChangedEvent((s, e) => UpdateRendererBackground()));
}

private void UpdateRendererBackground()
{
if (_window.Background is Microsoft.UI.Xaml.Media.SolidColorBrush brush)
Expand Down
8 changes: 3 additions & 5 deletions src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.x11events.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ internal partial class X11XamlRootHost
private readonly Action<bool> _visibilityCallback;
private readonly Action _configureCallback;

private int _needsConfigureCallback;

private X11PointerInputSource? _pointerSource;
private X11KeyboardInputSource? _keyboardSource;
private X11DragDropExtension? _dragDrop;
Expand Down Expand Up @@ -87,7 +85,7 @@ private unsafe void Run(X11Window x11Window)
{
var ret = X11Helper.poll(fds, 1, 1000); // timeout every second to see if the window is closed

if (_closed.Task.IsCompleted)
if (Closed.IsCompleted)
{
SynchronizedShutDown(x11Window);
return;
Expand Down Expand Up @@ -197,7 +195,7 @@ static IEnumerable<XEvent> GetEvents(IntPtr display)
}
break;
case XEventName.ConfigureNotify:
Interlocked.Exchange(ref _needsConfigureCallback, 1);
RaiseConfigureCallback();
break;
case XEventName.FocusIn:
QueueAction(this, () => _focusCallback(true));
Expand All @@ -209,7 +207,7 @@ static IEnumerable<XEvent> GetEvents(IntPtr display)
QueueAction(this, () => _visibilityCallback(@event.VisibilityEvent.state != /* VisibilityFullyObscured */ 2));
break;
case XEventName.Expose:
QueueAction(this, () => ((IXamlRootHost)this).InvalidateRender());
((IXamlRootHost)this).InvalidateRender();
break;
case XEventName.MotionNotify:
_pointerSource?.ProcessMotionNotifyEvent(@event.MotionEvent);
Expand Down
Loading