feat(vdd): 会话模式XML缓存（上限5个，启动失败自动清理） - #845
Conversation
- 模板密集的编译单元(confighttp.cpp)超过COFF 32767段上限,汇编器会静默 写坏COMDAT符号表导致typeinfo等符号链接时未定义,增加 -Wa,-mbig-obj - GUI托盘模式(SUNSHINE_TRAY=0)下main.cpp对system_tray::的引用缺少 预处理守卫,Debug(-O0)无死代码消除时链接失败,补齐与其他调用点 一致的守卫 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary by CodeRabbit
Walkthrough本次变更重构了 VDD 模式的预算、解析、缓存读取与 XML 持久化流程，补充失败路径缓存清理，并为 Windows 编译和系统托盘初始化增加编译期条件。 ChangesVDD 模式管线
平台编译与托盘条件
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant session_t
participant vdd_utils
participant confighttp
participant vdd_settings_xml
session_t->>vdd_utils: 准备 VDD 模式
vdd_utils->>vdd_settings_xml: 读取 global/temporary 模式
vdd_utils-->>session_t: 返回模式与缓存状态
session_t->>confighttp: 保存 VDD 模式
confighttp->>vdd_settings_xml: 写入 global/resolutions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/confighttp.h (1)
82-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value回退分支的预算估算与表注释前提相矛盾，建议补充说明其安全性依赖外部硬上限。
第 50 行注释明确指出 per-resolution 的
refresh_rate节点不是纯笛卡尔积行为，但第 95-97 行的回退路径恰恰用纯笛卡尔积差值来估算剩余额度。用实测表项校验：(8,6)实测 27，公式给出66-48=18（偏保守）；(0,6)实测 43，公式给出66-0=66（偏激进）。也就是说全局分辨率数越少，回退值越容易高估。当前之所以安全，是因为两个调用点（
src/confighttp.cpp的trim_vdd_temporary_modes、src/display_device/vdd_utils.cpp的trim_temporary_vdd_modes_to_limit）都对结果与VDD_MAX_CACHED_TEMPORARY_MODES（5）取了std::min。建议把这个隐式依赖写进注释，避免后续有人直接使用该函数返回值而失去兜底。📝 建议补充注释
const auto global_mode_budget = vdd_max_mode_combination_count(global_refresh_rate_count); const auto global_mode_count = global_resolution_count * global_refresh_rate_count; + // 注意：该回退值按纯笛卡尔积差值估算，在全局分辨率数较少时可能高估。 + // 调用方必须与 VDD_MAX_CACHED_TEMPORARY_MODES 取 min 后再使用。 return global_mode_count < global_mode_budget ? global_mode_budget - global_mode_count : VDD_FALLBACK_TEMPORARY_MODE_LIMIT;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/confighttp.h` around lines 82 - 98, 在 vdd_max_temporary_mode_count 的回退计算前补充注释，明确其笛卡尔积差值只是估算，可能高估剩余额度，且安全性依赖调用方使用 VDD_MAX_CACHED_TEMPORARY_MODES 通过 std::min 施加外部硬上限；不要修改现有计算逻辑。src/display_device/vdd_utils.cpp (1)
73-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value这些内部辅助函数具有外部链接却未在头文件声明，建议放入匿名命名空间。
make_vdd_mode_key、append_vdd_mode_latest、collect_vdd_mode_dimensions、count_vdd_mode_combinations、vdd_modes_equal、contains_vdd_mode都是本文件的实现细节，但目前都是外部链接符号，容易与其他 TU 中的同名符号发生 ODR 冲突。另外
vdd_modes_equal（第 114-126 行）手写的逐元素比较等价于lhs == rhs——std::vector<std::pair<std::string, std::string>>已提供operator==。♻️ 建议的重构
+ namespace { using vdd_mode_t = std::pair<std::string, std::string>; std::string make_vdd_mode_key(const std::string &resolution, const std::string &refresh_rate) {bool vdd_modes_equal(const std::vector<vdd_mode_t> &lhs, const std::vector<vdd_mode_t> &rhs) { - if (lhs.size() != rhs.size()) { - return false; - } - - for (std::size_t index = 0; index < lhs.size(); ++index) { - if (lhs[index] != rhs[index]) { - return false; - } - } - return true; + return lhs == rhs; }并在
append_configured_vdd_modes（第 239 行）之后关闭匿名命名空间。注意vdd_mode_t若被clear_vdd_mode_cache之外的外部代码使用，需保留在命名空间外。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/display_device/vdd_utils.cpp` around lines 73 - 134, 将 make_vdd_mode_key、append_vdd_mode_latest、collect_vdd_mode_dimensions、count_vdd_mode_combinations、vdd_modes_equal 和 contains_vdd_mode 放入匿名命名空间，并在 append_configured_vdd_modes 之后关闭该命名空间；根据实际外部使用情况保留 vdd_mode_t 在命名空间外。将 vdd_modes_equal 的逐元素比较替换为直接比较 lhs 与 rhs。src/display_device/session.cpp (1)
665-668: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
interface_missing也会触发缓存清理，但它与缓存内容无关。
update_vdd_resolution把interface_missing（旧驱动不提供 SETMODES IOCTL）和invalid_config一并折叠成vdd_mode_update_e::failed，因此这里会对这两种情况也清空缓存。这两种失败都不是"缓存里的模式导致驱动拒绝"，清理没有意义，只会在旧驱动上每次会话都产生一次无谓的 XML 写入。建议让
vdd_mode_update_e区分"驱动拒绝了模式列表"和"接口不可用/入参无效"，只对前者清理。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/display_device/session.cpp` around lines 665 - 668, 调整 update_vdd_resolution 使用的 vdd_mode_update_e，将“驱动拒绝模式列表”与 interface_missing、invalid_config 等接口不可用/参数无效情况区分开。更新会话处理逻辑中对 vdd_utils::clear_vdd_mode_cache() 的判断，仅在驱动拒绝模式列表时清理缓存；其他失败状态直接返回 false，保持旧驱动不会触发无谓写入。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/confighttp.cpp`:
- Around line 1385-1395: Update put_vdd_common_nodes to modify existing
“monitors” and “gpu” child nodes in iddOptionTree rather than replacing them
with new trees. Preserve all unrelated existing fields while updating only
“monitors.count” and “gpu.friendlyname”, creating the child nodes only when they
do not already exist.
- Around line 1444-1458: 在读取现有配置的 try/catch 流程中，解析 XML 失败后不要继续执行
put_vdd_common_nodes、global/resolutions
更新及后续落盘；应通过该保存函数的现有失败传播机制立即返回失败，让上层感知错误并保留原文件内容。仅在 pt::read_xml 成功时继续使用
iddOptionTree 更新配置。
- Around line 1306-1334: 统一分辨率解析的分隔符处理：更新 append_vdd_resolution_node 和
normalize_vdd_modes，使查找分隔符同时接受大小写形式的 x，确保类似 1920X1080
的配置不会被静默丢弃，并保持现有无效格式返回失败的行为。
- Around line 1270-1304: Limit the sizes of the resolutions and fps arrays in
parse_vdd_array before saveVddSettings can pass unbounded input into
normalize_vdd_modes, using a clear validation failure for oversized requests. In
normalize_vdd_modes, replace repeated linear remove_if/string concatenation
deduplication with hash-based key tracking, while preserving last-occurrence
ordering. Compute the number of modes to discard once and erase that prefix in a
single operation instead of rebuilding sets for each deletion.
In `@src/display_device/session.cpp`:
- Around line 671-682: 将 vdd_settings 临时模式的持久化从当前保存位置移到 prepare_vdd
流程全部成功、接近成功返回前的位置，确保失败路径不会调用 clear_vdd_mode_cache()
撤销已写入的历史模式；保留现有失败处理逻辑，并确保成功返回前只执行一次保存。
In `@src/display_device/vdd_utils.cpp`:
- Around line 200-204: Align VDD mode readback and generation by updating
append_vdd_mode_latest’s loop in src/display_device/vdd_utils.cpp lines 200-204
to iterate resolutions outermost and refresh rates innermost, matching
append_configured_vdd_modes; also update trim_temporary_vdd_modes_to_limit at
src/display_device/vdd_utils.cpp lines 136-151 to remove only modes outside the
intersection of global resolutions and global refresh rates, matching
trim_vdd_temporary_modes. Both sites require changes so needs_cache_update and
mode_combination_count reflect the persisted content.
- Around line 938-959: 将临时模式加入 settings 的分辨率和刷新率集合后，mode_combination_count
必须按合并后的完整集合计算实际笛卡尔积，而不是继续使用全局组合数加临时模式数量。更新 count_vdd_mode_combinations 的调用及
trim_temporary_vdd_modes_to_limit 的收敛判断，使裁剪依据 vdd_max_mode_combination_count，并与
set_vdd_session_mode 的实际下发数量一致。
- Around line 965-978: Update clear_vdd_mode_cache so temporary modes are
removed even when global_modes is empty. When no global modes exist, directly
remove the resolutions entries carrying refresh_rate from the persisted VDD
settings XML, rather than calling saveVddModeSettings; preserve the existing
save path for non-empty global_modes and keep warning behavior for cleanup
failures.
---
Nitpick comments:
In `@src/confighttp.h`:
- Around line 82-98: 在 vdd_max_temporary_mode_count
的回退计算前补充注释，明确其笛卡尔积差值只是估算，可能高估剩余额度，且安全性依赖调用方使用 VDD_MAX_CACHED_TEMPORARY_MODES 通过
std::min 施加外部硬上限；不要修改现有计算逻辑。
In `@src/display_device/session.cpp`:
- Around line 665-668: 调整 update_vdd_resolution 使用的
vdd_mode_update_e，将“驱动拒绝模式列表”与 interface_missing、invalid_config
等接口不可用/参数无效情况区分开。更新会话处理逻辑中对 vdd_utils::clear_vdd_mode_cache()
的判断，仅在驱动拒绝模式列表时清理缓存；其他失败状态直接返回 false，保持旧驱动不会触发无谓写入。
In `@src/display_device/vdd_utils.cpp`:
- Around line 73-134: 将
make_vdd_mode_key、append_vdd_mode_latest、collect_vdd_mode_dimensions、count_vdd_mode_combinations、vdd_modes_equal
和 contains_vdd_mode 放入匿名命名空间，并在 append_configured_vdd_modes
之后关闭该命名空间；根据实际外部使用情况保留 vdd_mode_t 在命名空间外。将 vdd_modes_equal 的逐元素比较替换为直接比较 lhs 与
rhs。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fa3dee55-28ce-4537-aa9a-f7a49d7c04cc
📒 Files selected for processing (7)
cmake/compile_definitions/windows.cmakesrc/confighttp.cppsrc/confighttp.hsrc/display_device/session.cppsrc/display_device/vdd_utils.cppsrc/display_device/vdd_utils.hsrc/main.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
cmake/**
⚙️ CodeRabbit configuration file
cmake/**: CMake 构建系统文件。审查跨平台兼容性、现代 CMake 实践。
Files:
cmake/compile_definitions/windows.cmake
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/main.cppsrc/display_device/vdd_utils.hsrc/display_device/session.cppsrc/display_device/vdd_utils.cppsrc/confighttp.hsrc/confighttp.cpp
🔇 Additional comments (11)
cmake/compile_definitions/windows.cmake (1)
17-20: 请将-Wa,-mbig-obj限定到实际支持的编译器。该参数当前无条件加入
SUNSHINE_COMPILE_OPTIONS。如果 Windows 构建矩阵包含 MSVC 或clang-cl，它可能被传给不兼容的编译器并导致编译失败；请确认项目是否明确只支持 MinGW GCC。若支持其他工具链，建议使用 compiler-ID/language guard 或COMPILE_LANG_AND_ID生成器表达式。CMake 官方文档也建议对编译器专用选项进行条件化。 (cmake.org)建议实现
- list(APPEND SUNSHINE_COMPILE_OPTIONS -Wa,-mbig-obj) + list(APPEND SUNSHINE_COMPILE_OPTIONS + "$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wa,-mbig-obj>" + "$<$<COMPILE_LANG_AND_ID:CXX,GNU>:-Wa,-mbig-obj>")src/main.cpp (1)
128-130: 🎯 Functional Correctness无需修改：
SUNSHINE_TRAY已通过SUNSHINE_DEFINITIONS传递给sunshine目标。
cmake/compile_definitions/common.cmake将该变量追加到定义列表，随后cmake/targets/common.cmake通过target_compile_definitions(sunshine PUBLIC ${SUNSHINE_DEFINITIONS})传入main.cpp。src/display_device/session.cpp (2)
709-709: 这些失败路径的清理调用与第 671-682 行评论中指出的"先写盘再清盘、全量清空"是同一根因，不再重复展开。Also applies to: 739-745, 762-762
10-10: LGTM!Also applies to: 559-559
src/confighttp.h (3)
7-16: LGTM!
19-59: LGTM!
100-114: LGTM!src/display_device/vdd_utils.h (1)
5-9: LGTM!Also applies to: 53-61, 269-276
src/confighttp.cpp (2)
1241-1268: LGTM!
1484-1501: LGTM!src/display_device/vdd_utils.cpp (1)
232-239: 🩺 Stability & Availability无需调整并发访问。
config::update_full_config只保护文件写入，VDD 设置保存走saveVddModeSettings，不会通过配置更新路径修改config::nvhttp.resolutions/.fps；这些字段仅在初始加载时解析配置文件，当前没有运行时写入路径。> Likely an incorrect or invalid review comment.
| std::vector<VddMode> | ||
| normalize_vdd_modes(const std::vector<VddMode> &modes, bool enforce_global_combination_limit = true) { | ||
| std::vector<VddMode> normalized; | ||
|
|
||
| for (const auto &mode : modes) { | ||
| auto resolution = trim_vdd_token(mode.first); | ||
| auto refresh_rate = trim_vdd_token(mode.second); | ||
| if (resolution.empty() || refresh_rate.empty() || resolution.find('x') == std::string::npos) { | ||
| continue; | ||
| } | ||
|
|
||
| const auto key = resolution + "@" + refresh_rate; | ||
| normalized.erase(std::remove_if(normalized.begin(), normalized.end(), [&](const auto &existing_mode) { | ||
| return existing_mode.first + "@" + existing_mode.second == key; | ||
| }), | ||
| normalized.end()); | ||
| normalized.emplace_back(std::move(resolution), std::move(refresh_rate)); | ||
| } | ||
|
|
||
| while (enforce_global_combination_limit && !normalized.empty()) { | ||
| std::set<std::string> resolutions; | ||
| std::set<std::string> refresh_rates; | ||
| for (const auto &mode : normalized) { | ||
| resolutions.insert(mode.first); | ||
| refresh_rates.insert(mode.second); | ||
| } | ||
|
|
||
| const auto combination_limit = vdd_max_mode_combination_count(refresh_rates.size()); | ||
| if (resolutions.size() * refresh_rates.size() <= combination_limit) { | ||
| break; | ||
| } | ||
| normalized.erase(normalized.begin()); | ||
| } | ||
| return normalized; | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
在 HTTPS 请求线程上执行的 O(n²·log n) 归一化，输入规模不受限。
两处都是二次复杂度：
- 第 1282-1285 行的去重对每个元素做一次全量
remove_if并拼接字符串，整体 O(n²)。 - 第 1289-1302 行的裁剪循环每轮重建两个
std::set（O(n log n)）却只删除一个元素，整体 O(n²·log n)。
输入 n = |resolutions| × |refresh_rates| 来自 saveVddSettings，而 saveConfig（第 1524-1539 行）只校验了 sunshine_name 和 adapter_name 的长度，对 resolutions/fps 数组没有任何数量限制。提交 500×500 的数组即可让请求线程长时间挂死；server.config.thread_pool_size = 2（第 3405 行），两个这样的请求就能拖垮整个 Web UI。
建议在 parse_vdd_array 处加数量上限，并把去重改用哈希集合：
🔧 建议修复
std::vector<VddMode>
normalize_vdd_modes(const std::vector<VddMode> &modes, bool enforce_global_combination_limit = true) {
+ // 防止请求线程被超大输入拖死
+ constexpr std::size_t MAX_INPUT_MODES = 4096;
+ if (modes.size() > MAX_INPUT_MODES) {
+ BOOST_LOG(warning) << "VDD 模式数量超过上限，拒绝处理: " << modes.size();
+ return {};
+ }
+
std::vector<VddMode> normalized;
+ std::unordered_map<std::string, std::size_t> index_by_key;
for (const auto &mode : modes) {
auto resolution = trim_vdd_token(mode.first);
auto refresh_rate = trim_vdd_token(mode.second);
if (resolution.empty() || refresh_rate.empty() || resolution.find('x') == std::string::npos) {
continue;
}
const auto key = resolution + "@" + refresh_rate;
- normalized.erase(std::remove_if(normalized.begin(), normalized.end(), [&](const auto &existing_mode) {
- return existing_mode.first + "@" + existing_mode.second == key;
- }),
- normalized.end());
- normalized.emplace_back(std::move(resolution), std::move(refresh_rate));
+ if (auto it = index_by_key.find(key); it != index_by_key.end()) {
+ continue; // 已存在，保留首次出现的顺序
+ }
+ index_by_key.emplace(key, normalized.size());
+ normalized.emplace_back(std::move(resolution), std::move(refresh_rate));
}裁剪循环也建议改为先算出需要删除的数量后一次性 erase(begin(), begin() + count)。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/confighttp.cpp` around lines 1270 - 1304, Limit the sizes of the
resolutions and fps arrays in parse_vdd_array before saveVddSettings can pass
unbounded input into normalize_vdd_modes, using a clear validation failure for
oversized requests. In normalize_vdd_modes, replace repeated linear
remove_if/string concatenation deduplication with hash-based key tracking, while
preserving last-occurrence ordering. Compute the number of modes to discard once
and erase that prefix in a single operation instead of rebuilding sets for each
deletion.
| bool | ||
| saveVddSettings(std::string resArray, std::string fpsArray, std::string gpu_name) { | ||
| pt::ptree iddOptionTree; | ||
| pt::ptree global_node; | ||
| pt::ptree resolutions_nodes; | ||
| append_vdd_resolution_node(pt::ptree &resolutions_nodes, const std::string &resolution, const std::string *refresh_rate = nullptr) { | ||
| const auto index = resolution.find('x'); | ||
| if (index == std::string::npos) { | ||
| return false; | ||
| } | ||
|
|
||
| // prepare resolutions setting for vdd | ||
| boost::regex pattern("\\[|\\]|\\s+"); | ||
| char delimiter = ','; | ||
| auto width = resolution.substr(0, index); | ||
| auto height = resolution.substr(index + 1); | ||
| boost::algorithm::trim(width); | ||
| boost::algorithm::trim(height); | ||
|
|
||
| // 添加全局刷新率到global节点 | ||
| for (const auto &fps : split(boost::regex_replace(fpsArray, pattern, ""), delimiter)) { | ||
| global_node.add("g_refresh_rate", fps); | ||
| if (width.empty() || height.empty()) { | ||
| return false; | ||
| } | ||
|
|
||
| std::string str = boost::regex_replace(resArray, pattern, ""); | ||
| boost::algorithm::trim(str); | ||
| for (const auto &resolution : split(str, delimiter)) { | ||
| auto index = resolution.find('x'); | ||
| if(index == std::string::npos) { | ||
| pt::ptree res_node; | ||
| res_node.put("width", width); | ||
| res_node.put("height", height); | ||
| if (refresh_rate) { | ||
| auto refresh_rate_value = trim_vdd_token(*refresh_rate); | ||
| if (refresh_rate_value.empty()) { | ||
| return false; | ||
| } | ||
| res_node.put("refresh_rate", refresh_rate_value); | ||
| } | ||
| resolutions_nodes.push_back(std::make_pair("resolution"s, res_node)); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
分辨率分隔符只接受小写 x，与读取侧的大小写容忍度不一致。
这里（第 1308 行）和 normalize_vdd_modes（第 1277 行）都只查找小写 'x'，而 vdd_utils.cpp 的 parse_vdd_resolution（第 437 行）显式接受 'x' 和 'X'。
结果是形如 1920X1080 的条目会在 normalize_vdd_modes 阶段被静默丢弃，用户既看不到错误提示也不知道配置没生效。
建议统一为大小写不敏感，或至少在丢弃时记日志：
🔧 建议修复
bool
append_vdd_resolution_node(pt::ptree &resolutions_nodes, const std::string &resolution, const std::string *refresh_rate = nullptr) {
- const auto index = resolution.find('x');
+ const auto index = resolution.find_first_of("xX");
if (index == std::string::npos) {
return false;
}normalize_vdd_modes 第 1277 行的 resolution.find('x') 同样需要改为 find_first_of("xX")。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool | |
| saveVddSettings(std::string resArray, std::string fpsArray, std::string gpu_name) { | |
| pt::ptree iddOptionTree; | |
| pt::ptree global_node; | |
| pt::ptree resolutions_nodes; | |
| append_vdd_resolution_node(pt::ptree &resolutions_nodes, const std::string &resolution, const std::string *refresh_rate = nullptr) { | |
| const auto index = resolution.find('x'); | |
| if (index == std::string::npos) { | |
| return false; | |
| } | |
| // prepare resolutions setting for vdd | |
| boost::regex pattern("\\[|\\]|\\s+"); | |
| char delimiter = ','; | |
| auto width = resolution.substr(0, index); | |
| auto height = resolution.substr(index + 1); | |
| boost::algorithm::trim(width); | |
| boost::algorithm::trim(height); | |
| // 添加全局刷新率到global节点 | |
| for (const auto &fps : split(boost::regex_replace(fpsArray, pattern, ""), delimiter)) { | |
| global_node.add("g_refresh_rate", fps); | |
| if (width.empty() || height.empty()) { | |
| return false; | |
| } | |
| std::string str = boost::regex_replace(resArray, pattern, ""); | |
| boost::algorithm::trim(str); | |
| for (const auto &resolution : split(str, delimiter)) { | |
| auto index = resolution.find('x'); | |
| if(index == std::string::npos) { | |
| pt::ptree res_node; | |
| res_node.put("width", width); | |
| res_node.put("height", height); | |
| if (refresh_rate) { | |
| auto refresh_rate_value = trim_vdd_token(*refresh_rate); | |
| if (refresh_rate_value.empty()) { | |
| return false; | |
| } | |
| res_node.put("refresh_rate", refresh_rate_value); | |
| } | |
| resolutions_nodes.push_back(std::make_pair("resolution"s, res_node)); | |
| return true; | |
| } | |
| bool | |
| append_vdd_resolution_node(pt::ptree &resolutions_nodes, const std::string &resolution, const std::string *refresh_rate = nullptr) { | |
| const auto index = resolution.find_first_of("xX"); | |
| if (index == std::string::npos) { | |
| return false; | |
| } | |
| auto width = resolution.substr(0, index); | |
| auto height = resolution.substr(index + 1); | |
| boost::algorithm::trim(width); | |
| boost::algorithm::trim(height); | |
| if (width.empty() || height.empty()) { | |
| return false; | |
| } | |
| pt::ptree res_node; | |
| res_node.put("width", width); | |
| res_node.put("height", height); | |
| if (refresh_rate) { | |
| auto refresh_rate_value = trim_vdd_token(*refresh_rate); | |
| if (refresh_rate_value.empty()) { | |
| return false; | |
| } | |
| res_node.put("refresh_rate", refresh_rate_value); | |
| } | |
| resolutions_nodes.push_back(std::make_pair("resolution"s, res_node)); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/confighttp.cpp` around lines 1306 - 1334, 统一分辨率解析的分隔符处理：更新
append_vdd_resolution_node 和 normalize_vdd_modes，使查找分隔符同时接受大小写形式的 x，确保类似
1920X1080 的配置不会被静默丢弃，并保持现有无效格式返回失败的行为。
| void | ||
| put_vdd_common_nodes(pt::ptree &iddOptionTree, const std::string &gpu_name) { | ||
| pt::ptree monitor_node; | ||
| monitor_node.put("count", 1); | ||
|
|
||
| pt::ptree gpu_node; | ||
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | ||
|
|
||
| iddOptionTree.put_child("monitors", monitor_node); | ||
| iddOptionTree.put_child("gpu", gpu_node); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
put_vdd_common_nodes 无条件覆盖整个 monitors 节点，会丢弃已有的其他子项。
put_child("monitors", monitor_node) 用一个只含 count 的新节点整体替换原有 monitors 子树。如果 vdd_settings.xml 中的 monitors 还包含 count 之外的字段（由驱动或用户手工添加），这些字段在每次保存后都会被静默丢弃。
gpu 节点同理——只保留 friendlyname。
建议改为在已读取的既有节点上按字段更新，而不是替换整个子树：
🔧 建议修复
void
put_vdd_common_nodes(pt::ptree &iddOptionTree, const std::string &gpu_name) {
- pt::ptree monitor_node;
- monitor_node.put("count", 1);
-
- pt::ptree gpu_node;
- gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name);
-
- iddOptionTree.put_child("monitors", monitor_node);
- iddOptionTree.put_child("gpu", gpu_node);
+ // 只更新自己负责的字段，保留同级的其他既有配置
+ iddOptionTree.put("monitors.count", 1);
+ iddOptionTree.put("gpu.friendlyname", gpu_name.empty() ? "default" : gpu_name);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void | |
| put_vdd_common_nodes(pt::ptree &iddOptionTree, const std::string &gpu_name) { | |
| pt::ptree monitor_node; | |
| monitor_node.put("count", 1); | |
| pt::ptree gpu_node; | |
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | |
| iddOptionTree.put_child("monitors", monitor_node); | |
| iddOptionTree.put_child("gpu", gpu_node); | |
| } | |
| void | |
| put_vdd_common_nodes(pt::ptree &iddOptionTree, const std::string &gpu_name) { | |
| iddOptionTree.put("monitors.count", 1); | |
| iddOptionTree.put("gpu.friendlyname", gpu_name.empty() ? "default" : gpu_name); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/confighttp.cpp` around lines 1385 - 1395, Update put_vdd_common_nodes to
modify existing “monitors” and “gpu” child nodes in iddOptionTree rather than
replacing them with new trees. Preserve all unrelated existing fields while
updating only “monitors.count” and “gpu.friendlyname”, creating the child nodes
only when they do not already exist.
| try { | ||
| pt::read_xml(idd_option_path.string(), existing_root); | ||
| // 如果现有配置文件中已有vdd_settings节点 | ||
| if (existing_root.get_child_optional("vdd_settings")) { | ||
| // 复制现有配置 | ||
| iddOptionTree = existing_root.get_child("vdd_settings"); | ||
|
|
||
| // 更新需要更改的部分 | ||
| pt::ptree monitor_node; | ||
| monitor_node.put("count", 1); | ||
|
|
||
| pt::ptree gpu_node; | ||
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | ||
|
|
||
| // 替换配置 | ||
| iddOptionTree.put_child("monitors", monitor_node); | ||
| iddOptionTree.put_child("gpu", gpu_node); | ||
| iddOptionTree.put_child("global", global_node); | ||
| iddOptionTree.put_child("resolutions", resolutions_nodes); | ||
| } else { | ||
| // 如果没有vdd_settings节点，创建新的 | ||
| pt::ptree monitor_node; | ||
| monitor_node.put("count", 1); | ||
|
|
||
| pt::ptree gpu_node; | ||
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | ||
|
|
||
| iddOptionTree.add_child("monitors", monitor_node); | ||
| iddOptionTree.add_child("gpu", gpu_node); | ||
| iddOptionTree.add_child("global", global_node); | ||
| iddOptionTree.add_child("resolutions", resolutions_nodes); | ||
| } | ||
| } catch(std::exception &e) { | ||
| // 读取失败，创建新的配置 | ||
| } | ||
| catch (std::exception &e) { | ||
| BOOST_LOG(warning) << "读取现有VDD配置失败，创建新配置: " << e.what(); | ||
| } | ||
|
|
||
| pt::ptree monitor_node; | ||
| monitor_node.put("count", 1); | ||
|
|
||
| pt::ptree gpu_node; | ||
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | ||
| put_vdd_common_nodes(iddOptionTree, gpu_name); | ||
|
|
||
| iddOptionTree.add_child("monitors", monitor_node); | ||
| iddOptionTree.add_child("gpu", gpu_node); | ||
| iddOptionTree.add_child("global", global_node); | ||
| iddOptionTree.add_child("resolutions", resolutions_nodes); | ||
| } | ||
| iddOptionTree.put_child("global", global_node); | ||
| iddOptionTree.put_child("resolutions", resolutions_nodes); | ||
| iddOptionTree.erase("sunshine_mode_cache"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
XML 解析失败时不应继续写入——当前实现会用空树重建文件，静默丢弃其他配置。
第 1450-1452 行捕获异常后只记 warning，iddOptionTree 保持为空，随后第 1454-1458 行照常写入并在第 1460 行整体落盘。结果是 vdd_settings.xml 一旦损坏，本次保存会把它整个重建，丢掉所有本函数不负责的节点——例如 vdd_utils.cpp 第 334 行写入的 vdd_settings.cursor.HardwareCursor。
这个风险在本 PR 中被放大：clear_vdd_mode_cache 正是在 VDD 启动失败路径上调用本函数，而"启动失败"与"XML 损坏"高度相关，恰好是最容易踩中覆盖逻辑的场景。
建议解析失败时直接放弃保存，让上层看到失败：
🔧 建议修复
try {
pt::read_xml(idd_option_path.string(), existing_root);
if (existing_root.get_child_optional("vdd_settings")) {
iddOptionTree = existing_root.get_child("vdd_settings");
}
}
catch (std::exception &e) {
- BOOST_LOG(warning) << "读取现有VDD配置失败，创建新配置: " << e.what();
+ // 不能用空树重建：那会丢掉 cursor.HardwareCursor 等本函数不负责的节点。
+ BOOST_LOG(error) << "读取现有VDD配置失败，放弃本次保存以免覆盖其他设置: " << e.what();
+ return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| pt::read_xml(idd_option_path.string(), existing_root); | |
| // 如果现有配置文件中已有vdd_settings节点 | |
| if (existing_root.get_child_optional("vdd_settings")) { | |
| // 复制现有配置 | |
| iddOptionTree = existing_root.get_child("vdd_settings"); | |
| // 更新需要更改的部分 | |
| pt::ptree monitor_node; | |
| monitor_node.put("count", 1); | |
| pt::ptree gpu_node; | |
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | |
| // 替换配置 | |
| iddOptionTree.put_child("monitors", monitor_node); | |
| iddOptionTree.put_child("gpu", gpu_node); | |
| iddOptionTree.put_child("global", global_node); | |
| iddOptionTree.put_child("resolutions", resolutions_nodes); | |
| } else { | |
| // 如果没有vdd_settings节点，创建新的 | |
| pt::ptree monitor_node; | |
| monitor_node.put("count", 1); | |
| pt::ptree gpu_node; | |
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | |
| iddOptionTree.add_child("monitors", monitor_node); | |
| iddOptionTree.add_child("gpu", gpu_node); | |
| iddOptionTree.add_child("global", global_node); | |
| iddOptionTree.add_child("resolutions", resolutions_nodes); | |
| } | |
| } catch(std::exception &e) { | |
| // 读取失败，创建新的配置 | |
| } | |
| catch (std::exception &e) { | |
| BOOST_LOG(warning) << "读取现有VDD配置失败，创建新配置: " << e.what(); | |
| } | |
| pt::ptree monitor_node; | |
| monitor_node.put("count", 1); | |
| pt::ptree gpu_node; | |
| gpu_node.put("friendlyname", gpu_name.empty() ? "default" : gpu_name); | |
| put_vdd_common_nodes(iddOptionTree, gpu_name); | |
| iddOptionTree.add_child("monitors", monitor_node); | |
| iddOptionTree.add_child("gpu", gpu_node); | |
| iddOptionTree.add_child("global", global_node); | |
| iddOptionTree.add_child("resolutions", resolutions_nodes); | |
| } | |
| iddOptionTree.put_child("global", global_node); | |
| iddOptionTree.put_child("resolutions", resolutions_nodes); | |
| iddOptionTree.erase("sunshine_mode_cache"); | |
| try { | |
| pt::read_xml(idd_option_path.string(), existing_root); | |
| if (existing_root.get_child_optional("vdd_settings")) { | |
| iddOptionTree = existing_root.get_child("vdd_settings"); | |
| } | |
| } | |
| catch (std::exception &e) { | |
| // 不能用空树重建：那会丢掉 cursor.HardwareCursor 等本函数不负责的节点。 | |
| BOOST_LOG(error) << "读取现有VDD配置失败，放弃本次保存以免覆盖其他设置: " << e.what(); | |
| return false; | |
| } | |
| put_vdd_common_nodes(iddOptionTree, gpu_name); | |
| iddOptionTree.put_child("global", global_node); | |
| iddOptionTree.put_child("resolutions", resolutions_nodes); | |
| iddOptionTree.erase("sunshine_mode_cache"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/confighttp.cpp` around lines 1444 - 1458, 在读取现有配置的 try/catch 流程中，解析 XML
失败后不要继续执行 put_vdd_common_nodes、global/resolutions
更新及后续落盘；应通过该保存函数的现有失败传播机制立即返回失败，让上层感知错误并保留原文件内容。仅在 pt::read_xml 成功时继续使用
iddOptionTree 更新配置。
| if (vdd_settings.needs_cache_update && | ||
| !confighttp::saveVddModeSettings(vdd_settings.global_modes, vdd_settings.temporary_modes, config::video.adapter_name)) { | ||
| BOOST_LOG(warning) << "VDD模式XML缓存保存失败 [mode_count: " << vdd_settings.mode_combination_count << "]"; | ||
| } | ||
|
|
||
| verify_mode_publication_after_create = device_zako.empty(); | ||
| if (mode_update == vdd_mode_update_e::recreate_monitor) { | ||
| mode_rebuild_old_vdd_id = device_zako; | ||
| verify_mode_publication_after_create = true; | ||
| if (!vdd_utils::destroy_vdd_monitor()) { | ||
| BOOST_LOG(error) << "Failed to destroy the VDD monitor for an IOCTL mode-list rebuild"; | ||
| vdd_utils::clear_vdd_mode_cache(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
同一次调用中先写盘再清盘，且清理是全量清空而非回滚本次新增的模式。
第 671-674 行刚把 temporary_modes 持久化到 XML，第 682 行（以及第 709、739、762 行）在后续任一失败路径上立刻调用 clear_vdd_mode_cache() 把它清掉——一次会话里对 vdd_settings.xml 做了两次写入，第二次撤销第一次。
更关键的是 clear_vdd_mode_cache() 会清空全部临时模式，而不只是本次新增的那一条。也就是说一次启动失败会连带丢掉之前 4 个已经验证可用的历史模式，缓存复用的价值被大幅削弱——这与 PR 想达成的"驱动重载或重启后客户端重连时复用模式列表"目标相悖。
建议二选一：
- 把持久化推迟到
prepare_vdd全部成功之后（第 816 行return true之前），这样失败路径根本不需要清理； - 或让清理接口只回滚本次新增的模式，例如
clear_vdd_mode_cache(const vdd_mode_t &mode_to_drop)。
方案 1 更简洁，也顺带消除了重复写盘。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_device/session.cpp` around lines 671 - 682, 将 vdd_settings
临时模式的持久化从当前保存位置移到 prepare_vdd 流程全部成功、接近成功返回前的位置，确保失败路径不会调用
clear_vdd_mode_cache() 撤销已写入的历史模式；保留现有失败处理逻辑，并确保成功返回前只执行一次保存。
| for (const auto &refresh_rate : global_refresh_rates) { | ||
| for (const auto &resolution : resolutions) { | ||
| append_vdd_mode_latest(global_modes, resolution, refresh_rate); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
模式列表的"读回表示"与"生成表示"未对齐，导致 needs_cache_update 恒为 true、每次会话都重写 vdd_settings.xml。
共同根因：prepare_vdd_settings 用 vdd_modes_equal 对逐位置相等做比对，但从 XML 读回的列表和本次构建的列表在顺序与裁剪规则两个维度上都没有归一化到同一形式，因此内容未变时也会被判为"已变更"。
src/display_device/vdd_utils.cpp#L200-L204：把 global_modes 的生成循环改为分辨率外层、刷新率内层，与第 233-239 行append_configured_vdd_modes的顺序保持一致。src/display_device/vdd_utils.cpp#L136-L151：在trim_temporary_vdd_modes_to_limit中补上"分辨率∈global 分辨率集合 且 刷新率∈global 刷新率集合"的交集剔除，与src/confighttp.cpp第 1369-1373 行trim_vdd_temporary_modes的落盘规则对齐。
两处修好后，无变更的会话才会真正跳过写盘，第 673 行日志中的 mode_combination_count 也才会与实际落盘内容一致。
📍 Affects 1 file
src/display_device/vdd_utils.cpp#L200-L204(this comment)src/display_device/vdd_utils.cpp#L136-L151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_device/vdd_utils.cpp` around lines 200 - 204, Align VDD mode
readback and generation by updating append_vdd_mode_latest’s loop in
src/display_device/vdd_utils.cpp lines 200-204 to iterate resolutions outermost
and refresh rates innermost, matching append_configured_vdd_modes; also update
trim_temporary_vdd_modes_to_limit at src/display_device/vdd_utils.cpp lines
136-151 to remove only modes outside the intersection of global resolutions and
global refresh rates, matching trim_vdd_temporary_modes. Both sites require
changes so needs_cache_update and mode_combination_count reflect the persisted
content.
| for (const auto &mode : temporary_modes) { | ||
| if (const auto parsed_resolution = parse_vdd_resolution(mode.first)) { | ||
| append_unique_resolution(settings.resolution_modes, *parsed_resolution); | ||
| } | ||
| if (const auto parsed_refresh_hz = parse_vdd_refresh_hz(mode.second)) { | ||
| append_unique_refresh_rate(settings.refresh_rates_hz, *parsed_refresh_hz); | ||
| } | ||
| } | ||
|
|
||
| if (config.resolution) { | ||
| append_unique_resolution(resolution_modes, *config.resolution); | ||
| append_unique_resolution(settings.resolution_modes, *config.resolution); | ||
| } | ||
| if (config.refresh_rate) { | ||
| if (const auto session_refresh_hz = rounded_refresh_hz(*config.refresh_rate)) { | ||
| append_unique_refresh_rate(refresh_rates_hz, *session_refresh_hz); | ||
| append_unique_refresh_rate(settings.refresh_rates_hz, *session_refresh_hz); | ||
| } | ||
| } | ||
|
|
||
| return { std::move(resolution_modes), std::move(refresh_rates_hz) }; | ||
| settings.needs_cache_update = config.resolution && config.refresh_rate && | ||
| (!vdd_modes_equal(existing_global_modes, global_modes) || | ||
| !vdd_modes_equal(existing_temporary_modes, temporary_modes)); | ||
| settings.mode_combination_count = count_vdd_mode_combinations(global_modes) + temporary_modes.size(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
临时模式被拆成分辨率集与刷新率集后由 SETMODES 做笛卡尔积下发，实际模式数远超预算模型的估算。
第 938-945 行把每个临时精确模式的分辨率和刷新率分别 append_unique 进两个独立集合，而 set_vdd_session_mode（第 528-536 行）是对 resolutions × refresh_rates_hz 做双重循环生成命令的。
举例：global 为 2 分辨率 × 2 刷新率，再加入 5 个各自引入新分辨率和新刷新率的临时模式，SETMODES 实际下发 7 × 7 = 49 个组合，而第 959 行的 mode_combination_count 只报告 4 + 5 = 9。
问题在于 trim_temporary_vdd_modes_to_limit 使用的 vdd_max_temporary_mode_count 预算是按 confighttp.h 第 49 行注释所说的 "temporary exact modes" 语义标定的，但下发路径把它们放大成了笛卡尔积。临时模式越分散，越容易突破 VDD_MODE_COMBINATION_LIMITS 中实测的组合上限，从而触发 SETMODES 被驱动拒绝——这正是本 PR 想规避的失败场景。
建议二选一：
mode_combination_count改为按实际笛卡尔积计算（count_vdd_mode_combinations应用于合并后的完整集合），并让裁剪循环以该值对照vdd_max_mode_combination_count收敛；- 或在
set_vdd_session_mode中区分"全局笛卡尔积模式"与"临时精确模式"，不对后者做叉乘。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_device/vdd_utils.cpp` around lines 938 - 959, 将临时模式加入 settings
的分辨率和刷新率集合后，mode_combination_count 必须按合并后的完整集合计算实际笛卡尔积，而不是继续使用全局组合数加临时模式数量。更新
count_vdd_mode_combinations 的调用及 trim_temporary_vdd_modes_to_limit 的收敛判断，使裁剪依据
vdd_max_mode_combination_count，并与 set_vdd_session_mode 的实际下发数量一致。
| void | ||
| clear_vdd_mode_cache() { | ||
| std::vector<vdd_mode_t> global_modes; | ||
| std::vector<vdd_mode_t> temporary_modes; | ||
| read_vdd_settings_modes(global_modes, temporary_modes); | ||
| if (temporary_modes.empty()) { | ||
| return; | ||
| } | ||
|
|
||
| BOOST_LOG(info) << "VDD启动失败，清除临时模式缓存 [count: " << temporary_modes.size() << "]"; | ||
| if (!confighttp::saveVddModeSettings(global_modes, {}, config::video.adapter_name)) { | ||
| BOOST_LOG(warning) << "清除VDD临时模式缓存失败"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
global 为空时清理会静默失败，坏缓存无法被清除，与文档承诺相悖。
saveVddModeSettings 在 normalized_global_modes.empty() 时直接 return false（confighttp.cpp 第 1405-1407 行），不会写入任何内容。
因此当 vdd_settings.xml 中没有 g_refresh_rate 节点、或没有不带 refresh_rate 的 resolution 节点时（用户从未在 Web UI 配置过全局分辨率/刷新率，而 temporary 由会话累积——这是完全常见的状态），global_modes 为空，本函数只会打一条 warning，坏的临时模式缓存原封不动留在文件里。
这恰好违背了 vdd_utils.h 第 271-272 行的文档承诺："a bad cached mode list cannot keep breaking every subsequent VDD start"——下一次 VDD 启动会读到同样的坏缓存并再次失败，形成死循环。
建议在 global 为空时直接从 XML 中移除 resolutions 下带 refresh_rate 的节点，而不是走依赖 global 非空的保存路径：
🔧 建议修复方向
void
clear_vdd_mode_cache() {
std::vector<vdd_mode_t> global_modes;
std::vector<vdd_mode_t> temporary_modes;
read_vdd_settings_modes(global_modes, temporary_modes);
if (temporary_modes.empty()) {
return;
}
BOOST_LOG(info) << "VDD启动失败，清除临时模式缓存 [count: " << temporary_modes.size() << "]";
+ if (global_modes.empty()) {
+ // saveVddModeSettings 在 global 为空时会拒绝写入，
+ // 此处必须直接改写 XML，否则坏缓存永远无法清除。
+ clear_temporary_modes_in_xml(); // 需新增：直接删除带 refresh_rate 的 resolution 节点
+ return;
+ }
if (!confighttp::saveVddModeSettings(global_modes, {}, config::video.adapter_name)) {
BOOST_LOG(warning) << "清除VDD临时模式缓存失败";
}
}需要我直接提交一份完整实现吗？
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_device/vdd_utils.cpp` around lines 965 - 978, Update
clear_vdd_mode_cache so temporary modes are removed even when global_modes is
empty. When no global modes exist, directly remove the resolutions entries
carrying refresh_rate from the persisted VDD settings XML, rather than calling
saveVddModeSettings; preserve the existing save path for non-empty global_modes
and keep warning behavior for cleanup failures.
![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: de110cc29f
ℹ️ 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".
| if (auto refresh_rate = child.second.get_optional<std::string>("refresh_rate")) { | ||
| append_vdd_mode_latest(temporary_modes, resolution, *refresh_rate); | ||
| } |
There was a problem hiding this comment.
This treats every <resolution> node that already has a <refresh_rate> as a disposable temporary cache entry. The packaged VDD settings file already ships exact 30 Hz modes in this form (src_assets/windows/misc/vdd/driver/vdd_settings.xml lines 18-42), so a non-global session that causes trim/rewrite, or any startup failure that calls clear_vdd_mode_cache(), will silently remove those built-in modes from vdd_settings.xml. Please distinguish Sunshine-owned cache entries from existing driver/user exact modes, or preserve the non-cache entries when rewriting.
Useful? React with 👍 / 👎.
| for (const auto &mode : temporary_modes) { | ||
| if (const auto parsed_resolution = parse_vdd_resolution(mode.first)) { | ||
| append_unique_resolution(settings.resolution_modes, *parsed_resolution); | ||
| } | ||
| if (const auto parsed_refresh_hz = parse_vdd_refresh_hz(mode.second)) { | ||
| append_unique_refresh_rate(settings.refresh_rates_hz, *parsed_refresh_hz); | ||
| } |
There was a problem hiding this comment.
When cached temporary modes have their own resolution/refresh pairs, this adds each resolution and each refresh rate independently to the SETMODES dimensions. Since set_vdd_session_mode() later sends every resolution with every refresh rate, a few exact cached modes such as 2560x1440@144 and 3840x2160@120 are replayed as extra combinations that were never cached or requested; on profiles already near the VDD mode budget, this can make the driver reject SETMODES and clear the cache on the next startup. Please either preserve exact pairs through SETMODES or enforce the limit against the actual Cartesian list being sent.
Useful? React with 👍 / 👎.
| saveVddModeSettings(const std::vector<VddMode> &modes, std::string gpu_name) { | ||
| return saveVddModeSettings(modes, {}, std::move(gpu_name)); |
There was a problem hiding this comment.
The legacy saveVddSettings() path is still called by saveConfig() on every Web UI config save, including saves unrelated to VDD modes, but this overload now rewrites vdd_settings.xml with an explicitly empty temporary list. Because saveVddModeSettings() replaces the whole <resolutions> node, any existing session-mode cache is dropped as soon as the user saves settings. Please merge the existing temporary cache here, pruning only entries covered by the new global mode list.
Useful? React with 👍 / 👎.
Review：方向正确，建议处理完 P0 后合入先说肯定的部分：SETMODES 被驱动接受后才落盘（避免运行态/持久态分裂）和 以下针对模式缓存主体，均为静态审查结论，未做编译/运行验证。 P0 — 建议合入前处理1. 改动前 用户在 WebUI 点保存的同时客户端发起串流，两个 read-modify-write 交错 → 丢更新，甚至 XML 结构损坏。 写入本身也不原子（ std::ofstream file(idd_option_path.string()); // 立即截断
file << xml_content;截断和写完之间存在窗口，而这个文件正是 VDD 驱动自己要读的 —— 驱动恰好在此刻重载就会读到空文件或半截 XML。 同仓库的 #848 给 2.
auto normalized_global_modes = normalize_vdd_modes(global_modes);
if (normalized_global_modes.empty()) {
return false; // ← 全局模式为空就整个放弃
}而 这不是理论情况： 建议清理路径走一条能接受空 global 的专用写入，或把空 global 的语义从"失败"改为"写空 resolutions 节点"。 3. 缓存写入位置偏早，失败会话要写两次 XML 描述说"SETMODES 被驱动接受后才写 XML 缓存"，但 结果：一次失败的 VDD 拉起 = 写 XML → 立刻读 XML → 再写 XML。既让 P0-1 的并发窗口翻倍，也和"避免运行态/持久态不一致"的初衷相悖——中间那段时间里，XML 记录的正是一个最终没能用起来的模式。 把写入挪到全部 bring-up 校验通过之后（ P14. 依赖方向反了：
副作用：这些 VDD 专属常量现在裸露在跨平台的 5. 同一套逻辑写了两遍
两侧 trim 逻辑必须保持一致才能正确——准备侧算出的列表和写入侧裁剪出的列表若不一致， 6. 零测试 仓库有
描述里的"测试"是"Windows Debug 全量重编译 + 链接通过"——那是构建验证，不是测试。尤其 P1-5 的两份重复实现，正需要测试来锁住一致性。 7. 经验常数表：不单调、来源单一、二次查找是死代码
for (const auto &limit : VDD_MODE_COMBINATION_LIMITS) {
if (refresh_rate_count <= limit.refresh_rate_count) { ... }
}表里 P2 / Nit
方向对，自愈设计有想法，构建修复实打实。P0 三条建议合入前解决——尤其第 1 条，它影响驱动配置文件的完整性，损坏后表现为 VDD 完全拉不起来、排查成本很高，而修复只需一个 mutex 加一次 P1-4/5（分层 + 重复）和 P1-6（测试）若放到跟进 PR，建议在此留个 TODO 或开 issue，避免两份 trim 逻辑无声漂移。 |
* fix(build): unblock Debug links in tray-less and template-heavy TUs Two independent link failures, both only reachable outside the Release CI matrix, cherry-picked from #845 (the rest of that PR is superseded by the SETMODES architecture). main.cpp held the only three unguarded system_tray:: references in the tree. Since the Windows tray moved to the bundled GUI agent, SUNSHINE_ENABLE_TRAY=ON with SUNSHINE_ENABLE_LEGACY_TRAY=OFF (the default) yields SUNSHINE_TRAY=0, and system_tray.cpp is then an empty translation unit. Release links anyway because `constexpr bool tray_is_enabled = false` lets the optimizer drop the branches before the linker sees them; at -O0 it does not, so the default configuration fails to link. Guard the three sites to match every other call site. The in-process tray is still the live path on Linux (SUNSHINE_TRAY=1 whenever appindicator and libnotify are present), so these calls are kept rather than removed. The #else branch in mainThreadLoop blocks on shutdown_event instead of falling through: run_loop is only ever set by the tray today, but a fall-through would return into main() and tear the host down if that ever changes. confighttp.cpp exceeds the 32767-section COFF limit, where the assembler silently emits a corrupt symbol table and its COMDAT entries (typeinfo, inline members) resolve as undefined. -Wa,-mbig-obj fixes it; the flag is scoped to CXX, and the Windows toolchain is MinGW GCC only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(build): gate -Wa,-mbig-obj on GNU clang only warns about an unknown -Wno-*, but rejects an unknown -Wa, argument outright, so the flag would hard-fail a clang-based MinGW build (MSYS2 CLANG64) where the adjacent GCC-only warning flags survive. Gate it on the compiler ID. Uses a plain if() rather than a COMPILE_LANG_AND_ID generator expression: cmake/targets/common.cmake wraps every SUNSHINE_COMPILE_OPTIONS entry as --compiler-options=<flag> when CUDA_INHERIT_COMPILE_OPTIONS is on (the default), so a genex in that list would degrade to a bare --compiler-options= for CUDA translation units.
2 participants
概述
为 VDD 增加会话模式的 XML 持久化缓存，使会话使用过的分辨率/刷新率在驱动重载、重启后仍保留在 `vdd_settings.xml` 中，客户端重连时无需重建监视器模式列表。
变更内容
模式缓存（vdd模式缓存）
构建修复（独立提交）
测试
🤖 Generated with Claude Code