Skip to content

Scale classic UI XCB theme metrics with DPI - #1620

Open
1024th wants to merge 1 commit into
fcitx:masterfrom
1024th:fix-xcb-hidpi
Open

Scale classic UI XCB theme metrics with DPI#1620
1024th wants to merge 1 commit into
fcitx:masterfrom
1024th:fix-xcb-hidpi

Conversation

@1024th

@1024th 1024th commented Jul 31, 2026

Copy link
Copy Markdown

This improves HiDPI support in the classic UI XCB path.

Theme margins, overlays, action images, input window layout metrics, menu spacing, menu icons, separators, click regions, blur regions, and masks are now scaled with the current DPI.

This keeps the input window and XCB menu visually consistent on high-DPI displays instead of mixing scaled text with unscaled theme metrics.

Comparison on my KDE Wayland 175% scale setting, with Xft.dpi: 168 for Xwayland:
Wayland:
图片

Xwayland, before this fix:
图片

Xwayland, after this fix:
图片

Summary by CodeRabbit

  • New Features
    • Added DPI-aware scaling for classic input windows and menus.
    • Adjusted dimensions, spacing, margins, navigation controls, highlights, separators, and themed assets for high-DPI displays.
    • Improved rendering and interaction regions to remain consistent across display resolutions.
    • Updated shadows, blur effects, check marks, submenu indicators, and background elements to scale smoothly.
    • Preserved accurate pixel alignment and sizing across supported display densities.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 339fefa9-2ab7-4523-89fb-d4f4626bdb78

📥 Commits

Reviewing files that changed from the base of the PR and between 6f40661 and bc071e2.

📒 Files selected for processing (9)
  • src/ui/classic/common.h
  • src/ui/classic/inputwindow.cpp
  • src/ui/classic/inputwindow.h
  • src/ui/classic/theme.cpp
  • src/ui/classic/theme.h
  • src/ui/classic/xcbinputwindow.cpp
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbmenu.h
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/ui/classic/common.h
  • src/ui/classic/xcbmenu.h
  • src/ui/classic/inputwindow.cpp
  • src/ui/classic/inputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/theme.h
  • src/ui/classic/theme.cpp

📝 Walkthrough

Walkthrough

This PR adds DPI-based metric scaling to the classic UI. It introduces metricScaleForDPI, scale-aware Theme::paint and Theme::mask APIs, and metricScale() methods for input windows and menus. Layout, painting, navigation controls, blur regions, and menu elements use scaled metrics.

Changes

DPI Metric Scaling

Layer / File(s) Summary
Scale helper and Theme paint/mask contract
src/ui/classic/common.h, src/ui/classic/theme.h, src/ui/classic/theme.cpp
Adds metric scaling helpers and targetScale parameters. Theme tiles, overlays, action images, masks, and rectangle shrinking use scaled and rounded metrics.
InputWindow scaled layout and painting
src/ui/classic/inputwindow.h, src/ui/classic/inputwindow.cpp
Adds the default metricScale() method. Candidate regions, navigation controls, background painting, and Yoga layout use scaled theme dimensions.
XCBInputWindow DPI scale override
src/ui/classic/xcbinputwindow.h, src/ui/classic/xcbinputwindow.cpp
Derives the scale from DPI. Shadow margins and blur-region masks use the scale. Blur regions refresh when the window size or DPI scale changes.
XCBMenu DPI-scaled rendering
src/ui/classic/xcbmenu.h, src/ui/classic/xcbmenu.cpp
Derives the scale from DPI. Menu dimensions, spacing, separators, assets, positions, and theme rendering use scaled metrics.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant XCBInputWindow
  participant InputWindow
  participant Theme

  XCBInputWindow->>XCBInputWindow: derive metricScale from DPI
  XCBInputWindow->>InputWindow: apply metricScale to layout and painting
  InputWindow->>Theme: paint(config, targetScale)
  Theme->>Theme: scale margins, tiles, and overlays
  Theme-->>InputWindow: render scaled output
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the DPI scaling changes for the classic UI XCB theme metrics.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/classic/inputwindow.cpp`:
- Around line 677-680: The Yoga layout calculation for button width combines the
individual action dimensions without rounding, while the paint path rounds each
action's width individually using roundedScaledMetric. At fractional scales,
this divergence causes the next button to extend beyond buttonNode_. Ensure that
when summing the individual widths (prevWidth and nextWidth, obtained from
roundedScaledMetric) in the Yoga path, the combined result is also rounded so it
matches the total width painted by the individual rounded action widths.

In `@src/ui/classic/theme.cpp`:
- Around line 787-845: The overlay clipping and containment calculations around
clipRect and rect currently truncate scaled fractional values through
fcitx::Rect::setPosition/setSize. Keep overlayClipMarginLeft, clipWidth,
overlayWidth, overlayHeight, x, and y in floating-point geometry for
intersection and contains checks, converting to integer Rect coordinates only
for the final rendering operation.
- Around line 334-342: Update the auto-size assignments in the height/width
calculation to round the scaled metric plus margins using the existing
roundedScaledMetric or equivalent std::lround conversion, rather than implicitly
truncating the double result when assigning to int. Preserve the current
fallback conditions and dimension components.

In `@src/ui/classic/xcbinputwindow.cpp`:
- Line 171: Update XCBInputWindow::calculatePositionX and calculatePositionY to
scale each shadowMargin value through roundedScaledMetric using metricScale()
before subtracting it, covering left, right, top, and bottom margins while
preserving the existing positioning logic.
- Around line 211-213: Update the mask/blur-region refresh logic around
parent_->theme().mask() to also track the applied metricScale() (or equivalent
DPI value). Rebuild and reapply the blur property whenever that scale changes,
even when width and height remain unchanged, while preserving the existing
size-change behavior.

In `@src/ui/classic/xcbmenu.cpp`:
- Around line 505-517: In the item.region_ setSize call, replace the second
vertical margin contribution with highlightMargin.marginBottom instead of
repeating highlightMargin.marginTop, while leaving the width and top-margin
calculation unchanged.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a2773dc-d548-48c7-b46f-1b837bc93a42

📥 Commits

Reviewing files that changed from the base of the PR and between 98058cf and 6f40661.

📒 Files selected for processing (9)
  • src/ui/classic/common.h
  • src/ui/classic/inputwindow.cpp
  • src/ui/classic/inputwindow.h
  • src/ui/classic/theme.cpp
  • src/ui/classic/theme.h
  • src/ui/classic/xcbinputwindow.cpp
  • src/ui/classic/xcbinputwindow.h
  • src/ui/classic/xcbmenu.cpp
  • src/ui/classic/xcbmenu.h

Comment thread src/ui/classic/inputwindow.cpp
Comment thread src/ui/classic/theme.cpp
Comment thread src/ui/classic/theme.cpp
Comment thread src/ui/classic/xcbinputwindow.cpp
Comment thread src/ui/classic/xcbinputwindow.cpp Outdated
Comment thread src/ui/classic/xcbmenu.cpp Outdated
@1024th

1024th commented Jul 31, 2026

Copy link
Copy Markdown
Author

Updated the PR to address the review comments:

  • Made the Yoga prev/next button width match the painted rounded widths:
    YGNodeStyleSetWidth(buttonNode_.get(),
    roundedScaledMetric(prev.width(), scale) +
    roundedScaledMetric(next.width(), scale));
  • Rounded natural-size tile dimensions instead of truncating scaled values:
    if (height < 0) {
    height = static_cast<int>(
    std::lround(scaledMetric(resizeHeight, targetScale) +
    targetMarginTop + targetMarginBottom));
    }
    if (width < 0) {
    width = static_cast<int>(
    std::lround(scaledMetric(resizeWidth, targetScale) +
    targetMarginLeft + targetMarginRight));
  • Scaled shadow margins used for XCB input window positioning:
    const auto scale = metricScale();
    int leftSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginLeft.value(), scale);
    int rightSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginRight.value(), scale);
    int actualWidth = width() - leftSW - rightSW;
    actualWidth = actualWidth <= 0 ? width() : actualWidth;
    if (closestScreen != nullptr) {
    int newX = std::max(x, closestScreen->left());
    if (newX + actualWidth > closestScreen->right()) {
    newX = closestScreen->right() - actualWidth;
    }
    x = std::max(newX, closestScreen->left());
    }
    // exclude shadow border width
    x -= leftSW;
    return x;
    }
    int XCBInputWindow::calculatePositionY(const Rect &cursorRect,
    const Rect *closestScreen) const {
    // TODO: RTL support.
    int y = cursorRect.top();
    auto &theme = parent_->theme();
    const auto scale = metricScale();
    int topSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginTop.value(), scale);
    int bottomSW = roundedScaledMetric(
    theme.inputPanel->shadowMargin->marginBottom.value(), scale);
  • Rebuilt the blur region when the DPI scale changes, not only when the window size changes:
    void XCBInputWindow::updateBlurRegion(int width, int height, double scale) {
    if (!atomBlur_) {
    return;
    }
    blurScale_ = scale;
    Rect rect(0, 0, width, height);
    shrink(rect, *ui_->parent()->theme().inputPanel->blurMargin, scale);
    if (!*ui_->parent()->theme().inputPanel->enableBlur || rect.isEmpty()) {
    xcb_delete_property(ui_->connection(), wid_, atomBlur_);
    return;
    }
    std::vector<uint32_t> data;
    if (ui_->parent()->theme().inputPanel->blurMask->empty()) {
    data.push_back(rect.left());
    data.push_back(rect.top());
    data.push_back(rect.width());
    data.push_back(rect.height());
    } else {
    auto region = parent_->theme().mask(parent_->theme().maskConfig(),
    width, height, scale);
    for (const auto &rect : region) {
    data.push_back(rect.left());
    data.push_back(rect.top());
    data.push_back(rect.width());
    data.push_back(rect.height());
    }
    }
    xcb_change_property(ui_->connection(), XCB_PROP_MODE_REPLACE, wid_,
    atomBlur_, XCB_ATOM_CARDINAL, 32, data.size(),
    data.data());
    }
    void XCBInputWindow::update(InputContext *inputContext) {
    if (!wid_) {
    return;
    }
    auto oldVisible = visible();
    if (inputContext) {
    updateDPI(inputContext);
    }
    auto [width, height] = InputWindow::update(inputContext);
    if (!visible()) {
    if (oldVisible) {
    xcb_unmap_window(ui_->connection(), wid_);
    hoverIndex_ = -1;
    }
    return;
    }
    const auto scale = metricScale();
    bool sizeChanged = width != this->width() || height != this->height();
    bool blurScaleChanged = atomBlur_ && scale != blurScale_;
    if (sizeChanged) {
    resize(width, height);
    }
    if (sizeChanged || blurScaleChanged) {
    updateBlurRegion(width, height, scale);
  • Used the bottom highlight margin for XCB menu item region height (this looks like an existing bug in the original code):
    .setSize(
    maxItemWidth +
    roundedScaledMetric(*highlightMargin.marginLeft, scale) +
    roundedScaledMetric(*highlightMargin.marginRight, scale),
    maxItemHeight +
    roundedScaledMetric(*highlightMargin.marginTop, scale) +
    roundedScaledMetric(*highlightMargin.marginBottom, scale));

@wengxt

wengxt commented Aug 1, 2026

Copy link
Copy Markdown
Member

The change could mostly be minimized to xcb part if you just simply downscale the X11 event coordinates and it shall be similar to wayland model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants