fix: 修复 setNewResolutionList 排序不一致导致分辨率索引错位#477
Merged
deepin-bot[bot] merged 1 commit intoMay 28, 2026
Conversation
setNewResolutionList 排序条件使用了 <=，相同宽度时无条件交换， 导致排序结果与 settingDialog 不一致。摄像头重连后保存的索引值 在重新打开设置界面时映射到了错误的分辨率（如 640x480 显示为 640x360）。 Log: 修复 setNewResolutionList 排序不一致导致分辨率索引错位 Bug: https://pms.uniontech.com/bug-view-363077.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefines the sorting logic in Settings::setNewResolutionList so that resolutions are ordered deterministically by width then height, preventing index misalignment between stored resolution indices and the settings dialog. Flow diagram for updated resolution comparison in Settings::setNewResolutionListflowchart TD
A[resolutiontemp1 width < resolutiontemp2 width?] -->|yes| B[Swap resolutions]
A -->|no| C[resolutiontemp1 width == resolutiontemp2 width?]
C -->|no| D[Keep order]
C -->|yes| E[resolutiontemp1 height < resolutiontemp2 height?]
E -->|yes| B[Swap resolutions]
E -->|no| D[Keep order]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The comparison condition inside
setNewResolutionListis now quite long and repetitive; consider extracting the width/height parsing into localint width1, height1, width2, height2variables (or a small helper) to improve readability and avoid multipletoInt()calls on the same values. - Given this is performing a sort, it might be clearer and less error-prone to replace the manual swap-and-loop logic with
std::sort(orstd::stable_sort) using a well-defined comparator that orders by width first, then height.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The comparison condition inside `setNewResolutionList` is now quite long and repetitive; consider extracting the width/height parsing into local `int width1, height1, width2, height2` variables (or a small helper) to improve readability and avoid multiple `toInt()` calls on the same values.
- Given this is performing a sort, it might be clearer and less error-prone to replace the manual swap-and-loop logic with `std::sort` (or `std::stable_sort`) using a well-defined comparator that orders by width first, then height.
## Individual Comments
### Comment 1
<location path="src/src/Settings.cpp" line_range="139-140" />
<code_context>
- if ((resolutiontemp1[0].toInt() <= resolutiontemp2[0].toInt())
- || (resolutiontemp1[1].toInt() < resolutiontemp2[1].toInt())) {
+ if ((resolutiontemp1[0].toInt() < resolutiontemp2[0].toInt()) ||
+ (resolutiontemp1[0].toInt() == resolutiontemp2[0].toInt() && (resolutiontemp1[1].toInt() < resolutiontemp2[1].toInt()))) {
QString resolutionstr = resolutionDatabase[j + 1];
resolutionDatabase[j + 1] = resolutionDatabase[j];
</code_context>
<issue_to_address>
**suggestion:** Avoid repeated toInt() calls and simplify the comparison expression.
Extract the int values before the condition and compare those instead:
```cpp
int w1 = resolutiontemp1[0].toInt();
int h1 = resolutiontemp1[1].toInt();
int w2 = resolutiontemp2[0].toInt();
int h2 = resolutiontemp2[1].toInt();
if (w1 < w2 || (w1 == w2 && h1 < h2)) {
...
}
```
This avoids repeated conversions inside the loop and makes the sort logic easier to follow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+139
to
+140
| if ((resolutiontemp1[0].toInt() < resolutiontemp2[0].toInt()) || | ||
| (resolutiontemp1[0].toInt() == resolutiontemp2[0].toInt() && (resolutiontemp1[1].toInt() < resolutiontemp2[1].toInt()))) { |
There was a problem hiding this comment.
suggestion: Avoid repeated toInt() calls and simplify the comparison expression.
Extract the int values before the condition and compare those instead:
int w1 = resolutiontemp1[0].toInt();
int h1 = resolutiontemp1[1].toInt();
int w2 = resolutiontemp2[0].toInt();
int h2 = resolutiontemp2[1].toInt();
if (w1 < w2 || (w1 == w2 && h1 < h2)) {
...
}This avoids repeated conversions inside the loop and makes the sort logic easier to follow.
deepin pr auto review你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这次修改的主要目的是修复分辨率排序的逻辑，将原本的“宽度小于等于或高度小于”修改为标准的字典序比较（即“宽度小于或宽度相等且高度小于”），这修复了原有的逻辑错误。 不过，这段代码在语法逻辑、代码质量、代码性能和代码安全方面仍有较大的改进空间。以下是详细的审查意见和改进建议： 1. 语法与逻辑
2. 代码质量
3. 代码安全
4. 代码性能
改进建议与重构代码综合以上分析，我建议对这段代码进行彻底重构。最安全、最高效的做法是：先将字符串解析为安全的结构体并缓存，使用 以下是改进后的代码示例： #include <algorithm> // for std::sort
#include <vector>
#include <QSize> // Qt中用于表示尺寸的类，非常适合此场景
void Settings::setNewResolutionList()
{
// 假设 resolutionDatabase 是 QStringList 或 QList<QString>
// 1. 预处理：安全解析并缓存，避免重复 split 和 toInt
struct ResolutionItem {
QSize size; // 使用QSize存储宽高，语义清晰
QString originalStr; // 保留原始字符串，防止解析丢失信息
};
std::vector<ResolutionItem> items;
items.reserve(resolutionDatabase.size());
for (const QString& str : resolutionDatabase) {
QStringList parts = str.split("x");
bool widthOk = false, heightOk = false;
// 安全检查：确保分割后有两个部分，并且都能正确转换为数字
if (parts.size() == 2) {
int width = parts[0].toInt(&widthOk);
int height = parts[1].toInt(&heightOk);
if (widthOk && heightOk) {
items.push_back({QSize(width, height), str});
continue; // 解析成功，处理下一个
}
}
// 解析失败的异常数据，可以选择记录警告日志，这里依然保留原始字符串并赋予默认值以防丢失
qWarning() << "Invalid resolution format:" << str;
items.push_back({QSize(0, 0), str});
}
// 2. 使用 std::sort 进行高效排序，逻辑清晰且无越界风险
std::sort(items.begin(), items.end(), [](const ResolutionItem& a, const ResolutionItem& b) {
if (a.size.width() != b.size.width()) {
return a.size.width() < b.size.width(); // 宽度升序
}
return a.size.height() < b.size.height(); // 宽度相同，按高度升序
});
// 3. 将排序后的结果写回原列表
resolutionDatabase.clear();
for (const ResolutionItem& item : items) {
resolutionDatabase.append(item.originalStr);
}
}重构代码的优势：
|
max-lvs
approved these changes
May 28, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, Resurgamz The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
setNewResolutionList 排序条件使用了 <=，相同宽度时无条件交换，
导致排序结果与 settingDialog 不一致。摄像头重连后保存的索引值
在重新打开设置界面时映射到了错误的分辨率（如 640x480 显示为 640x360）。
Log: 修复 setNewResolutionList 排序不一致导致分辨率索引错位
Bug: https://pms.uniontech.com/bug-view-363077.html
Summary by Sourcery
Bug Fixes: