@Campbell Barton (campbellbarton) Can you give me a hint on what missing functionality has to be implemented before this can be merged?

I will elaborate a bit on the remaining unimplemented methods and their usage within Blender.

So far, the remaining unimplemented functions and their partial callstack are:

• GHOST_SystemWayland::setCursorPosition
  • GHOST_SetCursorPosition
    • WM_cursor_warp

• GHOST_SystemWayland::getAllDisplayDimensions
  • GHOST_GetAllDisplayDimensions
    • wm_get_desktopsize
      • ghost_event_proc

• GHOST_WindowWayland::screenToClient
  • GHOST_ScreenToClient
    • wm_cursor_position_from_ghost
      • wm_get_cursor_position
        • GHOST_GetCursorPosition + wm_cursor_position_from_ghost
      • wm_window / ghost_event_proc (GHOST_kEventTrackpad, GHOST_kEventCursorMove)
        • case GHOST_kEventTrackpad: GHOST_GetEventData + wm_cursor_position_from_ghost
        • case GHOST_kEventCursorMove: GHOST_GetEventData + wm_cursor_position_from_ghost
    • playanim / ghost_event_proc
      • case GHOST_kEventButtonUp: GHOST_GetCursorPosition + GHOST_ScreenToClient
      • case GHOST_kEventCursorMove: GHOST_GetCursorPosition + GHOST_ScreenToClient

• GHOST_WindowWayland::clientToScreen
  • GHOST_ClientToScreen
    • wm_cursor_position_to_ghost
      • WM_cursor_warp
      • WM_cursor_grab_enable
      • WM_cursor_grab_disable

• GHOST_WindowWayland::invalidate
  • GHOST_InvalidateWindow

• GHOST_DisplayManagerWayland
  • initialize
    • GHOST_SystemWin32()
  • getNumDisplays
    • GHOST_SystemWin32::getNumDisplays()
  • getNumDisplaySettings
  • getDisplaySetting
  • getCurrentDisplaySetting
    • GHOST_System::beginFullScreen
  • setCurrentDisplaySetting
    • GHOST_System::beginFullScreen
      • GHOST_BeginFullScreen
    • GHOST_System::updateFullScreen
    • GHOST_System::endFullScreen
      • GHOST_EndFullScreen
      • GHOST_System::disposeWindow
      • GHOST_System::exit

Functions setCursorPosition and clientToScreen are only used for the warping and grabbing functionality which has been implemented in Wayland using the relative-pointer and pointer-constraints protocols.

⇒ These are therefore obsolete for the Wayland implementation.

Function screenToClient is applied after GHOST_GetCursorPosition or GHOST_GetEventData. For the Wayland implementation, the function GHOST_SystemWayland::getCursorPosition and the pointer events provide the pointer positions in the window coordinate frame, and not the screen coordinate frame.

⇒ The mapping via screenToClient is therefore obsolete.

The invalidate method (GHOST_InvalidateWindow) is only used in tests (MultiTest.c, GHOST_C-Test.c), but not in Blender. Is this a deprecated method? The documentation is also a bit ambiguous on this. It does not give details what the semantic of "invalidates the contents of this window" is. How is the window expected to behave after having this method called?

⇒ I think this method is unused.

getAllDisplayDimensions is described with "returns the dimensions of all displays combine (the current workspace)".
But GHOST_SystemX11::getAllDisplayDimensions and GHOST_SystemSDL::getAllDisplayDimensions do not follow this behaviour, they are described with "returns the dimensions of the main display on this system". I am therefore also not sure how GHOST_SystemWayland::getAllDisplayDimensions is supposed to behave.

⇒ This method needs to be implemented and either provides the main screen dimensions or the bounding box for a multi-screen setup.

The GHOST_DisplayManager/GHOST_DisplayManagerWayland, as GHOST_System::m_displayManager, is mainly used for managing fullscreen. However, full screen is already handled by GHOST_WindowWayland::beginFullScreen and GHOST_WindowWayland::endFullScreen. GHOST_System::updateFullScreen is actually not used.

⇒ I think the Wayland implementation does not require an implementation of GHOST_DisplayManager at all.

@Christian Rauch (christian.rauch), thanks for making a detailed list of whats left to do (could be moved into a list of TODO's similar to T73360: Fast highpoly mesh editing)

I think this can be merged in it's current state after 2.83 release.

In D6567#175796, @Christian Rauch (christian.rauch) wrote:

---

Functions setCursorPosition and clientToScreen are only used for the warping and grabbing functionality which has been implemented in Wayland using the relative-pointer and pointer-constraints protocols.

⇒ These are therefore obsolete for the Wayland implementation.

There may still be a handful of cases we would want to support this, good to know they can be avoided with wayland API's though.

---

Function screenToClient is applied after GHOST_GetCursorPosition or GHOST_GetEventData. For the Wayland implementation, the function GHOST_SystemWayland::getCursorPosition and the pointer events provide the pointer positions in the window coordinate frame, and not the screen coordinate frame.

⇒ The mapping via screenToClient is therefore obsolete.

Blender has functionality to pass events to another non-active window (for windowing systems that don't use sloppy focus), this wont work but don't think it's blocking if this isn't supported.

---

The invalidate method (GHOST_InvalidateWindow) is only used in tests (MultiTest.c, GHOST_C-Test.c), but not in Blender. Is this a deprecated method? The documentation is also a bit ambiguous on this. It does not give details what the semantic of "invalidates the contents of this window" is. How is the window expected to behave after having this method called?

⇒ I think this method is unused.

GHOST_InvalidateWindow can probably be removed in this case.

---

getAllDisplayDimensions is described with "returns the dimensions of all displays combine (the current workspace)".
But GHOST_SystemX11::getAllDisplayDimensions and GHOST_SystemSDL::getAllDisplayDimensions do not follow this behaviour, they are described with "returns the dimensions of the main display on this system". I am therefore also not sure how GHOST_SystemWayland::getAllDisplayDimensions is supposed to behave.

⇒ This method needs to be implemented and either provides the main screen dimensions or the bounding box for a multi-screen setup.

For now follow how other platforms work - assuming it's fairly simple to do it this way, noting that it's using main display.

Longer term this needs to be cleaned up, noted in T73586: Code Quality Day Tasks.

---

The GHOST_DisplayManager/GHOST_DisplayManagerWayland, as GHOST_System::m_displayManager, is mainly used for managing fullscreen. However, full screen is already handled by GHOST_WindowWayland::beginFullScreen and GHOST_WindowWayland::endFullScreen. GHOST_System::updateFullScreen is actually not used.

⇒ I think the Wayland implementation does not require an implementation of GHOST_DisplayManager at all.

If it's not needed, thats fine.

The API for GHOST_EventButton and GHOST_EventCursor changed. It now requires a GHOST_TabletData but I think this can be optional if there is no tablet data.

Is this ready for the 2.90 branch?

In D6567#180842, @Campbell Barton (campbellbarton) wrote:

The patch looks close to being ready to commit to master.

Some final things to go over.

• Extra TRUST_NO_ONE should be committed to master separately.
• Some changes to Ghost don't seems like they're needed (change to signature of tablet event for e.g.).

Some of these changes are required for the Wayland implementation. E.g. the default value GHOST_TabletData() for GHOST_EventButton is required since there is no tablet/touch implementation for Wayland as of now.

How should I provide these changes? At the moment, they are already separated and at the beginning of the branch. They could easily be merged into master before the Wayland changes.

When this gets merged, I would like to keep the commit history as it is now. The changes are quite extensive and it would help to keep the original history to track these changes.

Would it help to push this to dedicated wayland branch first, as proposed in https://developer.blender.org/D6567#170944?

I addressed the comments about the C-style comments and the GHOST_TABLET_DATA_NONE.

How should I proceed with the Wayland-unrelated commits (i.e. the first 3 commits in the branch)? Since they are separate commits they will appear in the history separately from the Wayland commits (if you do not squash this pull request, which I would strongly prefer).

• Added /* TODO: what functionality is missing when this is empty? */ to empty functions, if these are OK to be empty and are place-holders which may be useful in the future, this should be noted.
• Committed other changes to master rB54eb70197855: Cleanup: add newline to ghost warning print, rB0da05720dea2: GPU: add assert for immBegin buffer size & context

@Christian Rauch (christian.rauch) could you address the remaining TODO's, then I think this will be ready to go into master.

In D6567#181231, @Campbell Barton (campbellbarton) wrote:

@Christian Rauch (christian.rauch) could you address the remaining TODO's, then I think this will be ready to go into master.

I am not very familiar with this pull request review system. How do I pull your changes into my branch now? I don't see a wayland branch in the official git repo.

You can use arcanist, the command arc patch D6567, then diff the resulting branch with yours.

Otherwise you could download the patch and manually apply it on master, then diff it against your branch.

Generally it's better if you use a repository other Blender developers have access to if you're working on larger projects, so I've added your account to the bf-staging project.
So you can commit to that repository: https://developer.blender.org/diffusion/BS/

STR_String has been removed in master, this patch will need updating.

I do see some possible readability cleanup pass, but nothing what I would consider a stopper for committing this patch to master.
So I suggest we move development to master now.

I removed the usage of STR_String and documented the unimplemented event handlers by citing from the official Wayland protocol documentation. This should provide enough information if this functionality needs to be added later.

This patch is now on the same state as the wayland staging branch at https://developer.blender.org/diffusion/BS/history/wayland/. Please merge from there if possible to keep an accurate account of the history.

I recently updated to Ubuntu 20.04 and hit an issue with the EGL buffer format. This is in fact an Intel driver issue and has been fixed upstream (https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4294). It will take some time until this fix propagates to distributions.

As a workaround, one has to manually select the old i965 driver (in place of the newer iris) by MESA_LOADER_DRIVER_OVERRIDE=i965 blender.

@Brecht Van Lommel (brecht) @Campbell Barton (campbellbarton) @Julian Eisel (Severin) @Sergey Sharybin (sergey) What's the possibility of seeing the wayland patch enabled for one of experimental branches? https://builder.blender.org/download/branches/

In D6567#183307, @Alaska (Alaska) wrote:

@Brecht Van Lommel (brecht) @Campbell Barton (campbellbarton) @Julian Eisel (Severin) @Sergey Sharybin (sergey) What's the possibility of seeing the wayland patch enabled for one of experimental branches? https://builder.blender.org/download/branches/

This would be very useful as it would simplify the distribution of binaries for testing enormously.

Alternatively, I thought about using an environment variable to opt-in to the Wayland front end, similar to how it is done with SDL_VIDEODRIVER=wayland on SDL. That would mean that WITH_GHOST_WAYLAND is ON by default, but a user would need to set an environment variable, e.g. BLENDER_GHOST=wayland, to actually use the Wayland frontend.

@Christian Rauch (christian.rauch) That would also be a good way of implementing it. Didn't think about that. It also allows users to switch between Wayland and Xorg simply by re-opening Blender allowing people to narrow down if a bug is caused by the wayland implementation or a user interface design issue with Blender assuming there isn't issues with Xwayland.

Sorry if I'm getting some of this wrong. I'm relatively new to Linux and most of the technical details of Wayland vs Xorg have just gone over my head. I plan to re-research it again soon. Either way, I appreciate the work you're doing.

@Christian Rauch (christian.rauch) I've created a top-level wayland task T76428: GHOST/Wayland Support, listed some things I noticed to start it off.

I'm not so involved with the build-bot, in general I don't see any problems with enabling wayland, even by default (as long as it needs to be explicitly enabled, as you suggest).