[Enhancement] 网络背景图片缓存+异步下载#6113
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the background image loading mechanism to be asynchronous and includes a local caching strategy. It removes the synchronous WebURL-based image loading from FXUtils and introduces asyncFetchRemoteImage in DecoratorController to handle remote backgrounds. The review feedback identifies a potential data race on the changeBackgroundCount variable when accessed across different threads and highlights issues with the hardcoded cache path bg.png, which could lead to displaying stale images and file format inconsistencies.
| import static org.jackhuang.hmcl.util.logging.Logger.LOG; | ||
|
|
||
| public class DecoratorController { | ||
| private static final Path remoteBgCachePath = Metadata.HMCL_CURRENT_DIRECTORY.resolve("bg.png"); |
There was a problem hiding this comment.
remoteBgCachePath 被硬编码为程序根目录下的 bg.png。这存在以下问题：
- 缓存逻辑错误：由于缓存文件名固定且未与
backgroundImageUrl关联，当用户更改背景图 URL 时，getBackground(第 217 行) 会直接加载并显示旧 URL 的缓存图片，直到新图片下载完成。这会导致界面短暂显示错误的背景图。 - 文件规范：建议将缓存文件存放在专门的缓存目录（如
cache子目录）中，并根据 URL 生成唯一的文件名（例如使用 URL 的哈希值）。 - 格式兼容性：硬编码
.png后缀可能与实际下载的图片格式（如 JPG, WebP, SVG）不符。虽然 JavaFX 能够识别流内容，但错误的后缀可能在某些环境下引起问题。
| } | ||
|
|
||
| private void asyncFetchRemoteImage(@NotNull String backgroundImageUrl) { | ||
| final int currentCount = this.changeBackgroundCount; |
There was a problem hiding this comment.
此处存在多线程数据竞争（Data Race）。changeBackgroundCount 是一个普通的 int 字段，它在 JavaFX 线程中被 updateBackground 修改，但在此处（IO 线程）被读取。同样的问题也存在于第 354 行的异步回调中。
这可能导致 currentCount 获取到过期的值，从而在快速切换背景时导致逻辑错误（例如将旧请求的图片错误地应用或保存到缓存中）。
建议：
- 将
changeBackgroundCount声明为AtomicInteger。 - 或者将
currentCount作为参数从updateBackground传递给getBackground和asyncFetchRemoteImage，确保每个异步任务都持有其启动时的快照值。
|
|
||
| private void asyncFetchRemoteImage(@NotNull String backgroundImageUrl) { | ||
| final int currentCount = this.changeBackgroundCount; | ||
| new CacheFileTask(backgroundImageUrl) |
There was a problem hiding this comment.
请不要使用 CacheFileTask。CacheFileTask 会在设备上全局共享一些游戏相关的内容，而背景图更适合在局部进行缓存。把它缓存到全局文件夹的话，如果用户使用类似 Bing 壁纸这样的随机图片链接，那么可能会导致无用缓存越来越多。
2 participants
Closes #6079