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

Upgrade Avalonia to 11.2.1 #159

Merged
merged 21 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
<NoWarn>AVA3001</NoWarn>
</PropertyGroup>
<PropertyGroup>
<AvaloniaVersion>11.0.9</AvaloniaVersion>
<AvaloniaVersion>11.2.1</AvaloniaVersion>
</PropertyGroup>
</Project>
3 changes: 1 addition & 2 deletions src/Consolonia.Core/Consolonia.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@

<ItemGroup>
<PackageReference Include="Avalonia" Version="$(AvaloniaVersion)" />
<PackageReference Include="Avalonia.FreeDesktop" Version="$(AvaloniaVersion)" />
<PackageReference Include="Avalonia.Skia" Version="$(AvaloniaVersion)" />
<PackageReference Include="Avalonia.Desktop" Version="$(AvaloniaVersion)" />
<PackageReference Include="Avalonia.Controls.DataGrid" Version="$(AvaloniaVersion)" />
<PackageReference Include="CommunityToolkit.Mvvm" Version="8.3.2" />
<PackageReference Include="SkiaSharp.NativeAssets.Linux.NoDependencies" Version="2.88.8" />
Expand Down
12 changes: 9 additions & 3 deletions src/Consolonia.Core/Drawing/ConsoleBrush.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,16 @@ public static ConsoleBrush FromPosition(IBrush brush, int x, int y, int width, i
// Calculate the distance from the center
double dx = x - centerX;
double dy = y - centerY;
double distance = Math.Sqrt(dx * dx + dy * dy);

// Normalize the distance based on the brush radius
double normalizedDistance = distance / (Math.Min(width, height) * radialBrush.Radius);
// Calculate the distance based on separate X and Y radii
double distanceX = dx / (width * radialBrush.RadiusX.Scalar);
double distanceY = dy / (height * radialBrush.RadiusY.Scalar);
double distance = Math.Sqrt(distanceX * distanceX + distanceY * distanceY);

// Normalize the distance
double normalizedDistance = distance /
Math.Sqrt(radialBrush.RadiusX.Scalar * radialBrush.RadiusX.Scalar +
radialBrush.RadiusY.Scalar * radialBrush.RadiusY.Scalar);

// Clamp the normalized distance to [0, 1]
normalizedDistance = Math.Min(Math.Max(normalizedDistance, 0), 1);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using Avalonia;
using Avalonia.Platform;

namespace Consolonia.Core.Drawing
Expand All @@ -20,6 +21,13 @@ public IRenderTarget CreateRenderTarget(IEnumerable<object> surfaces)
return new RenderTarget(surfaces);
}

public IDrawingContextLayerImpl CreateOffscreenRenderTarget(PixelSize pixelSize, double scaling)
{
throw new NotImplementedException();
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved

public bool IsLost => false;

public IReadOnlyDictionary<Type, object> PublicFeatures => new Dictionary<Type, object>();
}
}
7 changes: 7 additions & 0 deletions src/Consolonia.Core/Drawing/ConsoloniaRenderInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,17 @@ public bool IsSupportedBitmapPixelFormat(PixelFormat format)
throw new NotImplementedException();
}

public IPlatformRenderInterfaceRegion CreateRegion()
{
throw new NotImplementedException();
}

public bool SupportsIndividualRoundRects => false;

public AlphaFormat DefaultAlphaFormat => throw new NotImplementedException();

public PixelFormat DefaultPixelFormat => throw new NotImplementedException();

public bool SupportsRegions => false;
}
}
26 changes: 23 additions & 3 deletions src/Consolonia.Core/Drawing/DrawingContextImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public DrawingContextImpl(ConsoleWindow consoleWindow, PixelBuffer pixelBuffer)

private Rect CurrentClip => _clipStack.Peek();

public RenderOptions RenderOptions { get; set; }

tomlm marked this conversation as resolved.
Show resolved Hide resolved
public void Dispose()
{
}
Expand Down Expand Up @@ -215,7 +217,7 @@ public void DrawGlyphRun(IBrush foreground, IGlyphRunImpl glyphRun)
DrawStringInternal(foreground, text, glyphRun.GlyphTypeface);
}

public IDrawingContextLayerImpl CreateLayer(Size size)
public IDrawingContextLayerImpl CreateLayer(PixelSize size)
{
return new RenderTarget(_consoleWindow);
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -234,6 +236,11 @@ public void PushClip(RoundedRect clip)
PushClip(clip.Rect);
}

public void PushClip(IPlatformRenderInterfaceRegion region)
{
throw new NotImplementedException();
}

tomlm marked this conversation as resolved.
Show resolved Hide resolved
public void PopClip()
{
_clipStack.Pop();
Expand Down Expand Up @@ -286,14 +293,27 @@ public object GetFeature(Type t)
throw new NotImplementedException();
}

public RenderOptions RenderOptions { get; set; }

public Matrix Transform
{
get => _transform;
set => _transform = value * _postTransform;
}

public void DrawRegion(IBrush brush, IPen pen, IPlatformRenderInterfaceRegion region)
{
throw new NotImplementedException();
}

tomlm marked this conversation as resolved.
Show resolved Hide resolved
public void PushLayer(Rect bounds)
{
throw new NotImplementedException();
}

public void PopLayer()
{
throw new NotImplementedException();
}

tomlm marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// Draw a straight horizontal line or vertical line
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/Consolonia.Core/Drawing/Line.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public bool TryGetSegment(double startDistance, double stopDistance, bool startO
public IGeometryImpl SourceGeometry { get; }
public Matrix Transform { get; }

public IGeometryImpl GetWidenedGeometry(IPen pen)
{
throw new NotImplementedException();
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved

public static Line CreateMyLine(Point p1, Point p2)
{
(double x, double y) = p2 - p1;
Expand Down
5 changes: 5 additions & 0 deletions src/Consolonia.Core/Drawing/Rectangle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public bool TryGetSegment(double startDistance, double stopDistance, bool startO
throw new NotImplementedException();
}

public IGeometryImpl GetWidenedGeometry(IPen pen)
{
throw new NotImplementedException();
}

public Rect Bounds => _rect;
public double ContourLength => (_rect.Width + _rect.Height) * 2;
public IGeometryImpl SourceGeometry { get; }
Expand Down
4 changes: 3 additions & 1 deletion src/Consolonia.Core/Drawing/RenderTarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ void IDrawingContextLayerImpl.Blit(IDrawingContextImpl context)

bool IDrawingContextLayerImpl.CanBlit => true;

public IDrawingContextImpl CreateDrawingContext()
public IDrawingContextImpl CreateDrawingContext(bool useScaledDrawing)
{
if (useScaledDrawing)
throw new NotImplementedException($"Consolonia doesn't support useScaledDrawing");
return new DrawingContextImpl(_consoleWindow, _bufferBuffer);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
//todo: check XTFocus https://github.com/jinek/Consolonia/issues/105#issuecomment-2089015880
private IInputRoot _owner;

public ArrowsAndKeyboardNavigationHandler(IKeyboardNavigationHandler keyboardNavigationHandler)
public ArrowsAndKeyboardNavigationHandler()

Check notice on line 18 in src/Consolonia.Core/Infrastructure/ArrowsAndKeyboardNavigationHandler.cs

View workflow job for this annotation

GitHub Actions / build

"[ConvertConstructorToMemberInitializers] Convert constructor into member initializers" on /home/runner/work/Consolonia/Consolonia/src/Consolonia.Core/Infrastructure/ArrowsAndKeyboardNavigationHandler.cs(18,16)
{
_keyboardNavigationHandler = keyboardNavigationHandler;
_keyboardNavigationHandler = new KeyboardNavigationHandler();
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider maintaining dependency injection pattern

The constructor change removes dependency injection capability by directly instantiating KeyboardNavigationHandler. This creates tight coupling and makes the class harder to test. Consider keeping the previous pattern:

-public ArrowsAndKeyboardNavigationHandler()
-{
-    _keyboardNavigationHandler = new KeyboardNavigationHandler();
-}
+public ArrowsAndKeyboardNavigationHandler(IKeyboardNavigationHandler keyboardNavigationHandler)
+{
+    _keyboardNavigationHandler = keyboardNavigationHandler ?? new KeyboardNavigationHandler();
+}

This maintains flexibility while providing a default implementation if needed.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public ArrowsAndKeyboardNavigationHandler()
{
_keyboardNavigationHandler = keyboardNavigationHandler;
_keyboardNavigationHandler = new KeyboardNavigationHandler();
public ArrowsAndKeyboardNavigationHandler(IKeyboardNavigationHandler keyboardNavigationHandler)
{
_keyboardNavigationHandler = keyboardNavigationHandler ?? new KeyboardNavigationHandler();

Copy link

Choose a reason for hiding this comment

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

Could you please explain why is it null? May be in new version we need to register the handler somewhere? I mean may be in older version it was registered by something else which is not in the game anymore?

Copy link
Collaborator Author

@tomlm tomlm Dec 2, 2024

Choose a reason for hiding this comment

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

The old Consololnia code base used to instantiate a KeyboardHandler in the ConsoloniaApplication and then pass it into the ArrowsAndKeyboardHandler instance as a singleton. Thus both the keyboard handler and the ArrowsAndKeyboard handler were singletons.

The new Avalonia code base has KeyboardHandler as a transient in the AvaloniaLocator, which means it is created on demand. According to documentation and discussions AvaloniaLocator is an internal resource resolver and not meant for general purpose dependency resolution. We are attempting to replace the one that is registered with the one that we have which wraps it. Soo...I looked at the registration of the dependency in the Avalonia side of things and it's just newing up a new instance, making it transient. I was not able to create a new registration in AvaloniaLocator for the outer navigator without creating a recursive dependency, so I elected to just instantiate it in the class.

If you have a cleaner way of doing that I'm open to it.

}

public void SetOwner(IInputRoot owner)
Expand Down
55 changes: 41 additions & 14 deletions src/Consolonia.Core/Infrastructure/ConsoleWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
using System.Threading.Tasks;
using Avalonia;
using Avalonia.Controls;
using Avalonia.Controls.Platform;
using Avalonia.Input;
using Avalonia.Input.Platform;
using Avalonia.Input.Raw;
using Avalonia.Input.TextInput;
using Avalonia.Platform;
using Avalonia.Platform.Storage;
using Avalonia.Rendering.Composition;
Expand Down Expand Up @@ -77,12 +78,21 @@ public IPopupImpl CreatePopup()

public void SetTransparencyLevelHint(IReadOnlyList<WindowTransparencyLevel> transparencyLevels)
{
throw new NotImplementedException("Consider this");
Debug.WriteLine($"ConsoleWindow.SetTransparencyLevelHint({transparencyLevels}) called, not implemented");
}

public void SetFrameThemeVariant(PlatformThemeVariant themeVariant)
{
//todo:
//todo: Light or dark
switch (themeVariant)
{
case PlatformThemeVariant.Dark:
Debug.WriteLine($"ConsoleWindow.SetFrameThemeVariant({themeVariant}) called, not implemented");
break;
case PlatformThemeVariant.Light:
Debug.WriteLine($"ConsoleWindow.SetFrameThemeVariant({themeVariant}) called, not implemented");
break;
}
}

public Size ClientSize
Expand Down Expand Up @@ -135,7 +145,8 @@ public void Activate()

public void SetTopmost(bool value)
{
throw new NotImplementedException();
// todo
Debug.WriteLine($"ConsoleWindow.SetTopmost({value}) called, not implemented");
}

public double DesktopScaling => 1d;
Expand All @@ -153,7 +164,6 @@ public void SetTopmost(bool value)
public Size MaxAutoSizeHint { get; }

// ReSharper disable once UnassignedGetOnlyAutoProperty todo: what is this property
public IScreenImpl Screen => null!;

public void SetTitle(string title)
{
Expand Down Expand Up @@ -185,7 +195,7 @@ public void ShowTaskbarIcon(bool value)

public void CanResize(bool value)
{
throw new NotImplementedException();
// todo, enable/dsiable resizing of window
tomlm marked this conversation as resolved.
Show resolved Hide resolved
}

public void BeginMoveDrag(PointerPressedEventArgs e)
Expand All @@ -211,22 +221,22 @@ public void Move(PixelPoint point)

public void SetMinMaxSize(Size minSize, Size maxSize)
{
throw new NotImplementedException();
//throw new NotImplementedException();
}

public void SetExtendClientAreaToDecorationsHint(bool extendIntoClientAreaHint)
{
throw new NotImplementedException();
// we don't support this, we can ignore
}

public void SetExtendClientAreaChromeHints(ExtendClientAreaChromeHints hints)
{
throw new NotImplementedException();
// we don't support this, we can ignore
}

public void SetExtendClientAreaTitleBarHeightHint(double titleBarHeight)
{
throw new NotImplementedException();
// we don't support this, we can ignore
}

public WindowState WindowState { get; set; }
Expand All @@ -249,12 +259,29 @@ public void SetExtendClientAreaTitleBarHeightHint(double titleBarHeight)

public object TryGetFeature(Type featureType)
{
if (featureType == typeof(ISystemNavigationManagerImpl))
return null;
if (featureType == typeof(ITextInputMethodImpl)) return null;
if (featureType == typeof(IStorageProvider))
return AvaloniaLocator.Current.GetService<IStorageProvider>();
throw new NotImplementedException("Consider this");

if (featureType == typeof(IInsetsManager))
// IInsetsManager doesn't apply to console applications.
return null;

if (featureType == typeof(IClipboard))
{
var clipboard = AvaloniaLocator.CurrentMutable.GetService<IClipboard>();
if (clipboard != null)
return clipboard;
}

// TODO ISystemNavigationManagerImpl should be implemented to handle BACK navigation between pages of controls like mobile apps do.
// TODO ITextInputMethodImplshould be implemented to handle text IME input
Debug.WriteLine($"Missing Feature: {featureType.Name} is not implemented but someone is asking for it!");
return null;
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved

public void GetWindowsZOrder(Span<Window> windows, Span<long> zOrder)
{
throw new NotImplementedException();
tomlm marked this conversation as resolved.
Show resolved Hide resolved
}

private void ConsoleOnMouseEvent(RawPointerEventType type, Point point, Vector? wheelDelta,
Expand Down
4 changes: 2 additions & 2 deletions src/Consolonia.Core/Infrastructure/ConsoloniaApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ public class ConsoloniaApplication : Application
public override void RegisterServices()
{
base.RegisterServices();
var keyboardNavigationHandler = AvaloniaLocator.Current.GetRequiredService<IKeyboardNavigationHandler>();

AvaloniaLocator.CurrentMutable.Bind<IKeyboardNavigationHandler>()
.ToConstant(new ArrowsAndKeyboardNavigationHandler(keyboardNavigationHandler));
.ToTransient<ArrowsAndKeyboardNavigationHandler>();
}
}
}
26 changes: 26 additions & 0 deletions src/Consolonia.Core/Infrastructure/ConsoloniaPlatform.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Diagnostics;
using System.Reflection;
using Avalonia;
using Avalonia.Controls.Platform;
using Avalonia.Input;
Expand Down Expand Up @@ -32,6 +33,11 @@
throw new NotImplementedException();
}

public ITopLevelImpl CreateEmbeddableTopLevel()
{
throw new NotImplementedException();
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved

public void Initialize()
{
NotSupported += InternalIgnore;
Expand All @@ -58,6 +64,26 @@
//.Bind<IPlatformSettings>().ToConstant(new PlatformSettingsStub())
//.Bind<ISystemDialogImpl>().ToConstant(new GtkSystemDialog())
/*.Bind<IMountedVolumeInfoProvider>().ToConstant(new LinuxMountedVolumeInfoProvider())*/;

if (OperatingSystem.IsWindows())
{
AvaloniaLocator.CurrentMutable.Bind<IClipboard>()
.ToFunc(() =>
{
var assembly = Assembly.Load("Avalonia.Win32");
var type = assembly.GetType(assembly.GetName().Name + ".ClipboardImpl");
var clipboard = (IClipboard)Activator.CreateInstance(type)!;

Check warning on line 75 in src/Consolonia.Core/Infrastructure/ConsoloniaPlatform.cs

View workflow job for this annotation

GitHub Actions / build

"[AssignNullToNotNullAttribute] Possible 'null' assignment to non-nullable entity" on /home/runner/work/Consolonia/Consolonia/src/Consolonia.Core/Infrastructure/ConsoloniaPlatform.cs(75,78)
return clipboard;
});
}
else if (OperatingSystem.IsMacOS())
{
// TODO: Implement or reuse MacOS clipboard
}
else if (OperatingSystem.IsLinux())
{
// TODO: Implement or reuse X11 Clipboard
}
}

[DebuggerStepThrough]
Expand Down
7 changes: 7 additions & 0 deletions src/Consolonia.Core/Text/FontManagerImpl.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using Avalonia.Media;
Expand Down Expand Up @@ -40,6 +41,12 @@ public bool TryCreateGlyphTypeface(string familyName, FontStyle style, FontWeigh
return true;
}

public bool TryCreateGlyphTypeface(Stream stream, FontSimulations fontSimulations,
[NotNullWhen(true)] out IGlyphTypeface glyphTypeface)
{
throw new NotImplementedException();
}
tomlm marked this conversation as resolved.
Show resolved Hide resolved

public bool TryCreateGlyphTypeface(Stream stream, out IGlyphTypeface glyphTypeface)
{
throw new NotImplementedException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public GalleryDataGrid()
// ReSharper disable once MemberCanBeMadeStatic.Local
private void DataGrid1_LoadingRow(object sender, DataGridRowEventArgs e)
{
e.Row.Header = e.Row.GetIndex() + 1;
e.Row.Header = e.Row.Index + 1;
}

private class ReversedStringComparer : IComparer<object>, IComparer
Expand Down
Loading
Loading