Feature/wayland support - #42
Conversation
817576c to
c976ac8
Compare
0b3fd09 to
0233fc5
Compare
|
@chrip let's do this one now? |
|
Thanks for the PR! Your continued dedication to the project is truly outstanding. Now that the Qt6 PR is in, we need to prep this one for review. Whenever you have a moment, could you:
This will make the review process much easier for the team. Thanks again! |
dc99c23 to
4d08141
Compare
|
@rikled rebased and the description was updated. Lmk what else you need. :-) |
| target "desktop-export" { | ||
| inherits = ["_common"] | ||
| context = ".." | ||
| dockerfile = "./build/.docker/desktop-composer.bake.Dockerfile" | ||
| target = "desktop-common" # points to the FROM scratch stage | ||
| tags = ["${REGISTRY}/desktop-common:${GIT_COMMIT}"] | ||
| target = "desktop-export" # points to the FROM scratch stage | ||
| tags = ["${REGISTRY}/desktop-export:${GIT_COMMIT}"] | ||
| contexts = { | ||
| desktop-js = "target:desktop-js" # even in stages before desktop-common | ||
| core-base = "target:core-base" | ||
| desktop-builder = "target:desktop-builder" | ||
| desktop-js = "target:desktop-js" |
There was a problem hiding this comment.
We now distinguish between common and os specific. So desktop-common builds web-apps, sdkjs, core-wasm and desktop-js. build/windows and build/linux use the output of common to build os specific code.
|
Thanks for rebasing the main PR! I think the submodules weren't rebased though. Could you rebase those as well? Regarding the PR description:
As far as I know, this project has never actually used CEF 120. These changes were just made during the creation of the PR, right? Upgrading to CEF 120 is definitely something we should do, but I think that should be handled in a separate PR.
I think there's a misunderstanding here. During the initial PR review, we changed the build process (see my earlier comment) so that common files and OS-specific files are built separately. Because of that, this main PR shouldn't need to modify the build process at all. It should only update the submodule references. Regarding the submodules themselves, as far as I can tell only desktop-sdk and desktop-apps contain relevant changes. Most of the changes in core have already been merged, except for the CEF-related changes. Since CEF is no longer pulled in through vcpkg, those entries shouldn't be added back to vcpkg.json/vcpkg.conf. Also, your web-apps PR doesn't appear to contain any changes. |
4d08141 to
5dea150
Compare
|
Hi @pplupo, Thanks again, You're incredibly fast! I have some more comments:
desktop-sdk:
desktop-apps:
Edit: There's one part that worries me a bit: https://github.com/Euro-Office/desktop-sdk/pull/9/changes#r3509757678 Edit 2: I tested this locally on Wayland and found that umlauts don't work (e.g. typing ä/ö/ü via dead keys produces nothing or the wrong character). Testing further, it looks like non-Latin input doesn't work in general. |
Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
6a7235f to
d73df9a
Compare
Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
Points desktop-sdk and desktop-apps at the fix for the Wayland freeze: the CEF off-screen buffer is now presented through a QOpenGLWidget overlay instead of the raster backing-store path, so the GL swap itself drives the compositor's frame-callback commit and no longer stalls until a physical input event forces a flush. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
0cb67d0 to
175a20e
Compare
Was pinned 211 commits behind at c6368d5 (Euro-Office#63), which predates the icon restoration fix in Euro-Office#67 and everything merged since. Build artifacts were missing that fix as a result. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
Re-signed a pre-existing commit that was missing a Signed-off-by trailer (dead key compose support); no content change. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
@rikled this was super hard. If I knew it was going to be this hard before I started idk if I would have. Please, check. :-) |
Sorry to hear that. You definitely didn't pick an easy challenge, which makes it even more impressive that you managed to pull this off in such a short time. That's genuinely remarkable. Great work!
I'll take a look at it today. I also need to test whether the Windows build still works. At first glance, though, it looks very promising again. :) Thank you for your hard work! |
|
Your kind words are always encouraging. Thanks. :-) |
Points at release/wayland-db-support in both submodules, which now include the native OS clipboard bridge fix for CEF OSR mode under Wayland. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
I've looked over the code and don't see any major issues at first glance. There are a few minor things that I'll point out later. I tested it locally in a Wayland session, and it works great. Then I switched to an X11 session, and unfortunately, it no longer works. I can take a look at it if you need a break. :) In general I think we're almost there! |
|
@rikled to be honest, I actually ran into a bug myself when I was testing the db support, and it's due to wayland. I'd appreciate very much if you could work on the x11 issue. It should work on both. |
|
@rikled I think I finally got the clipboard issue fixed. |
|
@pplupo Thanks, that sounds great! I have a few other things I need to take care of right now, but I'll get to it soon. |
|
@rikled I finally fixed the scaling issue. It was a bit tough to fix because OnlyOffice was too large and I had to find something to use as reference for the baseline size. I ended up comparing buttons and canvas sizes with Libre Office to find a correction factor, so it's not too small on scale 1x and it grows proportionally according to the monitor scaling setting. It looks good to me now. Comfortable, I'd say. Check it out and lmk what you think. If you are ok with it, I'm ok too. Couldn't find any other issues. @chrip FYI |
|
That's great; I think we're almost ready to merge. I've fixed the X11 issues. Two things, though:
Once these two things are taken care of, I’d suggest that @chrip takes another look at it, and then we can merge. |
|
I have absolutely no idea how I did that. I'll correct it. |
|
OK, I figured it out. I accidentally merged my configurable-arch-baseline here. And I also accidentally branched configurable-arch-baseline from "master" instead of "main," and "master" has onlyoffice stuff. |
A configurable-arch-baseline branch was briefly merged into this branch and later fully reverted (including transient upstream commits it had accidentally picked up via a stale master basis: a license text change, a changelog entry, a README update, and a submodule bump) -- net contribution to this branch is zero, folded into this commit so the intermediate churn doesn't stay in history. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
0dd0cc3 to
fa2e41a
Compare
|
ok, the scaling fixes are in! They were made in core and desktop-sdk. It only took me an entire week. lol This was a tough one. I felt the QT was there as a second thought, like a fallback. It didn't get as much love as GTK. The dialogs were poorer in formatting and overall design. After fixing the scaling, they looked horrible. I tried to reproduce the same look as the GTK dialogs so that users have a standardized (and more pleasant) experience. |
chrip
left a comment
There was a problem hiding this comment.
Thanks @pplupo!
I tried to get the CI green. The PR's CI is red only because the fork can't run the secret-gated stages, so I ran it in-repo.
Linux amd64/arm64, crosscompile, and common all pass, but windows both fail at CMake configure: https://github.com/Euro-Office/DesktopEditors/actions/runs/30266743437
@rikled I have no idea what the current status of the Windows build is. Does it theoretically already work in the gh action?
This is what Claude suggests to fix the Windows build:
Root cause is in desktop-sdk → ChromiumBasedEditors/lib/CMakeLists.txt: two Linux-only CEF headers got added to the unconditional ASCDOCUMENTSCORE_HEADERS list —
include/internal/cef_linux.h (line 497)
include/internal/cef_types_linux.h (line 512)
On Windows CEF_SRC_PATH points at the windows CEF tree, which has neither, so target_sources (line 1103) fails: "Cannot find source file: …/cef_linux.h". They're already listed in the Linux else() branch (lines 1086–87), so just drop them from the common list and re-bump the submodule pointer — Linux is unaffected. Worth a full re-run afterward to confirm nothing else surfaces on Windows.
|
@pplupo I tested on Linux X11 and Wayland which works great. On Windows however the editors no longer load. I will take care of that. I need your help to get a better understanding of the situation:
Thanks again! Unfortunately, the process is taking a little longer than expected, but it is, after all, a significant change to the code. Edit: I fixed the windows issue but I cannot push to you desktop-sdk branch. Could you grant me rights? |
- desktop-sdk: rebase release/wayland-db-support to drop the db-support/parquet content (Add parquet format support, force read-only mode for BerkeleyDB recent-file reopen, recent-file database read-only fix) per review feedback -- these are unrelated to Wayland support. - sdkjs: rebase feature/wayland-migration to drop the wavy-underline spellcheck feature commits, unrelated to Wayland support. - web-apps: point at origin/main instead of feature/wayland-migration, since that branch's diff vs main is now only the wavy-underline feature -- no Wayland-related content remains on it. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
@rikled Thanks for the detailed review! Addressed all four points:
Also invited you as a collaborator on Thanks for your patience on this one! |
|
@pplupo Thanks a lot for the update and the permissions. I pushed the Windows fix. Unfortunately, there are still a few issues:
Do you have the possibility to test within an X11 session? Sorry that it's taking so long. Just to be clear again: you're doing an amazing job, and your contribution is really helping us move the project forward! |
|
OK, I'll take a look at these things soon. I'm working on something else on a different branch but I'm almost done. I can test it using XWayland. Sorry for the dumb question, what are the cell labels? Can you post a screenshot of the title bars at that resolution and tell me your UI scaling factor? |
desktop-apps to c3366c6f9, desktop-sdk to 3ebc11d1. Brings in the fixes for all three issues raised on the PR: - Cursor no longer changing in Windows/X11 sessions: the title-button HoverLeave reset is now Wayland-only, and OnCursorChange in desktop-sdk's CCefView no longer intercepts cursor changes outside Wayland OSR mode (it was unconditionally telling CEF's windowed mode to skip its own native cursor-setting on every platform, freezing the cursor at a plain arrow everywhere in the editor content). - Inconsistent/oversized rescaling, especially the top bar: the app's manual DPI-ratio chrome sizing now divides out whatever scaling Qt already applies itself, instead of compounding with it. Also removes a UI-scale debounce that was delaying every real display-scale change (e.g. a cross-monitor move) by two calls, which read as the editor visibly rescaling twice. - QPainter 'painted by one painter at a time' errors on closing an editor on Wayland: QGraphicsDropShadowEffect is now disabled on Wayland, where the compositor draws window shadows natively and the Qt software shadow's own painter could collide with a concurrent compositor repaint during window teardown. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
core to e3c8f12765, web-apps to 31217eed8e. Both branches were rewound on the fork to drop commits that didn't belong on this PR's history: core's depot_tools/v8-pinning pair (preserved on fix/v8-gclient-paths-patch) and web-apps' wavy-underline-setting addition (preserved on misspell-wavy-line). No functional change to this PR's own scope -- these were unrelated commits that had accumulated on the branch via prior merges. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
Thanks for the detailed report, and sorry it took a few rounds to actually land — the fixes were sitting in the submodules but the superproject's pointers here hadn't been bumped to include them until now, so nothing you tested could have shown them. That's fixed; this branch is now current. Cursor no longer changing (Windows/X11): the title-button Rescaling inconsistency, especially the top bar: the app was multiplying its chrome sizes by a manually-detected DPI ratio and Qt was scaling the result again itself, so on a 1.25× display the chrome rendered at 1.5625× while the editor content (scaled once, via CEF page zoom) sat correctly at 1.25×. Worth flagging this wasn't Wayland-specific — Qt 6 scales on QPainter errors on close: I tested the scaling fix specifically at 1.25× and 1.5× across two monitors and confirmed the chrome and editor content now agree with each other and with X11 at the same configured scale. If your X11 session is still showing things too small, could you share the output of |
|
Thanks, @pplupo. I think it looks really good now. I've tested it on X11, Wayland, and Windows. It runs like a charm. I fixed a small scaling issue in X11, lgtm now. From now on, we should focus on fixing issues and not add anything new so we can get this merged. Some comments from my side:
|
|
I'll look into it. |
|
@rikled |
…ixes desktop-apps to 18fa981f5, desktop-sdk to 4537f97a, sdkjs to 4d7653abc0. - UpdateUIScalePercentage() no longer re-applies SetZoomLevel/ WasResized on every 1s poll tick when the scale hasn't changed (the per-frame JS injection still runs every call, since a newly loaded frame needs it regardless); dropped the [UIScale] console.log calls. - The native clipboard bridge (nativeClipboardWrite / clipboard_read) now requires an explicit isWayland signal instead of inferring OSR mode from window.AscDesktopEditor's mere presence, which was true on every desktop platform and raced the bridge against CEF's own clipboard.write() on X11/Windows. Added a real timeout to the native clipboard read (the doc comment already promised one; none existed). Copying an image file in the file manager and pasting it into a document now works under Wayland -- GetClipboardData() previously only checked QMimeData::hasImage(), never hasUrls(), which is what a file-manager copy actually populates. - device_scale.js's correctApplicationScale() had the same window.AscDesktopEditor-presence conflation, disabling the CSS-zoom scale correction on X11/Windows where it's still needed, not just Wayland OSR where Qt already provides correctly scaled DIPs. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
Sorry for not being more precise before. SendGatewayDevToolsMessage: int64_t e.g.: I thought again about restart rewrite and I think we can keep it. But for the records:
icons: |
desktop-apps to 5278219fd, desktop-sdk to 35d70efc. - desktop-sdk: invalidate the applied-UI-scale cache on navigation. CEF page zoom is per-load, so the dedup added for the review cached a value that navigation then discarded, leaving documents rendered unscaled until an unrelated scale change happened to invalidate it. - desktop-apps: revert selecting the zoom/scaling QSS variants by the real display scale. Those variant blocks carry geometry, not just fonts, and are pre-computed physical values; selecting them by the residual is what makes one shared stylesheet land on the same physical size whether or not Qt scales on a given platform. Feeding the real scale double-applied the factor to the geometry. - desktop-apps: raise the chrome base font from 11px to 12px, matching the ribbon's own tab labels (web-apps' @font-size-medium), which is what actually made the title bar and dialog text look undersized. Verified on both Wayland and X11 at 1.25 display scale: identical measured results (dpiRatio 1.0, rasterized at 1.25, 24x28 logical -> 30x35 physical). Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
desktop-sdk to fbe0ac39. Removes the two console.error calls left in the injected UI-scale script, completing the review request to drop that instrumentation. The try/catch blocks stay -- the script is injected into every frame and must not throw -- but are now empty with a comment explaining why. Signed-off-by: Peter P. Lupo <pplupo@gmail.com>
|
@rikled can you please take another look? |
|
Thanks! I'm happy. @chrip Can you have another look? |
chrip
left a comment
There was a problem hiding this comment.
LGTM on the substance. My earlier Windows CMake blocker is fixed (verified green on both
Windows arches in the in-repo run), and all of @rikled's 08-06 points hold up when checked
against the pinned code: the UI-scale dedup, the Wayland-gated clipboard bridge with its
timeout and paste fallback, the isWayland gate in device_scale.js, and the DevTools
bridge removal. Nice work @pplupo — and the code comments in this round are genuinely good.
Approving, as I trust @rikled to handle the submodule-pointer coordination at merge. Three
to-dos for the merge:
- web-apps: the pin (
31217eed) is not on any Euro-Office ref and has no PR. Its only
content delta vsmainisapps/spreadsheeteditor/main/app/controller/DocumentHolder.js
(+12/-1, the OSR context-menu fix — looks correct). Worth a small PR, then re-pin tomain. - Fresh CI on the final head: last green in-repo run was
b10197ba8(07-29); head is
4b2fa25e5, 5 submodule bumps later. The PR's own red checks are just the fork-token
artefact, not a build failure. - Merge order: four pins are PR heads (core#113, desktop-apps#30, desktop-sdk#11,
sdkjs#64 — note the body still lists the closed #9). Squash-merging any of them orphans
the pin, so merge-commit them and re-bump the pointers here.
Non-blocking: the int64→int64_t churn is still in desktop-sdk (and now inconsistent — new
lines still use int64), and message_warn.svg's colour change is unrelated.
Assisted-by: ClaudeCode:claude-opus-5
1. Wayland Compatibility & Crash Fixes
QX11Info,GuiPrivate, andX11Extrasthroughout the codebase (spanning bothdesktop-appsanddesktop-sdk). These legacy Qt5 dependencies were forcing X11 calls that caused immediate segmentation faults in pure Wayland environments. The application now fully relies on Qt 6's Platform Abstraction (QPA) layer, seamlessly supporting both X11 (xcb) and Wayland (wayland-egl) at runtime.2. Chromium Embedded Framework (CEF) Stabilization
desktop-sdkto version 109. The codebase had previously pulled in CEF 120 files, but the underlying pre-compiledvcpkgbinary remained at CEF 109. Aligning the headers with the binary resolves the silent startup segfaults caused by the API hash mismatch.cef_download_interrupt_reason_tandint64_t) back to their CEF 109 compatible types to fix associated build compilation failures.3. Build Environment & Submodule Synchronization
build/docker-bake.hcl) by introducing thedesktop-buildertarget and resolving conflicting export strategies to streamline CI/CD compilation.core,desktop-apps,desktop-sdk,web-apps, andsdkjsto accurately reflect the coordinated changes required for the Qt 6 / Wayland port.Depends on: Euro-Office/core#113 Euro-Office/desktop-apps#30 Euro-Office/desktop-sdk#9, Euro-Office/sdkjs#64