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

SDL3: Brings back ImGuiViewportFlags_NoTaskBarIcon support on win32 #7989

Closed
wants to merge 2 commits into from

Conversation

RT2Code
Copy link

@RT2Code RT2Code commented Sep 15, 2024

This PR is a follow-up of #7973.

It brings back full support for the ImGuiViewportFlags_NoTaskBarIcon, whether the viewport has a parent or not. If the viewport has a parent, the hack is still needed to reenable the task bar icon, because Win32 child windows aren't shown in the task bar by default and the SDL_WINDOW_UTILITY flag doesn't apply to them. On the other hand, if the viewport has no parent, the hack is not required and the SDL_WINDOW_UTILITY flag is enough.

If you wonder why I hide the window before calling SetWindowLong, it's because it's required to apply the change to the task bar.

I'm just puzzled about the comment for the SDL_WINDOW_UTILITY flag. I could not observe any adverse effect on the transition when a viewport is created, do you remember what the issue was?

@RT2Code RT2Code changed the title SDL3: Update ImGuiViewportFlags_NoTaskBarIcon win32 hack SDL3: Brings back ImGuiViewportFlags_NoTaskBarIcon support on win32 Sep 16, 2024
@ocornut
Copy link
Owner

ocornut commented Sep 16, 2024

I admit that I don't clearly understand what the comment implies. I vaguely recall that it maybe have to do with how the flag impacts Windows WM appearing animations and this made the embedded to external-viewport transition less seamless, it I may be wrong and perhaps it depends on older SDL3 specifics or Windows settings. I will investigate.

@ocornut
Copy link
Owner

ocornut commented Sep 16, 2024

For the record, this comment has been added in cb601d7, aka prehistoric era, and initially read "Note: SDL 2.0.6+ has a SDL_WINDOW_SKIP_TASKBAR flag [....]". The fact that it was changed to "Note: SDL 3.0.0+ has a SDL_WINDOW_UTILITY flag [...]" makes a little misleading at it implies it is a recent thing.

Something rather puzzling and unconclusive: and I've tried 2.0.8 is that I noticed that without the _NoDecoration flag, the time the window takes to "fade in / animate" on my Windows 10 seems to vary, and occasionally it is meaningful longer. That's not scientific not carefully measured but I would guess most of the time it takes say, ~0.2 seconds to fully fade in, and occasionally I feel it takes ~0.5 seconds and I feel maybe the hack was related to that but right now I'm been wiggling around with different settings and cannot find conclusive results. Maybe back in 2018 I stumbled on this and erroneously made a connection? I don't know. That being so old I wouldn't worry too much focus on present behavior.

I will test your PR now.

@ocornut
Copy link
Owner

ocornut commented Sep 16, 2024

Forgot my report above on SDL 2.0.8. Here is what I am observing with latest SDL3 on my Windows 10.

With your code:

    if (viewport->ParentViewportId != 0 && !(viewport->Flags & ImGuiViewportFlags_NoTaskBarIcon))
    {
        LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE);
        ex_style |= WS_EX_APPWINDOW;
        ex_style &= ~WS_EX_TOOLWINDOW;
        ::ShowWindow(hwnd, SW_HIDE);
        ::SetWindowLong(hwnd, GWL_EXSTYLE, ex_style);
    }

If I check _NoDefaultParent and uncheck _NoDecoration: when dragging a window outside the main viewport I see the brief fading animation.

If I alter it to e.g:

    if (/*viewport->ParentViewportId != 0 &&*/ !(viewport->Flags & ImGuiViewportFlags_NoTaskBarIcon))
    {
        LONG ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE);
        ex_style |= WS_EX_APPWINDOW;
        ex_style &= ~WS_EX_TOOLWINDOW;
        ::ShowWindow(hwnd, SW_HIDE);
        ::SetWindowLong(hwnd, GWL_EXSTYLE, ex_style);
    }

With the same settings I don't see the fade in.
So basically my comments had to do with how those flags are affecting how windows applies fade-in animations.

…s with no parent to fix the fade-in animation.
@RT2Code
Copy link
Author

RT2Code commented Sep 16, 2024

Oh, you're right, I didn't test it with the NoDecoration flag, and I get the same fade-in animation on Windows 11, therefore the hack must also be applied on nonchild windows to fix it. I made the change you suggested and tested every combination of these flags, everything seems to work perfectly:

NoTaskBarIcon NoDecoration NoDefaultParent Supported No fade in
false false false
false true false
false false true
false true true
true false false
true true false
true false true
true true true

The previous hack was not using the SDL_WINDOW_UTILITY flag on win32, but it is required even with the updated hack or NoTaskBarIcon won't work when NoDefaultParent is enabled.

During my tests, I also spotted another unrelated small issue: If you disable the NoDecoration flag and drag a window outside the main viewport, you can sometimes see the frame on the new window in the top left corner of the screen. I suppose this happens because the SDL3 SDL_CreateWindow function doesn't allow setting the window position at creation. So, because it must be set afterward, you can see the window frame is in the wrong position for a frame. The SDL3 now has an alternative way to create windows (SDL_CreateWindowWithProperties) which allows to specify the position before the window creation that should solve this issue. I will try this, and if it works, I will open a new PR.

@ocornut
Copy link
Owner

ocornut commented Sep 17, 2024

Merged as 1ab1e3c, will merge the other soon later.
I have to admit I even lost track of what the added value of this is. Which of the case are fixed/improved over previously?

@ocornut ocornut closed this Sep 17, 2024
@RT2Code
Copy link
Author

RT2Code commented Sep 17, 2024

I'm sorry if this is confusing. In a nutshell:
-#7973 adds support for parent windows but breaks the NoTaskBarIcon feature.
-#7989 (this one) is a follow-up of the previous one and fix the NoTaskBarIcon support.
-#7991 is unrelated and tries to fix an issue that was present before the parent windows support.

@ocornut
Copy link
Owner

ocornut commented Sep 17, 2024

Thanks. I’ll merge the end-user changelog entries accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants