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

Angle WPF #1521

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Angle WPF #1521

wants to merge 9 commits into from

Conversation

Mikolaytis
Copy link
Contributor

@Mikolaytis Mikolaytis commented Oct 6, 2020

Description of Change

I've added working ANGLE approach to the SkiaSharp.Views.WPF that enables GPU rendering with minimum performance overhead on transferring draw calls between OpenGL and DirectX.

This version of code is a minimum working example that's have minimum changes to the SKElement code. I've decided to do so to make PR as simple as I can, so it will be easy to understand the code and make some changes to it, if we will need some.

[WARNING]
ANGLE libs are attached by Avalonia nuget package, but it has an issue, somehow it do not copy angle libs to the connected projects output, like WPF sample. So to make ANGLE working you need to copy libs manually. We need to ask and make an update to Avalonia nuget package or make SkiaSharp ANGLE libs package that supports dlls copying to connected projects.

If this PR will be merged I will continue the work and add support of rendering in specific thread, and other tweaks.

Bugs Added

  • Window hang on any DEVICE_LOST. Device lost can be triggered by PC sleep/hybernate or by directX command line call.

API Changes

Added:

  • GLMode SKElement.Mode { get; }
  • GRContext SKElement.GRContext { get; }
  • void SKElement.StopRenderAndDisposeAllUnmanagedResources()

Behavioral Changes

  • My code do not support multiple instances of SKElement. I don't know if we need to make this feature.
  • I've removed openGL/WindowsFormsHost approach because it's bad and don't needed anymore (from my perspective)
  • My code do not support backend switch. Do I need to implement it?

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation
  • Angle Libs nuget issue is resolved

@ghost
Copy link

ghost commented Oct 6, 2020

CLA assistant check
All CLA requirements met.

@mattleibow
Copy link
Contributor

Maybe it will be easier to just start with a new view that is just for GPU?

Then there is no need for checks and fallback. If you are adding a GPU view to a window, then you probably would want it to fail if the GPU is not available? Also, what are the conditions that it would fail? Isn't WPF built on top of DX ?

@mattleibow
Copy link
Contributor

For a backend rendering thread, is it a lot of work? I think that is a feature that is worth adding to the base implementation. UWP is really a backend thread loop, and then I shuttle calls to the main thread.

One downside of a background thread is that you can't access the UI, such as bounds, in that thread... But I have a property in UWP to toggle this feature on and off.

@Mikolaytis
Copy link
Contributor Author

Mikolaytis commented Oct 6, 2020

Maybe it will be easier to just start with a new view that is just for GPU?

Then there is no need for checks and fallback. If you are adding a GPU view to a window, then you probably would want it to fail if the GPU is not available? Also, what are the conditions that it would fail? Isn't WPF built on top of DX ?

The reasons ANGLE backend will fail to initialize:

  1. No GPU device (server, or virtual machine, or a crazy old PC)
  2. ANGLE libs are missing, or no file access.
  3. Any driver/display issue.

I think that fallback to CPU rendering is an important feature that is need to exist in a default implementation. There is a lot of cases when user do not have a GPU device and do want to run an app to do the job done. For the instance - app can be launched in the server environment or in the virtual machine.

About async GPU rendering - I have a code already, but it's specialized for out app. All I need is to think what is needed, what is not and mindfully transfer code chunk by chunk with some refactoring afterwards. It will take me a day I think. I will do it on the weekend. Do you want it as a single PR? Or let's just do this simple PR first and then add an async things via separate PR.

@mattleibow
Copy link
Contributor

For the async bits, how much of a change to add it? I was thinking that if it is going to be a bit of re-engineering then maybe not duplicate work?

@mattleibow
Copy link
Contributor

mattleibow commented Oct 6, 2020

Something just occurred to me. The software fallback seemed a bit weird and I didn't recall even needing that for UWP. I didn't actually do all the testing, but I now recall that DX has a software rasterizer: WARP.

Isn't that the fallback? Or is it not enough? https://docs.microsoft.com/en-us/windows/win32/direct3darticles/directx-warp#enabling-rendering-when-direct3d-hardware-is-not-available
It seems it works on machines that are virtualized and if they don't have hardware or drivers. I think all the bits are built into Windows since 7. I may be misunderstanding things, but is it not better to use that, and less work?

When I create the dives, I try a few layers, DX11, DX9, WARP: https://github.com/mono/SkiaSharp/blob/master/source/SkiaSharp.Views/SkiaSharp.Views.UWP/GlesInterop/GlesContext.cs#L195

@AnarchyMob
Copy link

AnarchyMob commented Oct 6, 2020

Why do you need ANGLE? The 84th milestone already has support for the D3D backend, you need to add bindings.
https://github.com/mono/skia/blob/d8d90ca991feb9a45b1176e24a305f2397a5c575/include/gpu/GrContext.h#L86

@mattleibow
Copy link
Contributor

Do we know how far that support is? It is very recent. I'll check with them.

@Mikolaytis
Copy link
Contributor Author

@AnarchyMob WoW. Awesome news! Can't wait to use DirectX backend in our app. Is there any issue exists about D3D backend? If no - let's create one?

@mattleibow About async bits - OK. I'll add them to this PR on a weekend.

About fallback - I've asked my QA team to test the fallback code in our app in case I've missed something. That's why my response have a large delay.
It works like I described earlier - ANGLE backend cannot initialize without GPU device. You are right, D3D supports CPU fallback inside. The issue is looks like in my ANGLE/GLES/D3D/SharpDX/OpenTK Interop initialization code. Maybe it requires GPU and if we tweak something - this code will work with ANGLE backend in any situation. But for now, my implementation needs a fallback, sadly.

@fmbv
Copy link

fmbv commented Oct 27, 2020

Hi, thanks for your work on this issue!
I'm using SKElement to render charts in WPF and I'm really interested in switching to GPU rendering.

My code do not support multiple instances of SKElement. I don't know if we need to make this feature.

I think it would be immensely helpful to allow multiple instances. In my case, I define a DataTemplate like

<DataTemplate DataType="{x:Type vm:MyViewModel}">
    <!-- View contains 1 or more SKElement.   -->
    <vw:MyView/> 
</DataTemplate>

and frequently create multiple instances using an ItemsControl.

@Mikolaytis
Copy link
Contributor Author

Mikolaytis commented Nov 13, 2020

Update on a PR status:

  1. [Done, will commit later] CONTEXT_LOSS / DEVICE_LOST handling - tried a lot of things this month, have some improvements, but it's not a 100% fix :(.
  2. [Done, will commit later] Do not use the OpenTK.
  3. [ToDo] SKElement multiple instances.
  4. [ToDo] Async execution

Base automatically changed from master to main February 5, 2021 08:30
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.

4 participants