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

Graphics context management #9639

Merged
merged 15 commits into from
Dec 10, 2022
Merged

Graphics context management #9639

merged 15 commits into from
Dec 10, 2022

Conversation

kekekeks
Copy link
Member

@kekekeks kekekeks commented Dec 6, 2022

  • graphics contexts are now owned by the Compositor (it's still possible to use a shared context)
  • WinUI composition now exposes IDirect3D11TexturePlatformSurface instead of EGL one and re-creates a composition device when needed
  • OpenGL control now works for iOS and Linux framebuffer
  • various refactorings
  • fixed DPI reported by WriteableBitmap.Lock()

To survive a driver reset, our renderers need to be able to recreate the entire graphics infrastructure from scratch.
We also have a problem with graphics subsystem initialization blocking the rest of our initialization code on the UI thread, which is hurtful for embedded scenarious where initial GPU initialization might take some time due to drviver init, connected display scans, etc. So we'd rather initialize the GPU stuff on the render thread.
To do that, we need the renderer to be in control of the graphics resources rather than holding those in some static fields in SkiaPlatform.

  // The general interface for providing optional features. We can't really rely on type casts since
  // some features might be aggregated from other classes and C# doesn't support multi-inheritance nor
  // something like COM interface aggregation
  interface IFeatureProvider
  {
    object GetFeature(Type t);
  }

  interface IPlatformGraphics : IFeatureProvider
  {
    IPlatformGraphicsContext GetSharedContext();
    IPlatformGraphicsContext CreateContext();
  }
  
  // Can be IGlContext, ISkiaGpu or our new vulkan thingy
  interface IPlatformGraphicsContext : IDisposable, IFeatureProvider
  {
    bool IsLost { get; }
    // This is needed to make sure that we can dispose layers
    // outside of IRenderTarget-bound drawing session
    IDisposable EnsureCurrent();
  }
  
  interface IPlatformRenderInterface
  {
    // Instead of being initialized at startup it now wraps IPlatformGraphicsContext into IRenderingContext
    // which serves as a factory for render targets and layers
    IRenderingContext CreateRenderingContext(IPlatformGpuContext gpuContext);
  }
  interface IRenderingContext : IFeatureProvider, IDisposable
  {
    bool IsCorrupted { get; }
    IRenderTarget CreateRenderTarget(IEnumerable<object> surfaces);
    object GetFeature(Type t);
  }

Compositor would accept IPlatformGraphics directly, Deferred and Immediate renderers would use a wrapper object that creates the context on-demand

Renderer would be the sole entity that holds a reference to the graphics context. That causes a problem with texture-sharing powered opengl controls. For such controls the render platform context should expose:

    public interface IOpenGlTextureSharingRenderInterfaceContextFeature
    {
        bool CanCreateSharedContext { get; }
        IGlContext CreateSharedContext(IEnumerable<GlVersion> preferredVersions = null);
        IOpenGlBitmapImpl CreateOpenGlBitmap(PixelSize size, Vector dpi);
    }

while IRenderer now has

        public ValueTask<object?> TryGetRenderInterfaceFeature(Type featureType);

In case when the user wants to reuse their VkDevice, a similar interface will be provided as well. ValueTask is here due to the renderer potentially ticking on a background thread, so if context wasn't yet created, we need to wait for the next renderer tick.

Fixes #4128 #8320

@kekekeks kekekeks force-pushed the feature/context-management-8 branch 3 times, most recently from 8a6bab1 to a809faa Compare December 6, 2022 08:45
@kekekeks kekekeks force-pushed the feature/context-management-8 branch from 551e8e1 to eaf2ce3 Compare December 6, 2022 10:38
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027151-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027154-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

Egl = new EglInterface(eglGetProcAddress),
PlatformType = 0x31D7,
PlatformDisplay = device,
SupportsMultipleContexts = true,
Copy link
Member

Choose a reason for hiding this comment

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

API-wise, what's a point of SupportsMultipleContexts = true when IPlatformGraphics.CreateContext always throws an exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a difference between DrmOutput and EglDisplay

  • DrmOutput uses a pre-existing context to produce a render target and that can not use any other context to use said render target
  • EglDisplay needs to know if the native EGLDisplay can be trusted to create multiple contexts (which is the case here).

For example ANGLE uses single ID3D11Device for all contexts created from a single EGLDisplay which causes threading issues and native crashes, so we can't trust it to create multiple contexts properly.

We can change DrmOutput.CreateContext to allow context creation, but those contexts would be kinda useless.

@@ -19,6 +19,7 @@
using Avalonia.Utilities;
using Avalonia.Win32.Input;
using Avalonia.Win32.Interop;
using JetBrains.Annotations;
Copy link
Member

Choose a reason for hiding this comment

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

I want to remove JetBrains.Annotations at all from the repository. It can be and should be replaced with nullable attributes.
Out of scope of this PR tho.

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027174-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@danipen
Copy link

danipen commented Dec 6, 2022

@kekekeks does this PR fix Avalonia's OpenGL control rendering issues in windows os?

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027207-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@kekekeks
Copy link
Member Author

kekekeks commented Dec 7, 2022

No, this particular PR just fixes EGL_CONTEXT_LOST and provides better infrastructure for adding rendering backends

@danipen
Copy link

danipen commented Dec 7, 2022

@kekekeks are there any plans for fixing OpenGL control rendering issues in windows os soon?

@danwalmsley
Copy link
Member

@kekekeks are there any plans for fixing OpenGL control rendering issues in windows os soon?

@danipen that is also actively being worked on :)

@danipen
Copy link

danipen commented Dec 7, 2022

That's awesome thanks!

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027312-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@ltetak
Copy link
Contributor

ltetak commented Dec 9, 2022

see if you can break this with this DX commandline command
DXCap.exe -forcetdr

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027357-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member

maxkatz6 commented Dec 9, 2022

@ltetak it works with normal avalonia setup. But segfaults with UseWGL enabled with no meaningful error.

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027369-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0027379-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-nightly/nuget/v3/index.json) [PRBUILDID]

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.

OpenGlException: eglMakeCurrent failed with error EGL_CONTEXT_LOST
6 participants