chore: Re-implement the calculation method of exclusion zone #1227
Conversation
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
Reviewer's GuideRework onExclusionZoneChanged to calculate exclusion zones based on physical screen coordinates and scale factors for multi-screen setups, and add documentation outlining the algorithm and usage. Class diagram for LayerShellEmulation exclusion zone calculationclassDiagram
class LayerShellEmulation {
+onExclusionZoneChanged()
-m_dlayerShellWindow: DLayerShellWindow
-m_window: QWindow
}
class DLayerShellWindow {
+anchors()
+exclusionZone()
}
class QScreen {
+geometry()
}
class QWindow {
+screen()
+geometry()
+height()
+width()
+winId()
}
LayerShellEmulation --> DLayerShellWindow
LayerShellEmulation --> QWindow
QWindow --> QScreen
Flow diagram for exclusion zone calculation logicflowchart TD
A[Start: onExclusionZoneChanged] --> B{Which anchor?}
B -->|AnchorLeft| C[Calculate left exclusion zone: screen.x + exclusionZone * scaleFactor]
B -->|AnchorRight| D[Find desktop right boundary]
D --> E[Calculate current screen right boundary]
E --> F[Calculate distance to desktop right]
F --> G[Calculate right exclusion zone: distance + exclusionZone * scaleFactor]
B -->|AnchorTop| H[Calculate top exclusion zone: screen.y + exclusionZone * scaleFactor]
B -->|AnchorBottom| I{Find below screens with horizontal overlap}
I -->|Found| J[Sum below screens height]
J --> K[Calculate bottom exclusion zone: belowScreensHeight * scaleFactor + exclusionZone * scaleFactor]
I -->|Not found| L[Find desktop bottom boundary]
L --> M[Calculate distance to desktop bottom]
M --> N[Calculate bottom exclusion zone: distance + exclusionZone * scaleFactor]
C --> O[Update strut_partial]
G --> O
H --> O
K --> O
N --> O
O --> P[End]
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `frame/layershell/x11dlayershellemulation.cpp:266` </location>
<code_context>
+ << "result=" << strut_partial.bottom;
+
+ strut_partial.bottom_start_x = static_cast<uint32_t>(m_window->geometry().x());
+ strut_partial.bottom_end_x = static_cast<uint32_t>(m_window->geometry().x() + m_window->geometry().width() * scaleFactor);
}
</code_context>
<issue_to_address>
Potential for overflow or precision loss when casting to uint32_t.
Use a safe rounding method and validate for overflow before casting to uint32_t to prevent data loss.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
strut_partial.bottom_start_x = static_cast<uint32_t>(m_window->geometry().x());
strut_partial.bottom_end_x = static_cast<uint32_t>(m_window->geometry().x() + m_window->geometry().width() * scaleFactor);
=======
{
double start_x = m_window->geometry().x();
double end_x = m_window->geometry().x() + m_window->geometry().width() * scaleFactor;
auto safe_cast = [](double value) -> uint32_t {
if (value < 0) {
qWarning() << "Value underflow in bottom_start_x/bottom_end_x:" << value << ", clamping to 0";
return 0;
}
if (value > static_cast<double>(UINT32_MAX)) {
qWarning() << "Value overflow in bottom_start_x/bottom_end_x:" << value << ", clamping to UINT32_MAX";
return UINT32_MAX;
}
return static_cast<uint32_t>(std::round(value));
};
strut_partial.bottom_start_x = safe_cast(start_x);
strut_partial.bottom_end_x = safe_cast(end_x);
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+265
to
+266
| strut_partial.bottom_start_x = static_cast<uint32_t>(m_window->geometry().x()); | ||
| strut_partial.bottom_end_x = static_cast<uint32_t>(m_window->geometry().x() + m_window->geometry().width() * scaleFactor); |
There was a problem hiding this comment.
suggestion (bug_risk): Potential for overflow or precision loss when casting to uint32_t.
Use a safe rounding method and validate for overflow before casting to uint32_t to prevent data loss.
Suggested change
| strut_partial.bottom_start_x = static_cast<uint32_t>(m_window->geometry().x()); | |
| strut_partial.bottom_end_x = static_cast<uint32_t>(m_window->geometry().x() + m_window->geometry().width() * scaleFactor); | |
| { | |
| double start_x = m_window->geometry().x(); | |
| double end_x = m_window->geometry().x() + m_window->geometry().width() * scaleFactor; | |
| auto safe_cast = [](double value) -> uint32_t { | |
| if (value < 0) { | |
| qWarning() << "Value underflow in bottom_start_x/bottom_end_x:" << value << ", clamping to 0"; | |
| return 0; | |
| } | |
| if (value > static_cast<double>(UINT32_MAX)) { | |
| qWarning() << "Value overflow in bottom_start_x/bottom_end_x:" << value << ", clamping to UINT32_MAX"; | |
| return UINT32_MAX; | |
| } | |
| return static_cast<uint32_t>(std::round(value)); | |
| }; | |
| strut_partial.bottom_start_x = safe_cast(start_x); | |
| strut_partial.bottom_end_x = safe_cast(end_x); | |
| } |
Consider multiple screen combination styles Log: as title Pms: TASK-380785
yixinshark
force-pushed
the
fix-allExclusionZoneErrors
branch
from
93a567a to
07b5e46
Compare
deepin pr auto review代码审查报告：DDE-Shell 独占区域计算总体评价代码实现了复杂的屏幕布局和独占区域计算逻辑，考虑了多种屏幕配置和缩放比例。整体设计合理，但存在一些可以改进的地方。 详细分析1. 代码逻辑优点：
问题：
2. 代码质量优点：
问题：
3. 性能分析优点：
问题：
4. 安全性优点：
问题：
改进建议1. 代码逻辑优化// 在AnchorBottom处理中，可以预先计算当前屏幕的物理边界
const int currentScreenBottomPhysical = currentScreen->geometry().y() +
static_cast<int>(currentScreen->geometry().height() * scaleFactor);
const QRect currentRect = currentScreen->geometry();
// 添加输入验证
if (scaleFactor <= 0 || m_dlayerShellWindow->exclusionZone() < 0) {
qCWarning(layershell) << "Invalid scaleFactor or exclusionZone";
return;
}2. 性能优化// 可以预先计算并缓存常用的值
struct ScreenBounds {
int leftPhysical;
int rightPhysical;
int bottomPhysical;
};
// 在循环外预先计算
const auto currentScreenRightPhysical = currentRect.x() +
static_cast<int>(currentRect.width() * scaleFactor);3. 安全性增强// 添加数值范围检查
if (m_dlayerShellWindow->exclusionZone() > MAX_EXCLUSION_ZONE) {
qCWarning(layershell) << "Exclusion zone too large";
return;
}
// 使用安全的乘法运算，防止溢出
auto safeMultiply = [](int a, double b) {
if (a > INT_MAX / b || a < INT_MIN / b) {
qCWarning(layershell) << "Potential integer overflow";
return INT_MAX;
}
return static_cast<int>(a * b);
};4. 代码结构优化// 将每个锚定类型的处理提取为单独的函数
void calculateLeftAnchoredStrut(...);
void calculateRightAnchoredStrut(...);
void calculateTopAnchoredStrut(...);
void calculateBottomAnchoredStrut(...);
// 主函数简化为：
switch (anchors) {
case DLayerShellWindow::AnchorLeft:
case (DLayerShellWindow::AnchorLeft | DLayerShellWindow::AnchorTop | DLayerShellWindow::AnchorBottom):
calculateLeftAnchoredStrut(...);
break;
// ... 其他情况
}总结代码整体实现良好，但仍有改进空间。主要改进方向包括：
这些改进将使代码更加健壮、高效和易于维护。 |
18202781743
approved these changes
Aug 21, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, yixinshark 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 |
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.
Consider multiple screen combination styles
Log: as title
Pms: TASK-380785
Summary by Sourcery
Reimplement the exclusion zone calculation to support multi-screen layouts and proper DPI scaling for all panel anchor positions, replacing the previous single-screen logic and adding comprehensive documentation.
New Features:
Enhancements:
Documentation: