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 Renderer system from a simple enum to a sealed interface structure, introducing specialized Vulkan and OpenGL driver support. It adds automated detection of Vulkan ICD files on Windows, Linux, and macOS, and updates the launcher to configure environment variables accordingly. Key feedback includes a critical logic error where the OpenGL enum incorrectly returns the Vulkan API type and a performance concern regarding heavy hardware detection operations being performed synchronously, which could block the UI thread.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddbe930e0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| driverToIcdFile.putIfAbsent(driver, icdFile); |
There was a problem hiding this comment.
This path cache keeps the first ICD file seen for a driver (putIfAbsent), but the same driver key is populated from both 32-bit and 64-bit filenames (igvk32/igvk64, nv-vk32/nv-vk64, etc.). On machines where registry order lists the non-matching bitness first, the launcher will export a wrong-bitness ICD path and Vulkan initialization can fail for the selected renderer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
该 PR 旨在扩展 HMCL 的渲染器（尤其是 Vulkan）支持范围：通过在不同平台探测可用 Vulkan ICD/驱动文件，并重构渲染器模型与启动参数注入逻辑，让用户能够在更多 Vulkan 驱动/实现之间切换。
Changes:
- 将
Renderer从枚举重构为 sealed interface，并新增大量 Vulkan/OpenGL 驱动枚举项与“支持性/ICD 文件”探测逻辑 - 增强 GPU/驱动探测：Windows 从注册表读取 Vulkan ICD 文件；macOS 引入 Homebrew 路径推断并支持自定义 Vulkan 驱动加载
- 更新启动器环境变量与 UI/配置序列化：根据所选驱动设置
VK_*环境变量/org.lwjgl.vulkan.libname，并补齐多语言显示文案
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/windows/WindowsGPUDetector.java | 从注册表读取 VulkanDriverName 并解析为 Vulkan ICD 文件路径列表写入 GraphicsCard |
| HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/macos/HomebrewUtils.java | 新增 macOS Homebrew 前缀与 libvulkan.1.dylib 路径常量，供 Vulkan 驱动探测/加载使用 |
| HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/hardware/GraphicsCard.java | 为 GPU 信息新增 Vulkan 驱动文件列表字段与 builder 支持 |
| HMCLCore/src/main/java/org/jackhuang/hmcl/util/platform/Architecture.java | 增加 isArm() 便于按架构筛选 Vulkan 驱动支持 |
| HMCLCore/src/main/java/org/jackhuang/hmcl/launch/DefaultLauncher.java | 按所选驱动注入 mesa-loader/nativeDir、macOS Vulkan libname、以及 VK 环境变量 |
| HMCLCore/src/main/java/org/jackhuang/hmcl/game/Renderer.java | 渲染器系统核心重构：新增 Vulkan/OpenGL 驱动集、平台支持判断与 ICD 文件探测/映射 |
| HMCL/src/main/resources/assets/lang/I18N.properties | 增加大量 Vulkan 渲染器条目与描述（英文）并整理分组 |
| HMCL/src/main/resources/assets/lang/I18N_zh.properties | 增加大量 Vulkan 渲染器条目与描述（繁中）并调整标题文案 |
| HMCL/src/main/resources/assets/lang/I18N_zh_CN.properties | 增加大量 Vulkan 渲染器条目与描述（简中）并调整标题文案 |
| HMCL/src/main/java/org/jackhuang/hmcl/util/NativePatcher.java | 适配新的 Renderer 类型结构（例如 LLVMPIPE 常量位置变化） |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/AdvancedVersionSettingPage.java | UI 使用 Renderer.getSupported() 并按新 Renderer.Driver.api() 规则校验选择 |
| HMCL/src/main/java/org/jackhuang/hmcl/setting/VersionSetting.java | 反序列化改用 Renderer.of()；graphicsBackend 默认回退逻辑适配新模型 |
| HMCL/src/main/java/org/jackhuang/hmcl/game/LauncherHelper.java | 适配 mesaDriverName() 与新的 Driver 类型判断逻辑 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| List<Path> icdDirs = switch (OperatingSystem.CURRENT_OS) { | ||
| case LINUX -> List.of( | ||
| Path.of("/usr/share/vulkan/icd.d"), | ||
| Path.of("/etc/vulkan/icd.d") | ||
| ); | ||
| case FREEBSD -> List.of(Path.of("/usr/local/share/vulkan/icd.d")); | ||
| default -> List.of(); | ||
| }; | ||
|
|
||
| EnumSet<Vulkan> foundSupported = EnumSet.noneOf(Vulkan.class); | ||
| for (Path icdDir : icdDirs) { | ||
| if (!Files.isDirectory(icdDir)) | ||
| continue; | ||
| try (Stream<Path> stream = Files.list(icdDir)) { | ||
| for (Path icdFile : Lang.toIterable(stream)) { | ||
| String fileName = icdFile.getFileName().toString(); | ||
|
|
||
| Matcher matcher = icdFileNamePattern.matcher(fileName); | ||
| if (matcher.matches()) { | ||
| String icdName = matcher.group("name"); | ||
|
|
||
| Vulkan driver = icdNameToDriver.get(icdName); | ||
| if (driver != null) { | ||
| driverToIcdFile.put(driver, icdFile); | ||
|
|
||
| if (driver.isSupported(Platform.CURRENT_PLATFORM, SystemInfo.getGraphicsCards())) { | ||
| foundSupported.add(driver); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| LOG.warning("Failed to read Vulkan ICD files in " + icdDir, e); | ||
| } | ||
| } | ||
|
|
||
| supported.addAll(foundSupported); | ||
| } | ||
|
|
There was a problem hiding this comment.
Vulkan.Holder performs filesystem probing (Files.isDirectory / Files.list over system ICD directories) inside a static initializer. This introduces blocking I/O during class initialization, which can add noticeable latency when the UI first queries supported renderers. Consider deferring the directory scan to a lazily-computed method and caching the result, or running the probe off the UI thread.
| List<Path> icdDirs = switch (OperatingSystem.CURRENT_OS) { | |
| case LINUX -> List.of( | |
| Path.of("/usr/share/vulkan/icd.d"), | |
| Path.of("/etc/vulkan/icd.d") | |
| ); | |
| case FREEBSD -> List.of(Path.of("/usr/local/share/vulkan/icd.d")); | |
| default -> List.of(); | |
| }; | |
| EnumSet<Vulkan> foundSupported = EnumSet.noneOf(Vulkan.class); | |
| for (Path icdDir : icdDirs) { | |
| if (!Files.isDirectory(icdDir)) | |
| continue; | |
| try (Stream<Path> stream = Files.list(icdDir)) { | |
| for (Path icdFile : Lang.toIterable(stream)) { | |
| String fileName = icdFile.getFileName().toString(); | |
| Matcher matcher = icdFileNamePattern.matcher(fileName); | |
| if (matcher.matches()) { | |
| String icdName = matcher.group("name"); | |
| Vulkan driver = icdNameToDriver.get(icdName); | |
| if (driver != null) { | |
| driverToIcdFile.put(driver, icdFile); | |
| if (driver.isSupported(Platform.CURRENT_PLATFORM, SystemInfo.getGraphicsCards())) { | |
| foundSupported.add(driver); | |
| } | |
| } | |
| } | |
| } | |
| } catch (IOException e) { | |
| LOG.warning("Failed to read Vulkan ICD files in " + icdDir, e); | |
| } | |
| } | |
| supported.addAll(foundSupported); | |
| } | |
| startUnixLikeVulkanProbe(driverToIcdFile, supported); | |
| } | |
| private static void startUnixLikeVulkanProbe(Map<Vulkan, Path> driverToIcdFile, Set<Vulkan> supported) { | |
| Thread probeThread = new Thread(() -> { | |
| VulkanProbeResult probeResult = probeUnixLikeVulkanDrivers(); | |
| synchronized (driverToIcdFile) { | |
| driverToIcdFile.putAll(probeResult.driverToIcdFile); | |
| } | |
| synchronized (supported) { | |
| supported.addAll(probeResult.supported); | |
| } | |
| }, "Vulkan-ICD-Probe"); | |
| probeThread.setDaemon(true); | |
| probeThread.start(); | |
| } | |
| private static VulkanProbeResult probeUnixLikeVulkanDrivers() { | |
| List<Path> icdDirs = switch (OperatingSystem.CURRENT_OS) { | |
| case LINUX -> List.of( | |
| Path.of("/usr/share/vulkan/icd.d"), | |
| Path.of("/etc/vulkan/icd.d") | |
| ); | |
| case FREEBSD -> List.of(Path.of("/usr/local/share/vulkan/icd.d")); | |
| default -> List.of(); | |
| }; | |
| Map<Vulkan, Path> foundDriverToIcdFile = new EnumMap<>(Vulkan.class); | |
| EnumSet<Vulkan> foundSupported = EnumSet.noneOf(Vulkan.class); | |
| for (Path icdDir : icdDirs) { | |
| if (!Files.isDirectory(icdDir)) | |
| continue; | |
| try (Stream<Path> stream = Files.list(icdDir)) { | |
| for (Path icdFile : Lang.toIterable(stream)) { | |
| String fileName = icdFile.getFileName().toString(); | |
| Matcher matcher = icdFileNamePattern.matcher(fileName); | |
| if (matcher.matches()) { | |
| String icdName = matcher.group("name"); | |
| Vulkan driver = icdNameToDriver.get(icdName); | |
| if (driver != null) { | |
| foundDriverToIcdFile.put(driver, icdFile); | |
| if (driver.isSupported(Platform.CURRENT_PLATFORM, SystemInfo.getGraphicsCards())) { | |
| foundSupported.add(driver); | |
| } | |
| } | |
| } | |
| } | |
| } catch (IOException e) { | |
| LOG.warning("Failed to read Vulkan ICD files in " + icdDir, e); | |
| } | |
| } | |
| return new VulkanProbeResult(foundDriverToIcdFile, foundSupported); | |
| } | |
| private static final class VulkanProbeResult { | |
| private final Map<Vulkan, Path> driverToIcdFile; | |
| private final EnumSet<Vulkan> supported; | |
| private VulkanProbeResult(Map<Vulkan, Path> driverToIcdFile, EnumSet<Vulkan> supported) { | |
| this.driverToIcdFile = driverToIcdFile; | |
| this.supported = supported; | |
| } | |
| } |
No description provided.