Skip to content

Perf: Optimized list loading of FontComboBox#6254

Open
KSSJW wants to merge 5 commits into
HMCL-dev:mainfrom
KSSJW-Contribution:font-combo-box-list-perf
Open

Perf: Optimized list loading of FontComboBox#6254
KSSJW wants to merge 5 commits into
HMCL-dev:mainfrom
KSSJW-Contribution:font-combo-box-list-perf

Conversation

@KSSJW

@KSSJW KSSJW commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

优化FontComboBox列表加载

缘由

  • 若环境中字体过多,列表加载明显缓慢。

更改

  • 独立线程加载,避免阻塞UI
  • 分批次加载,避免爆发性压力

实况

测试环境字体总数:335

更改前

1.mp4

更改后

2.mp4

@KSSJW

KSSJW commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces asynchronous loading of system fonts in FontComboBox to prevent UI freezing. However, the current implementation still blocks the UI thread during class loading because Font.getFamilies() is called in static initializers. Additionally, there are concurrency issues, such as the potential to spawn multiple loading threads on multiple clicks, a lack of thread visibility for the loaded flag, and the background thread not being set as a daemon thread. The review comments suggest moving the font retrieval to the background thread, marking loaded as volatile, checking if a thread is already running before starting a new one, and setting the thread as a daemon.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
@KSSJW

KSSJW commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies FontComboBox.java to load font families asynchronously in a background thread, preventing UI blocking. It initially loads the first 10 fonts and then fetches the remaining fonts in batches of 30 using a daemon thread. The review feedback highlights critical thread-safety and performance issues: declaring allFonts and headFonts as static fields introduces race conditions across multiple instances, calling itemsProperty().unbind() on a background thread violates JavaFX threading rules, and using stream filtering to find remaining fonts is inefficient compared to using subList.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
@KSSJW

KSSJW commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies FontComboBox to load system fonts asynchronously in batches, preventing UI thread blocking when opening the font dropdown. The review feedback highlights opportunities to improve thread safety by copying the font list on the UI thread before passing it to the background thread, and to prevent a potential memory leak by clearing the loadingThread reference and updating the loaded state on the JavaFX Application Thread once loading completes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/FontComboBox.java Outdated
@KSSJW KSSJW marked this pull request as ready for review June 28, 2026 06:03
@Glavo

Glavo commented Jun 28, 2026

Copy link
Copy Markdown
Member

我考虑之后重构一下,做成基于 LineButton 的实现,让用户在新的对话框中选择字体。

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.

2 participants