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

AccessText.cs pen hardcoded to thickness of 1, should be using FontMetrics.UnderlinePosition and FontMetrics.UnderlineThickness. #17763

Open
tomlm opened this issue Dec 12, 2024 · 7 comments
Labels

Comments

@tomlm
Copy link

tomlm commented Dec 12, 2024

Describe the bug

 private protected override void RenderCore(DrawingContext context)
 {
     base.RenderCore(context);
     int underscore = Text?.IndexOf('_') ?? -1;

     if (underscore != -1 && ShowAccessKey)
     {
         var rect = TextLayout!.HitTestTextPosition(underscore);

         var x1 = Math.Round(rect.Left, MidpointRounding.AwayFromZero);
         var x2 = Math.Round(rect.Right, MidpointRounding.AwayFromZero);
         var y = Math.Round(rect.Bottom, MidpointRounding.AwayFromZero) - 1.5;

         context.DrawLine(
             new Pen(Foreground, 1),
             new Point(x1, y),
             new Point(x2, y));
     }
 }

This code is not using FontMetric.UnderlinPosition and FontMetric.UnderlingThickness. It is hard coded to thickness of 1.

To Reproduce

All access text rendering.

Expected behavior

It should honor the fontmetric data, something like this:

         double thickness = 1;
         double offset = 1.5;
         
         if (FontManager.Current.TryGetGlyphTypeface(new Typeface(FontFamily, FontStyle, FontWeight, FontStretch), out var glyphTypeface))
         {
             thickness = Math.Max(1, (double)glyphTypeface.Metrics.UnderlineThickness / (double)glyphTypeface.Metrics.DesignEmHeight);
             offset = (double)glyphTypeface.Metrics.UnderlinePosition / (double)glyphTypeface.Metrics.UnderlinePosition;
         }
         
         var x1 = Math.Round(rect.Left, MidpointRounding.AwayFromZero);
         var x2 = Math.Round(rect.Right, MidpointRounding.AwayFromZero);
         var y = Math.Round(rect.Bottom, MidpointRounding.AwayFromZero) - offset;

         context.DrawLine(
             new Pen(Foreground, thickness),
             new Point(x1, y),
             new Point(x2, y));
 }

Avalonia version

11.2.1

OS

Windows

Additional context

No response

@tomlm tomlm added the bug label Dec 12, 2024
@tomlm tomlm changed the title AccessText.cs is pen hardcoded to 1 pixel pen, should be using FontMetrics.UnderlinePosition and FontMetrics.UnderlineThickness. AccessText.cs pen hardcoded to thickness of 1, should be using FontMetrics.UnderlinePosition and FontMetrics.UnderlineThickness. Dec 12, 2024
@Gillibald
Copy link
Contributor

I think AccessText should just add a textStyleOverride to the TextLayout that shows a TextDecoration for the access key

@tomlm
Copy link
Author

tomlm commented Dec 14, 2024

Yeah, I was kind of surprised that approach was used given it derives from textblock. Seems like a text run that you toggle the decoration on would be better

@Gillibald
Copy link
Contributor

AccessText was never adjusted to the new capabilities. In the past you could not format text in such a way.

@tomlm
Copy link
Author

tomlm commented Dec 14, 2024

FWIW I implemented my own like this and it works perfectly:

    public sealed class MyAccessText : AccessText
    {
        private Run _accessRun;

        public MyAccessText()
        {
            this.PropertyChanged += OnPropertyChanged;
        }

        private void OnPropertyChanged(object sender, Avalonia.AvaloniaPropertyChangedEventArgs e)
        {
            switch (e.Property.Name)
            {
                case nameof(Text):
                    {
                        if (!String.IsNullOrEmpty(Text))
                        {
                            InlineCollection inlines = new InlineCollection();
                            var iPos = Text.IndexOf('_', StringComparison.Ordinal);
                            if (iPos >= 0 && iPos < Text.Length - 1)  
                            {
                                inlines.Add(new Run(Text[..iPos]));
                                _accessRun = new Run(Text[++iPos..++iPos]);
                                inlines.Add(_accessRun);
                                inlines.Add(new Run(Text[iPos..]));
                            }
                            else
                            {
                                _accessRun = null;
                                inlines.Add(new Run(Text));
                            }
                            this.Inlines = inlines;
                        }
                    }
                    break;

                case nameof(ShowAccessKey):
                    if (_accessRun != null)
                    {
                        if (this.ShowAccessKey)
                            _accessRun.TextDecorations = Avalonia.Media.TextDecorations.Underline;
                        else
                            _accessRun.TextDecorations = null;
                    }
                    break;
            }
        }
    }

@tomlm
Copy link
Author

tomlm commented Dec 14, 2024

Ah shoot. I can override AccessText for Menu/MenuItem, but it appears that overriding it for Label/Button/etc uses hardcoded readonly static definition in FuncDataTemplate.AccessText.

And ContentPresenter.CreateChild() is hard coded to use FuncDataTemplate.AccessText.

I sure wish Avalonia had a real dependency injection system exposed to clients.

@Gillibald
Copy link
Contributor

Just use the textStyleOverrides parameter on the TextLayout ctor

@tomlm
Copy link
Author

tomlm commented Dec 14, 2024

I figured out I can override the dependency in my application with an application level datatemplate. Thanks for your help @Gillibald

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

No branches or pull requests

2 participants