Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wayland: use server-side decorations if possible #385

Closed
wants to merge 1 commit into from

Conversation

mahkoh
Copy link

@mahkoh mahkoh commented May 22, 2024

This makes IntelliJ use native SSD on KDE, Sway, and Jay. On GNOME, it continues to use the existing CSD.

This solves the styling issue on compositors supporting SSD.

@mahkoh mahkoh mentioned this pull request May 22, 2024
@mkartashev mkartashev self-requested a review May 22, 2024 07:25
@mkartashev
Copy link
Collaborator

Thank you for the patch! I will test it out and get back to you

@mkartashev
Copy link
Collaborator

How does server-side decorations interact with the window size? I'm particularly concerned about the values passed to xdg_surface_set_window_geometry(); I don't see how they can be adjusted, but maybe simply skip this call so it does not interfere with server-side decorations?

@mkartashev
Copy link
Collaborator

What is the performance impact of using server-side decorations together with the shared memory buffer? Can you run SwingMark and RenderPerf with and without the patch and post the results?

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

How does server-side decorations interact with the window size?

xdg_surface_set_window_geometry is always called with the width/height from the configure event. These values are not affected by presence of client-side decorations. Client-side decorations only create an inset which causes the window contents (excluding the CSD) to be rendered into a smaller part of the surface.

What is the performance impact of using server-side decorations together with the shared memory buffer?

It should not make a difference but I will take a look.

@mkartashev
Copy link
Collaborator

How does server-side decorations interact with the window size?

xdg_surface_set_window_geometry is always called with the width/height from the configure event. These values are not affected by presence of client-side decorations. Client-side decorations only create an inset which causes the window contents (excluding the CSD) to be rendered into a smaller part of the surface.

This aligns with my understanding of CSD and this is why I specifically asked about server-side decorations. I don't know if the server counts the size the decorations it provides towards the window geometry or not.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

SwingMark is essentially unchanged with roughly the following results:

Starting SwingMark
SwingMark Test started at Wed May 22 10:52:05 CEST 2024
Program will automatically terminate after last run
Setting L&F to: javax.swing.plaf.metal.MetalLookAndFeel
Startup Time: 229
Sub-Menus = 568   (Paint = 0)
TextArea = 190   (Paint = 691)
Sliders = 148   (Paint = 501)
Lists = 152   (Paint = 493)
Table Rows = 160   (Paint = 213)
Tree = 321   (Paint = 756)
Score: 1813

I was not able to get RenderPerfTest to work before or after these changes. After starting it, it just keeps asking to capture the output. If you allow it to capture the output, it just asks again.

I don't know if the server counts the size the decorations it provides towards the window geometry or not.

It doesn't. If SSD is used, the geometry should be set to the size of the window contents.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

I was not able to get RenderPerfTest to work before or after these changes.

Strike that. I had forgotten to pass -Dawt.toolkit.name=WLToolkit to this test. The results are the same before and after this PR.

@mkartashev
Copy link
Collaborator

On KDE, this is how IDEA's "Search Everywhere" window look with this patch:
image

It definitely doesn't need any decorations, but those are present for some reason.

I understand that this protocol doesn't allow for any type of configuration of the decorations aside from on/off, but having a maximized button on a dialog feels weird. It also works weird: when toggled, the IDEA's About dialog, for example, does not get reset into its original shape and size, so its content gets distorted.
image

Did you have any experience working with libdecor? Maybe it provides more control over what should and should not be on the title bar of a window?

Notice also the icon that is located at the top left corner: it is the generic Wayland icon instead of the product's icon. Do you have an idea about how to fix that? (FWIW, Firefox's window also has the same icon).

@mkartashev
Copy link
Collaborator

In addition to the always-present "maximize" button, it seems that decorating any window will make it resizable, for which many, if not all, IDEA dialogs are not prepared.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

It definitely doesn't need any decorations, but those are present for some reason.

I hadn't considered that you might want to have undecorated toplevels. This should now be fixed.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

Notice also the icon that is located at the top left corner: it is the generic Wayland icon instead of the product's icon. Do you have an idea about how to fix that? (FWIW, Firefox's window also has the same icon).

There is currently no API to set an application icon. Some compositors read the icon from desktop files.

Signed-off-by: Julian Orth <ju.orth@gmail.com>
@mkartashev
Copy link
Collaborator

@mahkoh All this Wayland-related work is intended to be committed to OpenJDK under the umbrella of project Wakefield some day. Since this patch is not created by a JetBrains employee, we won't be able to push it with the rest of the code, which will create some merging problems in the future. Would you be able to contribute this patch directly to OpenJDK so I can then pull it from there?

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

Since this patch is not created by a JetBrains employee, we won't be able to push it with the rest of the code, which will create some merging problems in the future.

I have signed the CLA which gives JetBrains a license to distribute the code under any license. Is this not sufficient?

@mkartashev
Copy link
Collaborator

I have signed the CLA which gives JetBrains a license to distribute the code under any license. Is this not sufficient?

No.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

So should I create a PR in https://github.com/openjdk/wakefield/tree/jdk21.0.1-wayland?

@mkartashev
Copy link
Collaborator

Are you an OpenJDK contributor? Have you signed https://oca.opensource.oracle.com/ already?

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

I have now.

@mkartashev
Copy link
Collaborator

Then you can open a PR against https://github.com/openjdk/wakefield/tree/jdk21.0.1-wayland

@mkartashev
Copy link
Collaborator

Notice also the icon that is located at the top left corner: it is the generic Wayland icon instead of the product's icon. Do you have an idea about how to fix that? (FWIW, Firefox's window also has the same icon).

FWIW, this is solved by placing jetbrains-idea-ce.desktop (or whatever your IDE is) into ~/.local/share/applications/.

@mahkoh
Copy link
Author

mahkoh commented May 22, 2024

openjdk/wakefield#7

@mkartashev
Copy link
Collaborator

This will be further tracked as openjdk/wakefield#7. Closing.

@mkartashev mkartashev closed this May 23, 2024
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