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

Introduced a convenient API for OpenGL drawing #14653

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kekekeks
Copy link
Member

@kekekeks kekekeks commented Feb 19, 2024

This PR adds a swapchain-like object, that allows presenting OpenGL content to CompositionDrawingSurface.

We've previously only had OpenGL control, but it has several shortcomings like not being able to share GPU resources between multiple OpenGL controls (i. e. instead of drawing the same mesh in 3 projections you have to upload the same mesh to 3 different OpenGL contexts) and said GPU resources being force-destroyed once the control gets detached from the visual tree.

With the new API the user has full control over the OpenGL context lifetime.

A convenience method is provided for creating a swapchain that can draw into an arbitrary Visual, said method automatically creates a CompositionVisual for drawing and makes it to be a child of the Visual and then manages resize handling.

[NotClientImplementable, Unstable]
public interface ICompositionGlContext : IAsyncDisposable
{
    /// <summary>
    /// The associated compositor
    /// </summary>
    Compositor Compositor { get; }
    /// <summary>
    /// The OpenGL context
    /// </summary>
    IGlContext Context { get; }
    /// <summary>
    /// Creates a swapchain that can draw into provided CompositionDrawingSurface instance
    /// </summary>
    /// <param name="surface">The surface to draw into</param>
    /// <param name="size">The pixel size for the textures generated by the swapchain</param>
    /// <param name="onDispose">The callback to be called when the swapchain is about to be disposed</param>
    ICompositionGlSwapchain CreateSwapchain(CompositionDrawingSurface surface, PixelSize size, Action? onDispose = null);
}

[NotClientImplementable, Unstable]
public interface ICompositionGlSwapchain : IAsyncDisposable
{
    /// <summary>
    /// Attempts to get the next texture in the swapchain. If all textures in the swapchain are currently queued for
    /// presentation, returns null
    /// </summary>
    ICompositionGlSwapchainLockedTexture? TryGetNextTexture();
    /// <summary>
    /// Gets the the next texture in the swapchain or extends the swapchain.
    /// Note that calling this method without waiting for previous textures to be presented can introduce
    /// high GPU resource usage
    /// </summary>
    ICompositionGlSwapchainLockedTexture GetNextTextureIgnoringQueueLimits();
    /// <summary>
    /// Asynchronously gets the next texture from the swapchain once one becomes available
    /// You should not be calling this method while your IGlContext is current
    /// </summary>
    ValueTask<ICompositionGlSwapchainLockedTexture> GetNextTextureAsync();
    /// <summary>
    /// Resizes the swapchain to a new pixel size
    /// </summary>
    void Resize(PixelSize size);
}

[NotClientImplementable, Unstable]
public interface ICompositionGlSwapchainLockedTexture : IDisposable
{
    /// <summary>
    /// The task you can use to wait for presentation to complete on the render thread
    /// </summary>
    public Task Presented { get; }

    /// <summary>
    /// The texture you are expected to render into. You can bind it to GL_COLOR_ATTACHMENT0 of your framebuffer.
    /// Note that the texture must be unbound before this object is disposed 
    /// </summary>
    public int TextureId { get; }
}

public static class OpenGLCompositionInterop
{
    public static async ValueTask<ICompositionGlContext?> TryCreateCompatibleGlContextAsync(this Compositor compositor,  OpenGLCompositionInteropContextCreationOptions? options = null);
}



public static class CompositionGlContextExtensions
{
    public static ICompositionGlSwapchain CreateSwapchain(this ICompositionGlContext context, Visual visual, PixelSize size);
}

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0045077-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]


public static class CompositionGlContextExtensions
{
public static ICompositionGlSwapchain CreateSwapchain(this ICompositionGlContext context, Visual visual, PixelSize size)
Copy link
Member

Choose a reason for hiding this comment

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

I would expect this method to have API semantics of CreateSurfaceVisual and SetElementChildVisual(visual, surfaceVisual) combined. As it's not much clear from the method name, that compositor visual is involved at all, without looking in the implementation.

Maybe something like CreateAndSetVisualSwapchain? Some docs would help as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

somewhat not agree with you Max, because swapchain is not for Visual. it's a standalone object for GPU buffer presenting, it's part of OpenGl although it's optional in a rendering system.

Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

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

My OpenGL knowledge is very inconsistent. Looks good overall.

@kekekeks kekekeks marked this pull request as draft February 23, 2024 07:44
grokys added a commit that referenced this pull request Mar 29, 2024
This can happen in cases like #14653 where:

1. An event is raised with 2 binding expression subscribers
2. The first subscriber causes the 2nd subscriber to be stopped
3. The second subscriber is called from the event, even though it has been stopped (as the event list was cached at step 1)
4. It calls `PublishValue` causing an exception

Easiest to just do nothing in `PublishValue` when this scenario happens.
maxkatz6 pushed a commit that referenced this pull request Mar 30, 2024
* Added failing test for #14753.

* Don't try to publish on non-running binding.

This can happen in cases like #14653 where:

1. An event is raised with 2 binding expression subscribers
2. The first subscriber causes the 2nd subscriber to be stopped
3. The second subscriber is called from the event, even though it has been stopped (as the event list was cached at step 1)
4. It calls `PublishValue` causing an exception

Easiest to just do nothing in `PublishValue` when this scenario happens.
maxkatz6 pushed a commit that referenced this pull request Apr 20, 2024
* Added failing test for #14753.

* Don't try to publish on non-running binding.

This can happen in cases like #14653 where:

1. An event is raised with 2 binding expression subscribers
2. The first subscriber causes the 2nd subscriber to be stopped
3. The second subscriber is called from the event, even though it has been stopped (as the event list was cached at step 1)
4. It calls `PublishValue` causing an exception

Easiest to just do nothing in `PublishValue` when this scenario happens.
@maxkatz6 maxkatz6 mentioned this pull request Apr 25, 2024
@jcyuan
Copy link
Contributor

jcyuan commented Apr 29, 2024

looks good, 👍
because it has these key features:
1, swapchain texture creation control (count limition)
2, swapchain size controlling
3, a way to create context

for 3, is there a sharewith option in the OpenGLCompositionInteropContextCreationOptions? because normally for a rich opengl app it will have its own context management, so share texture with this new context is a very basic requirement.

@jcyuan
Copy link
Contributor

jcyuan commented Apr 29, 2024

ah, oversight, there is actually no texture count controlling strategy. if i want that swapchain can only have 2 textures maximum (or even only 1), how to achieve this? (i will discard the coming drawing if its currently busy)

@jcyuan
Copy link
Contributor

jcyuan commented Apr 29, 2024

I'm thinking do we really need a swapchain for this purpose? if you say the Server compositor needs a swapchain, i can understand it because it's the final place to present everything. but for a user opengl texture (i believe texture is enough), ain't it just a resource for Server compositor to use? just like the way using skcanvas to draw an opengl texture.

@kekekeks
Copy link
Member Author

kekekeks commented May 1, 2024

Render thread might be running an entirely different drawing API and also might require utilization of some synchronization primitives (we currently have problems with the lack of sync, BTW).
So a queue of sorts is required.

Another point is being in sync with the rest of the visual tree, i. e. if you draw something in opengl control and have an overlay controls, you'd want the overlay state/position to match with your rendering.

@jcyuan
Copy link
Contributor

jcyuan commented May 1, 2024

Render thread might be running an entirely different drawing API and also might require utilization of some synchronization primitives (we currently have problems with the lack of sync, BTW). So a queue of sorts is required.

Another point is being in sync with the rest of the visual tree, i. e. if you draw something in opengl control and have an overlay controls, you'd want the overlay state/position to match with your rendering.

it does make sense, personally i prefer a lock to sync, unless there is a feature to limit the maximum texture count of the swapchain, in the old Avalonia version it takes too high GPU usage when the app is busy.

@jcyuan
Copy link
Contributor

jcyuan commented May 6, 2024

I'm wondering, why the way I'm using (skcanvas) it doesn't need a dual buffer? i guess it uses a copy mode or something? (copy my texture into it's own buffer?)
anyway recently this way (skcanvas to draw) brings me a blinking screen, not sure why, this only happens when the gpu is a little busy, i guess should be a problem related to sync. but if it uses copy mode, no sync needed i believe?

@kekekeks
Copy link
Member Author

kekekeks commented May 6, 2024

We currently have synchronization problems and need to introduce glWaitSync usage. Which is why the PR got postponed, IIRC.

@jcyuan
Copy link
Contributor

jcyuan commented May 7, 2024

We currently have synchronization problems and need to introduce glWaitSync usage. Which is why the PR got postponed, IIRC.

i got it, i feel you guys is illustrating a RHI backend. actually design a api which similar with vulkan's barrier/semphore that would be enough, because all modern backend has this (with different name)

@kekekeks
Copy link
Member Author

kekekeks commented May 7, 2024

We currently use DXGI shared mutexes for ANGLE-based OpenGL on windows, so synchronization works there. But for scenarios with shared OpenGL contexts we need to add gl fence support (or issue glFinish calls for GLES 2.0 contexts).

@jcyuan
Copy link
Contributor

jcyuan commented May 8, 2024

We currently use DXGI shared mutexes for ANGLE-based OpenGL on windows, so synchronization works there. But for scenarios with shared OpenGL contexts we need to add gl fence support (or issue glFinish calls for GLES 2.0 contexts).

yes i know that,
for DX i'm not faimilar with it, so not sure what exactly the mutex is, i guess just a CPU sync? because ANGLE is just a single threaded command queue

i don't think we need to support GLES2.0, because if there is no sync support, there is not, and it's old now.... for sync, glFinish is not enough i believe, even not work.

DX: (not sure)
VK: semphore (barrier only works within the same queue)
OPENGL: 3.x, Fence

for the design above, obviously it's a little complex for use and has the problem of memory overuse.

i'm thinking, if use 'server' waits for 'client' mode, will it lag the server rendering process? hmmm, should be fine bcause that's just a GPU command bubble gap and it won't be very long. but anyway i think this way is very easy to use, i just submit to server a texture & a fence i created for server to wait.

complex part i'd say. could take a lot of your time.

@kekekeks
Copy link
Member Author

kekekeks commented May 8, 2024

With ANGLE we are using a separate Direct3D device for UI thread rendering, IDXGIKeyedMutex is needed to properly synchronize access to the texture shared between UI and render threads. This also works well for Vulkan interop, since Vulkan supports those natively on Windows.

With Linux we support using EXT_external_objects mostly for Vulkan(UI)->OpenGL(render) interop, VK_KHR_external_semaphore_fd is supported when Vulkan is driving the render thread too.

However for Linux/mac/iOS/Android and WGL on Windows we are relying on OpenGL context sharing. So we need to use fences.

@MajesticBevans
Copy link

Unfortunately my knowledge of OpenGL isn't at the level where I can particularly help to implement this, but this is something I'd really like to see merged as soon as it's ready, so wanted to try and give it a priority bump!

@timunie
Copy link
Contributor

timunie commented Jul 18, 2024

You can ask for priority if you have paid support. Otherwise it's done when it's done

@MajesticBevans
Copy link

MajesticBevans commented Jul 19, 2024

You can ask for priority if you have paid support. Otherwise it's done when it's done

Ah okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants