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

Fix window resize jitter with Metal #17415

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

kekekeks
Copy link
Member

@kekekeks kekekeks commented Nov 2, 2024

  1. use CAMetalLayer.presentsWithTransaction = YES; and manually trigger [CAMetalDrawable present] while wrapping it into a CATransaction when drawing from a non-UI thread
  2. Create a separate contentless layer as a backing layer for AvnView and let AppKit and CoreAnimation do their thing with it. CAMetalLayer is now added as a sublayer and is manually updated to proper size inside of a separate CATransaction with disabled animations.

Note that this needs extensive testing on multiple machines first (both Mx and Intel with and without dedicated GPU), unless it can be done quickly we should switch back to OpenGL as our default renderer).

Tested platforms:

  • M3 Max (MBP 2023) Sonoma
  • Intel + Radeon Pro 555 (MBP 2017) Ventura

Still need to test on Intel without dedicated GPU.

@kekekeks kekekeks added bug area-rendering os-macos customer-priority Issue reported by a customer with a support agreement. backport-candidate-11.2.x Consider this PR for backporting to 11.2 branch labels Nov 2, 2024
@kekekeks kekekeks force-pushed the fixes/metal-resize-jitter branch from 57cfb70 to ae337aa Compare November 2, 2024 09:22
@avaloniaui-bot
Copy link

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

@avaloniaui-bot
Copy link

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

var session = _native.BeginDrawing();
return new MetalDrawingSession(session);
}
catch (COMException com) when (com.HResult == (int)AvnResultCodes.E_AVN_RENDER_TARGET_NOT_READY)
Copy link
Member

Choose a reason for hiding this comment

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

Minor. But would it be possible to return HResult from BeginDrawing, instead of throwing COMException?
Just to avoid unfilterable exceptions noise.
Throwing RenderTargetNotReadyException exception is fine, as it can be easily filtered-out by the debugger by type.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't have non-HRESULT returns for methods with out object arguments. Not supported by MicroCOM and would require some extensive refactoring of the way argument marshaling works.

@danipen
Copy link

danipen commented Nov 4, 2024

I tested Intel Core i9 + AMD Radeon Pro 5500M, macOS Sonoma 14.7 on this PR (cibuild 11.3.999-cibuild0052991-alpha), but unfortunately I'm still getting the resize jitter. See attached video:

jitter.mov

Should I enable or config something?

@kekekeks
Copy link
Member Author

kekekeks commented Nov 4, 2024

@danipen I think we'll need a repro

@danipen
Copy link

danipen commented Nov 4, 2024

In 11.2.0 we're able to reproduce using the default application template:

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
        x:Class="AvaloniaResizeIssue.MainWindow"
        Title="AvaloniaResizeIssue">
    Welcome to Avalonia!
</Window>
resize.mov

I'm trying this PR now ... I'll post here if I'm able to reproduce too.

@danipen
Copy link

danipen commented Nov 4, 2024

Yes, same with nugets from this PR (11.3.999-cibuild0052991-alpha)

PR-repro.mov

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 5, 2024

@danipen can you double check that .NET sdk didn't use any cached nugets? Or if built wasn't found and it fallback to another.

Sandbox before this PR:
https://github.com/user-attachments/assets/56520d96-18bd-4b5f-897a-bff8e312a787

Sandbox on this PR:
https://github.com/user-attachments/assets/6f8afa3d-90f8-4dcf-be17-b329a9a4c363

@danipen
Copy link

danipen commented Nov 5, 2024

Yes, it seems that build 11.3.999-cibuild0052991-alpha is not available and it was falling back to 11.3.999-cibuild0052993. I tried 11.3.999-cibuild0052981-alpha and it works as expected. Sorry for the noise.

Just for the record, I'm getting the following error in the console:

Error compiling MSL shader: #include <metal_stdlib>
#include <simd/simd.h>
using namespace metal;
struct Inputs {
    float2 position  [[attribute(0)]];
    float4 color  [[attribute(1)]];
};
struct Outputs {
    float4 sk_Position [[position]];
    float4 vcolor_Stage0  [[user(locn0)]];
    float sk_PointSize [[point_size]];
};
struct uniformBuffer {
    float4 sk_RTAdjust;
};
struct Globals {
    constant uniformBuffer* _anonInterface0;
};


vertex Outputs vertexMain(Inputs _in [[stage_in]], constant uniformBuffer& _anonInterface0 [[buffer(0)]], uint sk_VertexID [[vertex_id]], uint sk_InstanceID [[instance_id]]) {
    Globals globalStruct{&_anonInterface0};
    thread Globals* _globals = &globalStruct;
    (void)_globals;
    Outputs _outputStruct;
    thread Outputs* _out = &_outputStruct;
    _out->vcolor_Stage0 = _in.color;
    _out->sk_Position = float4(_in.position.x, _in.position.y, 0.0, 1.0);
    _out->sk_Position = float4(_out->sk_Position.xy * _globals->_anonInterface0->sk_RTAdjust.xz + _out->sk_Position.ww * _globals->_anonInterface0->sk_RTAdjust.yw, 0.0, _out->sk_Position.w);
    _out->sk_Position.y = -_out->sk_Position.y;
    return *_out;
}

Error Domain=org.skia.ganesh Code=1 "Compilation took longer than 300 ms" UserInfo={NSLocalizedDescription=Compilation took longer than 300 ms}
Error compiling MSL shader: #include <metal_stdlib>
#include <simd/simd.h>
using namespace metal;
struct Inputs {
    float4 vcolor_Stage0  [[user(locn0)]];
};
struct Outputs {
    float4 sk_FragColor [[color(0)]];
};
struct uniformBuffer {
    float4 sk_RTAdjust;
};
struct Globals {
    constant uniformBuffer* _anonInterface0;
};
fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant uniformBuffer& _anonInterface0 [[buffer(0)]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
    Globals globalStruct{&_anonInterface0};
    thread Globals* _globals = &globalStruct;
    (void)_globals;
    Outputs _outputStruct;
    thread Outputs* _out = &_outputStruct;
    float4 outputColor_Stage0;
    {
        outputColor_Stage0 = _in.vcolor_Stage0;
    }
    {
        _out->sk_FragColor = outputColor_Stage0;
    }
    return *_out;
}

Error Domain=org.skia.ganesh Code=1 "Compilation took longer than 300 ms" UserInfo={NSLocalizedDescription=Compilation took longer than 300 ms}

@danipen
Copy link

danipen commented Nov 5, 2024

Additionally, I feel the OpenGL renderer is more performant. When resizing the window, it follows the mouse better when resizing. Please see the attached video: (left is OpenGL and right is Metal).

resize-perf.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-rendering backport-candidate-11.2.x Consider this PR for backporting to 11.2 branch bug customer-priority Issue reported by a customer with a support agreement. os-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants