Conversation
WalkthroughThe pull request adjusts text rendering calculations in the SDL2 backend and modifies the default font size configuration. The left-aligned text rectangle width now factors in string length multiplied by font size, and the height includes margin padding. The default font size is reduced from 35 to 12. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tinyui.cpp (1)
41-41: Replace the hardcoded default font size with a namedconstexpr.Line 41 uses a magic compile-time literal. Extract it to a named constant so default sizing is centralized and safer to tune.
Proposed diff
+static constexpr uint32_t kDefaultFontSize = 12; + static Style DefaultStyle { @@ - { "Arial.ttf", 12, nullptr } + { "Arial.ttf", kDefaultFontSize, nullptr } };As per coding guidelines, "Use
constexprfor compile-time constants andstatic constexprfor class-level constants".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tinyui.cpp` at line 41, The literal font size 12 is a magic number — define a compile-time constant (e.g. constexpr int DEFAULT_FONT_SIZE = 12; or if this is inside a class use static constexpr int DEFAULT_FONT_SIZE = 12;) and replace the numeric literal in the initializer { "Arial.ttf", 12, nullptr } with that named constant (DEFAULT_FONT_SIZE) so the default is centralized and editable via the symbol used by the initializer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backends/sdl2_renderer.cpp`:
- Around line 236-237: Replace the estimated width/height that use font->mSize
and strlen with the actual dimensions of the SDL surface returned by
TTF_RenderText_Solid(): set Message_rect.w = textSurface->w (or surface->w) and
Message_rect.h = textSurface->h (add margin if you still need padding) so
left-aligned bounds use the real rendered pixel size; locate the
TTF_RenderText_Solid() call and the Message_rect assignments to update them
accordingly.
---
Nitpick comments:
In `@src/tinyui.cpp`:
- Line 41: The literal font size 12 is a magic number — define a compile-time
constant (e.g. constexpr int DEFAULT_FONT_SIZE = 12; or if this is inside a
class use static constexpr int DEFAULT_FONT_SIZE = 12;) and replace the numeric
literal in the initializer { "Arial.ttf", 12, nullptr } with that named constant
(DEFAULT_FONT_SIZE) so the default is centralized and editable via the symbol
used by the initializer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a2650c5-23de-46fc-9d99-2229d45eff5a
📒 Files selected for processing (2)
src/backends/sdl2_renderer.cppsrc/tinyui.cpp
| Message_rect.w = font->mSize*static_cast<int>(strlen(string)); | ||
| Message_rect.h = font->mSize + margin*2; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current drawText sizing logic around SDL_ttf surface creation and rect sizing.
rg -n -C3 'TTF_RenderText_Solid|Message_rect\.w|Message_rect\.h|surfaceMessage->(w|h|clip_rect\.w|clip_rect\.h)' src/backends/sdl2_renderer.cppRepository: kimkulling/tiny_ui
Length of output: 1360
Use the actual rendered surface dimensions for left-aligned text bounds.
Lines 236–237 estimate text dimensions using font->mSize * strlen(string) and font->mSize + margin*2, which is inaccurate for proportional fonts and causes potential text clipping or misalignment. The rendered surface from TTF_RenderText_Solid() (line 216) is already available—use its actual dimensions instead.
Proposed diff
case Alignment::Left:
Message_rect.x = r.top.x + margin;
Message_rect.y = r.top.y + margin;
- Message_rect.w = font->mSize*static_cast<int>(strlen(string));
- Message_rect.h = font->mSize + margin*2;
+ Message_rect.w = surfaceMessage->w;
+ Message_rect.h = surfaceMessage->h;
break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backends/sdl2_renderer.cpp` around lines 236 - 237, Replace the estimated
width/height that use font->mSize and strlen with the actual dimensions of the
SDL surface returned by TTF_RenderText_Solid(): set Message_rect.w =
textSurface->w (or surface->w) and Message_rect.h = textSurface->h (add margin
if you still need padding) so left-aligned bounds use the real rendered pixel
size; locate the TTF_RenderText_Solid() call and the Message_rect assignments to
update them accordingly.
1 participant
Summary by CodeRabbit