fix: Resolve compilation issues for v20. #183
Merged
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 GuideThis PR fixes compilation issues on v20 by making the build system and several source files properly support both Qt5 and Qt6, removing outdated Qt5-only branches and centralizing version selection via CMake variables. 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 - here's some feedback:
- In CSizeHandleRect::paint you now call m_lightRenderer.render twice (once with boundingRect() and once with rect) regardless of Qt version, which is different from the previous Qt5 path and might cause redundant drawing or visual changes; consider whether one of these calls can be removed or guarded to preserve the original behavior.
- In the top-level CMakeLists.txt you first call find_package(Qt6 QUIET) and then, if found, call find_package(Qt6 REQUIRED COMPONENTS ...); the initial QUIET call is redundant and can be dropped to simplify the configuration logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In CSizeHandleRect::paint you now call m_lightRenderer.render twice (once with boundingRect() and once with rect) regardless of Qt version, which is different from the previous Qt5 path and might cause redundant drawing or visual changes; consider whether one of these calls can be removed or guarded to preserve the original behavior.
- In the top-level CMakeLists.txt you first call find_package(Qt6 QUIET) and then, if found, call find_package(Qt6 REQUIRED COMPONENTS ...); the initial QUIET call is redundant and can be dropped to simplify the configuration logic.
## Individual Comments
### Comment 1
<location> `src/drawshape/drawItems/csizehandlerect.cpp:89-95` </location>
<code_context>
- //在Qt6中，QGraphicsSvgItem的renderer()方法已经被移除。在Qt5中，renderer()方法是可用的，因此需要分别处理。
- #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
if (!m_isRotation) {
- if (renderer() != &m_lightRenderer) {
- setSharedRenderer(&m_lightRenderer);
- }
- }
- #else
- // Qt6 处理逻辑
- if (!m_isRotation) {
- //// 在Qt6中直接使用m_lightRenderer进行绘制
m_lightRenderer.render(painter, this->boundingRect());
}
- #endif
painter->setClipping(false);
QRectF rect = this->boundingRect();
-
- #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
- this->renderer()->render(painter, rect);
- #else
- // 在Qt6中使用QSvgRenderer直接绘制
m_lightRenderer.render(painter, rect);
- #endif
</code_context>
<issue_to_address>
**issue (performance):** Rendering the same SVG twice for non-rotation cases may be redundant or unintentional.
`m_lightRenderer.render` is invoked twice when `!m_isRotation`: first with `this->boundingRect()` and then with `rect`, which currently has the same value. Unless a double render is required for a visual effect (e.g., blending or multipass painting), consider keeping only a single call to reduce redundant work and simplify the logic.
</issue_to_address>
### Comment 2
<location> `src/frame/cmultiptabbarwidget.cpp:304-307` </location>
<code_context>
#include <DApplication>
#include <QScrollBar>
#include <QWindow>
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
#include <QtSvgWidgets/QGraphicsSvgItem>
+#else
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Different `split` behavior between Qt5 and Qt6 may introduce inconsistencies.
In Qt6 you use `suffixes.split(' ', Qt::SkipEmptyParts)`, but in Qt5 you call `suffixes.split(' ')` without the flag. Since Qt5 defaults to `KeepEmptyParts`, multiple spaces will create empty entries only in the Qt5 branch. If you need consistent behavior, update the Qt5 code to also use `Qt::SkipEmptyParts` (Qt5 supports this flag).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
解决主分支在v20上的编译问题。 v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383477.html
lichaofan2008
force-pushed
the
develop/snipe1
branch
from
c28affb to
17c913a
Compare
deepin pr auto review我来对这个diff进行审查，主要关注代码质量、性能和安全性方面的改进建议：
# 建议将Qt版本检测逻辑改为更明确的方式
find_package(Qt6 QUIET)
if(NOT Qt6_FOUND)
find_package(Qt5 REQUIRED)
set(QT_VERSION_MAJOR 5)
set(DTK_VERSION_MAJOR "")
else()
find_package(Qt6 REQUIRED COMPONENTS Core Widgets Gui DBus Xml Svg Test)
set(QT_VERSION_MAJOR 6)
set(DTK_VERSION_MAJOR 6)
endif()
# 添加版本检查
if(QT_VERSION_MAJOR EQUAL 5 AND CMAKE_CXX_STANDARD LESS 17)
message(WARNING "Qt5 requires C++17 standard")
endif()
// 建议将正则表达式提取为类成员变量，避免重复编译
class Application {
private:
static const QRegularExpression splitExp; // 在cpp文件中初始化
// 添加错误码枚举
enum FileNameError {
NoError = 0,
PathNotFound = 2
};
};
// 建议使用统一的头文件包含方式
#ifdef QT_VERSION_MAJOR
#if QT_VERSION_MAJOR >= 6
#include <QtSvgWidgets/QGraphicsSvgItem>
#else
#include <QtSvg/QGraphicsSvgItem>
#endif
#else
#error "Qt version not defined"
#endif
// 建议使用RAII管理painter状态
class ScopedPainterState {
public:
explicit ScopedPainterState(QPainter* painter) : m_painter(painter) {
m_painter->save();
}
~ScopedPainterState() {
m_painter->restore();
}
private:
QPainter* m_painter;
};
void CSizeHandleRect::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) {
ScopedPainterState state(painter);
// ... 绘制代码 ...
}
// 建议使用现代C++的lambda表达式
connect(fontComboBox, QOverload<int>::of(&QComboBox::currentIndexChanged),
this, [this](int index) {
const QString family = fontComboBox->itemText(index);
handleFontFamilyChange(family);
});
// 建议创建版本兼容性工具类
class QtCompat {
public:
static QStringList splitSkipEmpty(const QString &str, const QString &sep) {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
return str.split(sep, Qt::SkipEmptyParts);
#else
return str.split(sep, QString::SplitBehavior::SkipEmptyParts);
#endif
}
};这些改进建议主要围绕：
建议在实施这些改进时，优先处理安全性和性能相关的问题，然后再逐步改进代码质量和可维护性。 |
lzwind
approved these changes
Dec 2, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, lzwind 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 |
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.
解决主分支在v20上的编译问题。
v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383477.html
Summary by Sourcery
Make the project build against both Qt5 and Qt6 while resolving v20 compilation issues.
Bug Fixes:
Enhancements:
Build: