Skip to content

perf: split the rendering cycle on X11 to paint and composite on 2 different threads #20177

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

Merged

Conversation

ramezgerges
Copy link
Contributor

GitHub Issue (If applicable): closes https://github.com/unoplatform/uno-private/issues/1206

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@github-actions github-actions bot added platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform area/skia ✏️ Categorizes an issue or PR as relevant to Skia platform/x11 🐧 Categorizes an issue or PR as relevant to X11 labels May 2, 2025
@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/index.html

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164177 has failed on Uno.UI - CI.

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164378 has failed on Uno.UI - docs.

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164379 has failed on Uno.UI - CI.

@ramezgerges ramezgerges force-pushed the skia_render_on_separate_thread branch from b8a980d to 6881b09 Compare May 5, 2025 19:08
@unodevops
Copy link
Contributor

⚠️⚠️ The build 164392 has failed on Uno.UI - docs.

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-native-net9/index.html

@ramezgerges ramezgerges marked this pull request as ready for review May 5, 2025 19:20
@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-native-net9/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/docs/index.html

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164396 has failed on Uno.UI - CI.

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164393 has failed on Uno.UI - CI.

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App 2 stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-skia-net9/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/docs/index.html

@unodevops
Copy link
Contributor

⚠️⚠️ The build 164740 has failed on Uno.UI - CI.

@jeromelaban jeromelaban requested a review from Copilot May 12, 2025 13:58
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves rendering performance on X11 by splitting the rendering cycle into two threads and updating several related APIs and thread naming conventions. Key changes include a simplified scheduling API in the event loop, explicit default settings for OpenGL selection, and updated rendering implementations for both software and OpenGL paths.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Uno.UWP/Helpers/EventLoop.cs Removed the explicit priority parameter from the Schedule method.
src/Uno.UI/FeatureConfiguration.cs Added an explicit default (false) for UseOpenGLOnX11.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.x11events.cs Renamed the thread counter variable to _seqNumber and updated thread naming.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.cs Instantiated the rendering event loop with updated thread naming.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.Rendering.cs Revised the rendering scheduling logic using an SKPictureRecorder.
src/Uno.UI.Runtime.Skia.X11/X11SoftwareRenderer.cs & X11OpenGLRenderer.cs Updated renderer implementations to inherit from a common base and adjust resource handling.
Other files Minor adjustments to scheduling calls and recorder dimensions in platform-specific views.
Comments suppressed due to low confidence (4)

src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.x11events.cs:18

  • [nitpick] Renaming the thread counter variable to _seqNumber improves clarity; ensure any associated documentation or usage comments are updated accordingly.
private static int _seqNumber;

src/Uno.UI/FeatureConfiguration.cs:864

  • Explicitly setting the default value of UseOpenGLOnX11 to false may change existing behavior if code relied on a null default. Confirm that this default value aligns with the intended rendering fallback logic.
public static bool? UseOpenGLOnX11 { get; set; } = false;

src/Uno.UI.Runtime.Skia.AppleUIKit/UI/Xaml/UnoSKMetalView.cs:123

  • [nitpick] Increasing the canvas recording bounds substantially may impact memory usage and performance. Please verify that these extended bounds are required and consider potential performance implications.
var canvas = recorder.BeginRecording(new SKRect(-999999, -999999, 999999, 999999));

src/Uno.UI.Runtime.Skia.X11/X11OpenGLRenderer.cs:51

  • Replacing a logged error with an exception in MakeCurrent changes the error handling behavior. Ensure that throwing a NotSupportedException here is acceptable within the application's error handling strategy.
throw new NotSupportedException($"glXMakeCurrent failed for Window {_x11Window.Window.GetHashCode().ToString("X", CultureInfo.InvariantCulture)}");

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/docs/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-skia-net9/index.html

@jeromelaban jeromelaban requested a review from Copilot May 22, 2025 15:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the X11 rendering pipeline to split the rendering cycle into separate painting and compositing threads and refines several scheduling and rendering strategies across multiple platforms. Key changes include:

  • Removal of dispatcher priority parameters from scheduling methods.
  • Introduction of a rendering event loop and refactoring of renderer abstractions.
  • Updates to thread naming and default configuration settings (e.g. UseOpenGLOnX11 defaults).

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Uno.UWP/Helpers/EventLoop.cs Removed the dispatcher priority parameter from Schedule.
src/Uno.UI/FeatureConfiguration.cs Set the default UseOpenGLOnX11 value to false.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.x11events.cs Changed thread counter logic and updated thread naming using _id.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.cs Updated rendering event loop initialization and disposal patterns.
src/Uno.UI.Runtime.Skia.X11/X11XamlRootHost.Rendering.cs Added a new implementation for scheduling render operations on a background thread.
src/Uno.UI.Runtime.Skia.X11/X11SoftwareRenderer.cs Refactored renderer implementation to derive from the new X11Renderer base class.
src/Uno.UI.Runtime.Skia.X11/X11Renderer.cs Created as an abstract base for X11 renderers consolidating common logic.
src/Uno.UI.Runtime.Skia.X11/X11OpenGLRenderer.cs Modified OpenGL renderer to use exceptions for context failures and refactored resource management.
src/Uno.UI.Runtime.Skia.X11/X11ApplicationHost.cs Removed the dispatcher priority parameter in scheduling.
src/Uno.UI.Runtime.Skia.X11/X11AirspaceRenderHelper.cs Simplified path-based clipping logic by removing a redundant null check.
src/Uno.UI.Runtime.Skia.X11/IX11Renderer.cs Removed the legacy renderer interface.
src/Uno.UI.Runtime.Skia.Linux.FrameBuffer/FramebufferHost.cs Adjusted scheduling calls by removing the priority parameter.
src/Uno.UI.Runtime.Skia.AppleUIKit/UI/Xaml/UnoSKMetalView.cs Increased composite recording rectangle limits.
src/Uno.UI.Runtime.Skia.Android/Microsoft/UI/Xaml/UnoSKCanvasView.cs Increased composite recording rectangle limits.

@ramezgerges ramezgerges force-pushed the skia_render_on_separate_thread branch from 53248ac to 6fb9759 Compare May 22, 2025 18:38
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/docs/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-skia-net9/index.html

@ramezgerges ramezgerges force-pushed the skia_render_on_separate_thread branch from 6fb9759 to 5812ea1 Compare May 23, 2025 01:11
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-20177/docs/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-20177/wasm-skia-net9/index.html

@jeromelaban jeromelaban merged commit 305386e into unoplatform:master May 23, 2025
91 checks passed
@ramezgerges ramezgerges deleted the skia_render_on_separate_thread branch May 23, 2025 15:46
DevTKSS pushed a commit to DevTKSS/uno that referenced this pull request May 31, 2025
…separate_thread

perf: split the rendering cycle on X11 to paint and composite on 2 different threads
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/skia ✏️ Categorizes an issue or PR as relevant to Skia platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/x11 🐧 Categorizes an issue or PR as relevant to X11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants