bobbylight / RSyntaxTextArea

A syntax highlighting, code folding text editor for Java Swing applications.
BSD 3-Clause "New" or "Revised" License
1.12k stars 259 forks source link

Make it possible to use Font and FontMetrics per Token rather than To… #557

Closed robert-dbvis closed 4 months ago

robert-dbvis commented 4 months ago

This change creates the base for using fallback fonts.

RSyntaxTextArea gets two new methods getFontForToken and getFontMetricsForToken and the code is updated to call these instead of the ForTokenType versions. By default the two new methods just calls the ForTokenType methods, but a subclass can inspect the actual token and change the font/fontmetrics when needed.

bobbylight commented 4 months ago

@robert-dbvis - this seems like a fine change, but my understanding was that Swing already handled fallback fonts. Are you seeing that it doesn't? At least on OS X; maybe it's a Mac thing. But my understanding was that CJK glyphs would cause Swing to use a fallback font on all platforms, if the current font didn't support them.

If you can reproduce a fallback font issue, can you see whether you still see it with the changes in this ligature PR branch? That branch is mostly ready to merge, and it has notable changes to how Fonts are created and share rendering attributes. Might or might not make any difference, but might be worth checking.

robert-dbvis commented 4 months ago

A trivial swing program, so not using RSyntaxTextArea, does not show fallback for custom fonts for me, on linux (ubuntu) with jdk 17. Screenshot from 2024-07-01 10-18-54

Source code (But file renamed to .txt to be allowed): InternationalChars.txt

To run this program you need to get the jetbrains font and put the ttf in the same directory as the class.

So, to me it seems pretty clear that swing does not do automatic fallback on custom loaded fonts. Does this example program render the Chinese characters in both frames on Mac?

bobbylight commented 4 months ago

I'll try to check Mac tomorrow, but from what I recall, it does do fallbacks automagically there (as well as much better font rendering in general).

FWIW you can seem to get fallback fonts on Windows by using StyleContext, like RSTA does/tries to do, at least before the branch I linked to, but it seems to hard-code a fallback of the logical dialog font, which in turn means it's not monospaced:

Font font = StyleContext.getDefaultStyleContext().getFont("JetBrains Mono", Font.PLAIN, 15);

Of course this requires the font to be installed with the OS. So between that limitation and the fact that you can't control the fallback being monospaced, I don't think it's really a solution. I've gone down a rabbit hole of old OpenJDK bugs about fallback fonts, and from what I can tell it seems just not possible to override the default behavior beyond that.

bobbylight commented 4 months ago

Interestingly, it looks like the non-monospaced fallback behavior is the same in VS Code. So if this branch adds back the use of StyleContext for font creation, we'll have a fallback font, though not a great one, and match the behavior of arguably the most popular code editor. Right now I'm tempted to go that route.

That said I'm still open to giving you enough API to implement your own fallbacks if you think your app can do better, though I might not implement that functionality into RSTA itself. Does using StyleContext for fallback fonts break your desired approach?

Font settings: image

Result: image

Current RSTA release builds (using StyleContext): image

robert-dbvis commented 4 months ago

Monospaced fonts are not monospaced when it comes to more complex CJK characters, yes and that is fine for me.

I am not sure I understand what the image for "Current RSTA release builds (using StyleContext):" is supposed to show. It is not using the Jetbrains mono font (check the 'C', its ends are supposed to point up/down, not be horizontal).

I did a bit of experimentation on my own using StyleContext with the basic swing app I posted earlier and it seems I can get quite good results if I create and register the font before I call StyleContext to get a font.

bobbylight commented 4 months ago

Yeah, the screenshot was more to show that composite fonts were possible, and that RSTA didn't break that (since it does a lot of crazy stuff with fonts to support multiple fonts, etc.). Not specifically JetBrains Mono.

That said - does that mean you're good to go with StyleContext? I'd imagine you'd still need this PR merged if you were planning on shipping a font and using createFont(), right?

robert-dbvis commented 4 months ago

We tested a bit more, trying to use StyleContext and as I said it worked fine on my machine, but it did not work on another machine (probably because of fewer installed fonts). So, at the moment we are looking at a mix if StyleContext and font per token. So getting this merged would be helpful.

Since we have our own fork we can of course take care of this branch on our own, but since I was working on this and saw the requests from other users, then we thought it would be good to share back what we had done.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.29%. Comparing base (b230bf1) to head (c648c18). Report is 12 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #557 +/- ## ============================================ - Coverage 74.29% 74.29% -0.01% - Complexity 6839 6841 +2 ============================================ Files 177 177 Lines 30269 30271 +2 Branches 3915 3915 ============================================ + Hits 22489 22490 +1 - Misses 5940 5941 +1 Partials 1840 1840 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bobbylight commented 4 months ago

Thanks for the PR!