search

toptensoftware / RichTextKit

Rich text rendering for SkiaSharp
Other
366 stars 73 forks source link

StrikeThrough and Underline not visible for certain fonts and small sizes #58

Closed handerss-spotfire closed 1 year ago

handerss-spotfire commented 2 years ago

Certain fonts, especially when font size is small, result in strikethrough and underline not being rendered. I'm guessing this is because the thickness is determined by the font metric's StrikeoutThickness and UnderlineThickness properties, which may be smaller than 1. For some fonts this is worse than others. Whether the line is rendered also depends on the subpixel position of the line (and I guess what anti aliasing is used).

Should be reproducible with Arial font with a pixel font size smaller than 10 for example.

I can submit a PR to fix this. My two suggestions for the fix would be:

  1. Add UnderlineThickness and StrikeThroughThickness as properties to IStyle to let the user override. I guess this would lead to strikethrough not being centered when thickness is increased, maybe that's fine since it is an override.
  2. Set the minimum size to 1. Maybe this leads to too thick lines for certain anti aliasing values? I notice that underline thickness is set to 1 if UnderlineThickness is null, and strikethrough thickness is set to 0 if StrikeoutThickness. Settings it to 1 makes sense to me, but I don't understand defaulting to 0 for strikethrough.

Any other suggestion I could also take a look at.

toptensoftware commented 2 years ago

Not sure, but this may have already been fixed in the preview build by this commit:

https://github.com/toptensoftware/RichTextKit/commit/c10941b8aa4d37b56ad797d750ee9098a58e50a3

What build are you using?

handerss-spotfire commented 2 years ago

We're building for master. That commit does indeed fix the underline issue. Should something similar be done for strikethrough?

Is the preview branch a a stable "beta-esque" branch or is it simply the working branch?

EDIT: I see now that the preview packages on nuget are built from that branch. Will look into switching to that instead.

toptensoftware commented 1 year ago

I've made a similar change to underline for strikethrough - let me know if that helps.

Regarding the "preview" releases - that was a nuget imposed requirement because the package was dependent on preview SkiaSharp and Harfbuzz packages. Non-preview versions of those are now available so I've updated to those and removed the preview suffix from RichTextKit too.

This fix is in 0.4.164 and up on nuget now.

handerss-spotfire commented 1 year ago

That does fix the issue, thank you! Also thanks for clarifying the packaging situation.

I noticed that the fix seems to set the strokewidth for the incorrect paint object in the case of the strikethrough halo. I have opened a PR for that: https://github.com/toptensoftware/RichTextKit/pull/60.

On a related note to the drawing of halos I noticed that the ordering of the render calls can result in the underline disappearing with wide halos since the text halo is rendered after the underline. Strikethrough is handled correctly however since it is rendered last.