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

Ensure fallback enabled fonts are used. #618

Merged
merged 1 commit into from Oct 7, 2020

Conversation

weisJ
Copy link
Contributor

@weisJ weisJ commented Sep 9, 2020

Description

Makes sure the used fonts are fallback enabled.

Motivation and Context

As reported in #499 (comment) Darklaf has issues with displaying certain character sets. This has been fixed in in darklaf version 2.4.7.

The issue stems from the fact that darklaf uses the native system font instead of the default Dialog font. Unlike the logical fonts physical fonts don't fallback to another font if certain characters are unsupported.
Because constructs like new Font(...) and Font#derive(getAttributes()) either don't produce a fallback enabled font or don't preserve the underlying Font2D instance (which is responsible for fallback handling if of type CompositeFont) those had to be replaced to ensure all characters are displayed correctly.

  • Font#derive(getAttributes()) was used to produce a non UIResource font and has been replaced by subclassing Font through NonUIResourceFont which exposes a constructor that takes a Font instance and preserves the Font2D handle.
  • new Font(...) was replaced by StyleContext#getFont(String,int,int) which creates a CompositeFont if necessary. (The result is wrapped using NonUIResourceFont because the result may be of type FontUIResource).

I'm not quite sure whether JMeterUIDefaults is the correct/best class to place the Font factory method though.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

@weisJ weisJ mentioned this pull request Sep 9, 2020
2 tasks
@weisJ weisJ force-pushed the composite_font branch 2 times, most recently from 4ca685e to df9f134 Compare September 9, 2020 16:06
…eserve the underlying Font2D handle.

Update darklaf to version 2.4.7 for fallback font support with darklaf themes.
@pmouawad
Copy link
Contributor

Hello @weisJ ,
It looks good to me. Thanks

@pmouawad
Copy link
Contributor

pmouawad commented Sep 27, 2020

Hello @weisJ ,
Did you also test with non darklaf LAF ?
Thanks

@weisJ
Copy link
Contributor Author

weisJ commented Sep 28, 2020

Hello @weisJ ,
Did you also test with non darklaf LAF ?
Thanks

Yes this does work with all LaFs. In particular this fix isn't really specific to darklaf but rather any LaF/Component that uses a non-logical font i.e. regardless of the Laf the following won't display the characters:

JFrame frame = new JFrame("Test");
JPanel content = new JPanel(new BorderLayout());
JLabel label = new JLabel("日 月 木 水 火 犬 王 正 出 本 右 四 左 玉");
label.setFont(new Font("Arial", Font.PLAIN, 12));
content.add(label);
frame.setContentPane(content);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.pack();
frame.setLocationRelativeTo(null);
frame.setVisible(true);

@pmouawad
Copy link
Contributor

Hello @weisJ ,
I don't reproduce the reported issue on Mac OSX.
So I would need somebody to test on Windows where it happens I guess.

Regards

@pmouawad
Copy link
Contributor

pmouawad commented Oct 7, 2020

Hello @weisJ ,
I was not able to reproduce the issue on windows neither.
Can you clarify what problem this issue fixes and how to reproduce the bug to confirm it's fixed ?
Thanks

@weisJ
Copy link
Contributor Author

weisJ commented Oct 7, 2020

Using Windows with Java 12.0.1 and Japanese as the selected language in JMeter

  • Master branch:
    font_issue

  • This branch:
    font_issue_fixed

Notice how the glyphs for the test plan label aren't rendered on master. The label is using a modified font-size. The old method of obtaining the derived font stripped away the composite font2d handle which is why the characters can't be displayed.

@pmouawad pmouawad merged commit fc334b0 into apache:master Oct 7, 2020
kkalinin pushed a commit to kkalinin/jmeter that referenced this pull request Mar 11, 2021
…eserve the underlying Font2D handle. (apache#618)

Ensure fallback enabled fonts are used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants