fix: wrap local t2i headers and inline code#6079
fix: wrap local t2i headers and inline code#6079stablegenius49 wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:L
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the text-to-image rendering capabilities by addressing text overflow issues for headers and inline code. By applying intelligent text wrapping, the visual presentation of generated images is significantly improved, preventing content from extending beyond the defined image width. This ensures a more polished and readable output for text-based elements. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
dosubot
bot
added
the
area:core
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我在这里给出了一些高层次的反馈：
InlineCodeElement的垂直间距和内边距值（例如_PADDING、_LINE_HEIGHT_EXTRA，以及那些硬编码的+ 4偏移）有点随意；可以考虑把它们集中到一些命名清晰的常量或辅助方法中，这样布局计算会更容易理解，也更容易保持一致。InlineCodeElement.calculate_height和InlineCodeElement.render都在手动重新计算line_height和行数；可以提取一个共享的辅助方法（例如_line_height(font_size)或_layout_lines(...)），以减少重复代码，也降低这两条逻辑路径出现偏差的风险。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The vertical spacing and padding values for `InlineCodeElement` (e.g., `_PADDING`, `_LINE_HEIGHT_EXTRA`, and the hardcoded `+ 4` offsets) are a bit ad hoc; consider centralizing them into clearly named constants or helper methods so the layout math is easier to understand and keep consistent.
- Both `InlineCodeElement.calculate_height` and `InlineCodeElement.render` manually recompute `line_height` and line counts; extracting a shared helper (e.g., `_line_height(font_size)` or `_layout_lines(...)`) would reduce duplication and the risk of these two paths drifting apart.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进之后的 Review。
Original comment in English
Hey - I've left some high level feedback:
- The vertical spacing and padding values for
InlineCodeElement(e.g.,_PADDING,_LINE_HEIGHT_EXTRA, and the hardcoded+ 4offsets) are a bit ad hoc; consider centralizing them into clearly named constants or helper methods so the layout math is easier to understand and keep consistent. - Both
InlineCodeElement.calculate_heightandInlineCodeElement.rendermanually recomputeline_heightand line counts; extracting a shared helper (e.g.,_line_height(font_size)or_layout_lines(...)) would reduce duplication and the risk of these two paths drifting apart.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The vertical spacing and padding values for `InlineCodeElement` (e.g., `_PADDING`, `_LINE_HEIGHT_EXTRA`, and the hardcoded `+ 4` offsets) are a bit ad hoc; consider centralizing them into clearly named constants or helper methods so the layout math is easier to understand and keep consistent.
- Both `InlineCodeElement.calculate_height` and `InlineCodeElement.render` manually recompute `line_height` and line counts; extracting a shared helper (e.g., `_line_height(font_size)` or `_layout_lines(...)`) would reduce duplication and the risk of these two paths drifting apart.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of long headers and inline code overflowing the image boundaries by implementing text wrapping. The changes are well-implemented, particularly in InlineCodeElement where a helper method and constants improve clarity. The addition of focused unit tests for the new wrapping logic is a great practice. I've identified a minor issue regarding the handling of empty inline code elements, which could result in incorrect height calculation and rendering. My specific comments provide suggestions to address this.
| return font_size + 16 # 包含内边距和上下间距 | ||
| font = FontManager.get_font(font_size) | ||
| lines = self._wrapped_lines(image_width, font) | ||
| return max(len(lines), 1) * (font_size + self._LINE_HEIGHT_EXTRA) |
There was a problem hiding this comment.
When self.content is empty, _wrapped_lines returns an empty list, making len(lines) zero. However, max(len(lines), 1) forces the result to be 1, causing the method to calculate the height for a single line of an empty element. This allocates unnecessary space. Removing max(..., 1) will correctly calculate a height of 0 for empty inline code. If self.content is not empty, _wrapped_lines will return a list with at least one element, so len(lines) will be sufficient.
| return max(len(lines), 1) * (font_size + self._LINE_HEIGHT_EXTRA) | |
| return len(lines) * (font_size + self._LINE_HEIGHT_EXTRA) |
| ) | ||
|
|
||
| return y + text_height + 16 # 返回新的y坐标 | ||
| return y + max(len(lines), 1) * line_height # 返回新的y坐标 |
There was a problem hiding this comment.
Similar to the issue in calculate_height, using max(len(lines), 1) will cause the y coordinate to advance even for an empty inline code element, which is inconsistent since nothing is rendered. By using len(lines) directly, the y coordinate will not change if there are no lines to render, which is the correct behavior.
| return y + max(len(lines), 1) * line_height # 返回新的y坐标 | |
| return y + len(lines) * line_height # 返回新的y坐标 |
2 participants
Summary
calculate_height()Testing
PYTHONPATH=. pytest -q tests/unit/test_local_t2i_strategy.pypython3 -m py_compile astrbot/core/utils/t2i/local_strategy.py tests/unit/test_local_t2i_strategy.pyCloses #5998.
Summary by Sourcery
包装本地 T2I 标题和行内代码，以适配图片宽度并改进渲染文本的布局。
Bug Fixes:
Tests:
Original summary in English
Summary by Sourcery
Wrap local T2I headers and inline code to respect image width and improve layout of rendered text.
Bug Fixes:
Tests: