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

Fix Linux X11 Atoms lost: Before the window was mapped, only the first atom was set. #16110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

walterlv
Copy link
Contributor

What does the pull request do?

  1. We can set multiple atoms before the window is mapped, e.g. ShowInTaskBar="False", Topmost="True", WindowState = WindowState.FullScreen.
  2. Before this PR, only the first atom was set. All other atoms were lost.
  3. This PR fixes the issue by setting all atoms before the window is mapped.

What is the current behavior?

For example, if we set window properties like this:

public MainWindow()
{
    InitializeComponent();

    ShowInTaskbar = false;
    Topmost = true;
    WindowState = WindowState.FullScreen;
}

Before this PR, we can only find that the taskbar icon is hidden, but the window is not topmost and not full screen.

What is the updated/expected behavior with this PR?

After this PR, we can find that the taskbar icon is hidden, the window is topmost, and the window is full screen.

How was the solution implemented (if it's not obvious)?

See the code before this PR:

XGetWindowProperty(_x11.Display, _handle, _x11.Atoms._NET_WM_STATE, IntPtr.Zero, new IntPtr(256),
    false, (IntPtr)Atom.XA_ATOM, out _, out _, out var nitems, out _,
    out var prop);
var ptr = (IntPtr*)prop.ToPointer();
var newAtoms = new HashSet<IntPtr>();
for (var c = 0; c < nitems.ToInt64(); c++) 
    newAtoms.Add(*ptr);
XFree(prop);
foreach(var atom in atoms)
    if (enable)
        newAtoms.Add(atom);
    else
        newAtoms.Remove(atom);

The newAtoms add only the header of the pointer ptr. So only the first atom was added even if the for runs multiple times.

We changed the code to:

    for (var c = 0; c < nitems.ToInt64(); c++) 
--      newAtoms.Add(*ptr);
++      newAtoms.Add(*(ptr+c));

So all atoms are added to the newAtoms.

Checklist

  • Added unit tests (if possible)? No unit tests are added
  • Added XML documentation to any related classes? No XML documentation is added
  • Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation This is a bug fix, no user documentation is needed

Breaking changes

None.

The X11 atoms change is unique and the X system auto removes the repeated atoms so it is safe to add multiple atoms even if the developers add the same atoms themselves.

Obsoletions / Deprecations

None.

Fixed issues

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0049249-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added os-linux area-x11 backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Jun 25, 2024
@maxkatz6 maxkatz6 requested a review from kekekeks June 25, 2024 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-x11 backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch os-linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants