Skip to content
Snippets Groups Projects

qt: allow enforcing compositor and preliminary support for wasm video integration through `CompositorPlatform`

Open Fatih Uzunoğlu requested to merge fuzun/vlc:qt/compplatformenvvar into master
2 unresolved threads

I have not tested the Wasm video integration yet, but since Qt 6.5 Wasm platform plugin patch Use the browser compositor for drawing windows on WASM (https://codereview.qt-project.org/c/qt/qtbase/+/437285) we should be able to have video embedding with Wasm.

The build system needs adjustment to test it, so it is only used when CompositorPlatform is enforced via --qt-compositor=platform. I don't know how much change the build system needs for Qt at the moment, I'm really not familiar with the build system. It seems (https://github.com/qt/qtbase/blob/059210b17e50fd7c45af4ca0631ddcf8dd7c67f8/src/corelib/Qt6WasmMacros.cmake#L80) that we at least need to copy qtloader.js.

Previously in !5542 (merged), we already started to use the Qt Wasm platform plugin when __EMSCRIPTEN__ is defined.

I have taken a look at https://code.videolan.org/jbk/vlc.js to see what the wasm "window" would need. I initially thought that it would be a DOM canvas, but turns out it is a GL context. I'm not sure how it is supposed to handle resize that way, but in the future we can change it.

Similar to wasm, wayland and xcb can also be enforced with --qt-compositor=platform. They are not used by default, since there is already CompositorWayland for Wayland, and there is no support for child window transparency with xcb (Qt with xcb native painting feature and QQuickWidget might be necessary to support that. Currently, with Qt 6.8 and KWin X11 OpenGL backend there is no support).

Request review @chub.

Edited by Fatih Uzunoğlu

Merge request reports

Members who can merge are allowed to add commits.

Merge request pipeline #559815 passed

Merge request pipeline passed for f23446b8

Test coverage 17.72% (-0.01%) from 1 job
Approval is optional
Ready to merge by members who can write to the target branch.

Merge details

  • The source branch is 222 commits behind the target branch.
  • 6 commits will be added to master.
  • Source branch will be deleted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Fatih Uzunoğlu changed the description

    changed the description

  • Fatih Uzunoğlu added 5 commits

    added 5 commits

    • 0907351c - qt: provide information if the compositor is enforced to the compositor
    • 49fa35d7 - qt: add possibility to enforce `CompositorPlatform`
    • 0d65d818 - qt: consider `xcb` platform when enforced in `CompositorPlatform`
    • ab335995 - qt: consider `wasm` platform when enforced in `CompositorPlatform`
    • f23446b8 - qt: consider `wayland` platform when enforced in `CompositorPlatform`

    Compare with previous version

  • The build system needs adjustment to test it, so it is only used when CompositorPlatform is enforced via --qt-compositor=platform. I don't know how much change the build system needs for Qt at the moment, I'm really not familiar with the build system

    TBF, the first part would be building the contrib for wasm. :)

    • If this is known to be broken on windows7 and XCB, why allow this?

    • Author Reporter

      I'm not sure what is better. I thought that we can not with certainty know if it works or not considering that the platform plugin may be patched.

      I think that if it is enforced, it is on the user if it is enforced and does not work. At the same time, it is only advertised on Windows (the list is static, so it has been advertised on Windows 7 as well unfortunately) and Apple.

    • I thought that we can not with certainty know if it works or not considering that the platform plugin may be patched.

      I don't think that's a realistic expectation.

    • Author Reporter

      I don't think that's a realistic expectation.

      The reason can be anything, not only limited to that. But we expect that it would not work, so they are not supported currently.

      When enforced, we probably should allow, as that is what the user wants. We should respect the user's decision.

      The change for enabling a platform in PlatformCompositor is trivial, so it is also not bad for maintenance.

    • We shouldn't knowingly allow running broken configurations, we already have enough systems and configurations to support.

    • Author Reporter

      We shouldn't knowingly allow running broken configurations, we already have enough systems and configurations to support.

      This is only the case when it is enforced, it is on the user if they want to use --qt-compositor=platform with unsupported configuration.

      And it is only advertised on Windows and Apple builds.

    • Author Reporter

      That being said, I agree that it is definitely not worth spending effort on supporting extra configuration if it requires extra effort (than only setting the window ID), provided that there is already a first class supported integration method (--qt-compositor=x11, --qt-compositor=wayland).

    • Please register or sign in to reply
  • Pierre Lamot changed milestone to %4.0

    changed milestone to %4.0

Please register or sign in to reply
Loading