-
Notifications
You must be signed in to change notification settings - Fork 38
fix(print): set white background for print rendering #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
set white background for print rendering log: set white background for print rendering bug: https://pms.uniontech.com/bug-view-338245.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates print dialog invocation so that page rendering uses a white background when generating the preview/print image. Sequence diagram for updated print rendering with white backgroundsequenceDiagram
actor User
participant TopTilte
participant DrawApp
participant DrawBoard
participant Page
participant PageContext
participant CPrintManager
User->>TopTilte: clickPrintAction()
TopTilte->>DrawApp: drawBoard()
DrawApp-->>TopTilte: DrawBoard
TopTilte->>DrawBoard: currentPage()
DrawBoard-->>TopTilte: Page
TopTilte->>Page: context()
Page-->>TopTilte: PageContext
TopTilte->>PageContext: renderToImage(Qt_white)
PageContext-->>TopTilte: renderedImage
TopTilte->>CPrintManager: CPrintManager(topMainWindowWidget)
TopTilte->>CPrintManager: showPrintDialog(renderedImage, topMainWindowWidget, pageName)
CPrintManager-->>User: displayPrintDialogWithWhiteBackground
Updated class diagram for print rendering interactionclassDiagram
class TopTilte {
+initMenu() void
}
class DrawApp {
+topMainWindowWidget() QWidget
+drawBoard() DrawBoard
}
class DrawBoard {
+currentPage() Page
}
class Page {
+name() QString
+context() PageContext
}
class PageContext {
+renderToImage(backgroundColor QtColor) QImage
}
class CPrintManager {
+CPrintManager(parent QWidget)
+showPrintDialog(image QImage, parent QWidget, title QString) void
}
TopTilte --> DrawApp
DrawApp --> DrawBoard
DrawBoard --> Page
Page --> PageContext
TopTilte --> CPrintManager
TopTilte ..> PageContext : uses_renderToImage
TopTilte ..> CPrintManager : uses_showPrintDialog
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Forcing a white background in
renderToImage(Qt::white)may override documents that intentionally use a different page/background color; consider deriving the background from the page context or making the print background color configurable rather than hard-coded.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Forcing a white background in `renderToImage(Qt::white)` may override documents that intentionally use a different page/background color; consider deriving the background from the page context or making the print background color configurable rather than hard-coded.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码修改主要是在调用 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议
总结当前修改是合理的,解决了打印时的背景色问题,但可以通过避免硬编码和增加注释来提高代码的可维护性和可读性。如果背景色可能动态变化,建议进一步抽象为配置项。 |
|
Note
详情{
"src/frame/toptoolbar.cpp": [
{
"line": " dApp->setApplicationAcknowledgementPage(\"https://www.deepin.org/acknowledgments/deepin-draw/\");",
"line_number": 235,
"rule": "S35",
"reason": "Url link | a2d44da975"
}
]
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, 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 |
|
/merge |
set white background for print rendering
log: set white background for print rendering
bug: https://pms.uniontech.com/bug-view-338245.html
Summary by Sourcery
Bug Fixes: