Skip to content

Fix embedded webview geometry on scaled displays - #63

Open
Aunali321 wants to merge 2 commits into
webliteca:masterfrom
Aunali321:fix/hidpi-embed-geometry
Open

Aunali321 wants to merge 2 commits into
webliteca:masterfrom
Aunali321:fix/hidpi-embed-geometry

Conversation

@Aunali321

Copy link
Copy Markdown

This fixes two bugs on scaled displays.

Windows (heavyweight): webview_embed_set_bounds sized the child HWND and the WebView2 controller from the Java w, h, which are AWT user-space units. Above 100% scaling the page fills only part of the canvas (about 57% at 175%). It now uses the canvas HWND's client rect, the same way create_engine sizes it.

Linux (lightweight): the hidden popup is moved to (-32000, -32000). GTK multiplies that by the window scale, and X11 positions are signed 16-bit, so at 2x it wraps to (+1536, +1536) and the popup shows up on screen as a second copy of the page. The offset is now divided by the root window's scale factor.

Canvas 6 (sizeNative) and Canvas 33 (D2) are updated to match.

Tested with natives from this branch's CI:

  • Windows 11, 4K at 175%: the page fills the canvas.
  • Fedora 44, GNOME on Wayland (XWayland) at 2x: the popup lands at (-32000, -32000) device px, off screen.

@shannah shannah left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, both diagnoses check out. At 2x, -32000 * 2 = -64000, which wraps to +1536 as an int16. On Windows, create_engine already sizes the child from GetClientRect(parent), so set_bounds now matches it. AWT's Windows reshape is a synchronous call on the toolkit thread, so the canvas HWND has its new size by the time componentResized, and then sizeNative, run on the EDT. Reading the client rect there is safe.

SPDD conformance

  • PR touches the right artifacts. It's a behaviour fix, and the Canvas is amended in the same commit as the code (Canvas 6 §6 sizeNative, Canvas 33 D2).
  • Every code change traces to a Canvas line, and there's no scope creep. Each native hunk maps one-to-one to the added Canvas text.
  • REASONS sections are substantive. They name the exact calls (gdk_window_get_scale_factor of the root window, GetClientRect of the canvas HWND) and say why.
  • Upstream chain. The analysis (spdd/analysis/GGQPA-XXX-202609271830-…media-plays…md:44) still says "-32000 is off every monitor", and the story (requirements/[User-story-9]…md:44) still says "moved to (-32000, -32000)". Both are now wrong at scale > 1. Optional: add a line to the analysis risks so the history isn't misleading.
  • N · Norms / S · Safeguards honoured. Work stays on the WebView2 worker via dispatch_to_thread, and the comments match the Canvas wording.
  • [n/a] last_generated_at. Neither Canvas carries that field.
  • Tests. No automated coverage, which is expected because this depends on display scaling. Manual verification is in the PR description. No check runs reported on the head commit (fork PR). A maintainer may need to approve the workflow run so the natives build in this repo's CI before merging.

Code review

  • windows/webview_embed.cc (webview_embed_set_bounds), RECT r; and the unchecked GetClientRect. The rect is left uninitialized. If GetClientRect fails, for example because the canvas peer's HWND is already destroyed when a late dispatched lambda runs, SetWindowPos and put_Bounds get garbage dimensions. Suggest:
    RECT r{};
    if (!GetClientRect(e->parent, &r)) return;
  • Windows follow-up, not blocking (the bug was already there). If the window moves between monitors with different scaling, the canvas's device size changes but its AWT user-space size may not. Then componentResized doesn't fire, and the child HWND and controller keep the old device size. The native side now ignores Java's w, h, so a more robust follow-up would resize from the canvas HWND's own WM_SIZE (subclass the parent) rather than relying on a Java callback.
  • src_c/webview_embed.cpp (gtk_off_create_engine), scale read once. The popup's scale is read at creation. If the X settings window scale (Gdk/WindowScaling) changes while the engine is alive, GTK re-applies the new scale to the stored position, and a larger scale could wrap it back on screen. This is an edge case and fine to leave; a notify::scale-factor handler that re-moves the window would close it. Integer division is fine (-32000/3 * 3 = -31998).

Overall this looks correct and conforms to the SPDD workflow. The RECT initialization is the only change I'd make before merging.


Generated by Claude Code

This branch has not been deployed

No deployments
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