Kill HRGN

[beviu]

Summary of the Pull Request

My understanding is that the XAML framework uses another way of getting mouse input that doesn't work with WM_SYSCOMMAND with SC_MOVE. It looks like it "steals" our mouse messages like WM_LBUTTONDOWN.

Before, we were cutting (with HRGNs) the drag bar part of the XAML islands window in order to catch mouse messages and be able to implement the drag bar that can move the window. However this "cut" doesn't only apply to input (mouse messages) but also to the graphics so we had to paint behind with the same color as the drag bar using GDI to hide the fact that we were cutting the window.

The main issue with this is that we have to replicate exactly the rendering on the XAML drag bar using GDI and this is bad because:

1. it's hard to keep track of the right color: if a dialog is open, it will cover the whole window including the drag bar with a transparent white layer and it's hard to keep track of those things.
2. we can't do acrylic with GDI

So I found another method, which is to instead put a "drag window" exactly where the drag bar is, but on top of the XAML islands window (in Z order). I've found that this makes us receive the WM_LBUTTONDOWN messages. That window has to have the WS_EX_LAYERED style and I don't know why. Also we have to recreate the window on every resize (see comments).

References

PR Checklist

• Closes Investigate Acrylic in titlebar / not using a HRGN for the titlebar #4744. Closes When a dialog is open, the titlebar is the wrong color #2100.
• CLA signed. If not, go over here and sign the CLA
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: Investigate Acrylic in titlebar / not using a HRGN for the titlebar #4744

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

[DHowett-MSFT]

!!!!

[beviu]

It works but I wouldn't be that happy about it because it's very hacky:

1. I have to recreate a window on every resize because otherwise it mysteriously doesn't receive mouse messages and I don't understand why.
2. I have to use WS_EX_LAYERED on a child window and I don't understand why.

It's just to give an idea of the idea because I had already wrote this code but I'll let you judge if it's worth accepting or if it's too hacky.

[zadjii-msft]

I mean, I really like this. I'm certainly always hesitant about changes to our window, so I'd want to keep a careful eye on this, but just playing with this, it doesn't seem like anything else has regressed.

I'm definitely curious about why we need to remake a new HWND every time the window resizes. That's certainly really odd. Maybe the island window is placing itself on top of the draw window on a resize, and we just need to move the drag window back on top of the island window (in the z-order)? I'm no expert here so that's just a guess.

[zadjii-msft]

Would this also probably fix #4166 (by virtue of just not having a grab bar at all)?

@DHowett-MSFT and I spoke about it, and if it doesn't end up working, and creates more issues that are less solvable, it's easy enough to revert before 1.0. I've played with this and I honestly don't think it's bad. @greg904 If you're okay with this going in, I think we're okay with this change.

[beviu]

It doesn't fix #4166. Now the big gray rectangle that covers the min/max controls is "invisible" when the bug happens but it's still "there" and catches mouse input:

@greg904 If you're okay with this going in, I think we're okay with this change.

OK, I'm OK.

[beviu]

Maybe the island window is placing itself on top of the draw window on a resize, and we just need to move the drag window back on top of the island window (in the z-order)? I'm no expert here so that's just a guess.

Sorry I've explained the problem badly. The problem is that when I resize the window with SetWindowPos or MoveWindow, it looks like it doesn't do anything! The new area of the window doesn't receive any WM_LBUTTONDOWN messages, as if the resize didn't happen. Maybe I'm using the API wrong. I checked with a tool and the window's size actually changes, but it just doesn't receive any mouse message over the new area. If I recreate the window with the new position and width, it works again. I'm no expert either.

[beviu]

Oh wait I've removed the code that for WM_PAINT because I thought it was no longer needed to paint the drag bar but we can see the old title bar beneath the XAML island when resizing (this is debug build, slowed down):

Should I add the old code back for WM_PAINT that paints the whole window with the drag bar color to hide that title bar? Or maybe just paint black on top of the old title bar to hide it? Or maybe leave it like that because it's hard to notice, especially when running the release build?

[DHowett-MSFT]

I'd prefer we preserve the color -- it's more uniform-looking 😄

[DHowett-MSFT]

I absolutely love that this lets us hit test even over the tab bar and do, largely, whatever the heck we want. In the future, we could even make it ask XAML "what is underneath the cursor" before doing anything. I am so excited about this.

[beviu]

@DHowett-MSFT

I'd prefer we preserve the color -- it's more uniform-looking 😄

Ok. I will do this later when I have time. BTW, note that if I use the title bar color then we will get #2100 but instead of title bar it will be the background color while resizing.

I absolutely love that this lets us hit test even over the tab bar and do, largely, whatever the heck we want. In the future, we could even make it ask XAML "what is underneath the cursor" before doing anything. I am so excited about this.

I'm not sure that this is true. Right now, either the XAML islands window receives the input either it's the "drag window" that is put on top of the drag bar.

Maybe you got that impression from the fact that I said this originally:

I got this idea by using looking at the Settings UWP app's window using a tool called Spy++.
It looks like it does something similar to this (but not exactly this because the window's size is not exactly the same as the title bar's size) with a window called ApplicationFrameInputSinkWindow that only receive messages when I put my mouse on the drag bar:

Unfortunately I was not able to replicate the behavior from ApplicationFrameHost.exe. It looks like it's able to somehow control when either the ApplicationFrameInputSinkWindow window receives the message or when the window for XAML receives the messages. I don't understand how it works.

Another mystery is that for UWP apps, this ApplicationFrameInputSinkWindow window is not actually on the drag bar. It's below the drag bar and covers the whole window yet it receives messages when I move my cursor on the drag bar and those messages have negative Y coordinates because the cursor is not actually on the window... except when it's not the case, I have an UWP app where I called the title bar API to set everything as draggable and now this ApplicationFrameInputSinkWindow window doesn't even receive messages anymore when hovering over the title bar.

Basically, the more I try to understand how this thing works, the weirder it gets everytime 😆.

[DHowett-MSFT]

Nah, I was being more wishful/hopeful 😄 like, if we could get input on the drag bar window we could push it down into Xaml land ourselves, or some-other-how reflect it into the input queue. That's a problem for future-us.

[beviu]

I'd prefer we preserve the color -- it's more uniform-looking 😄

done

[beviu]

The root cause of #2100 causes two bugs:

1. When a dialog is open, the titlebar is the wrong color (When a dialog is open, the titlebar is the wrong color #2100)
2. When a dialog is open, the color that is painted temporarily while resizing the window until the XAML Islands window is resized is the wrong color

This fixes the first issue but not the second.

I don't think that this is really important because when this is fixed: microsoft/microsoft-ui-xaml#759, we will be able to get rid of this workaround and this issue will go away.

So I don't think it's necessary to open a new issue for this small thing.

[miniksa]

@msftbot make sure @zadjii-msft and @DHowett-MSFT sign off

[zadjii-msft]

I'm tempted to let this one sit until we've got a fix for #4166, simply from the fact that it'll be easier to debug #4166 without this fix 😄

At the same time, I also really like this fix for what doors it opens for us, so ¯\_(ツ)_/¯

[DHowett-MSFT]

So, I had a go at testing this. I like it! Couple bits of feedback.

• I bet there's a way to get it working without having to recreate the
  window every time.
• I tried briefly to do the resizing thing, and yeah -- I was having
  some Z order issues and couldn't even get the window to reliably paint
  (i tried making it red so that i could see what was happening)
  • Weirdly, adding a WM_PAINT handler made the window totally
    hit-testable even when it was resized... perhaps there's something
    interesting here?
• The drag region is still hit testable when the application has been
  fullscreened; maybe we should suppress it?
• It doesn't feel incorrect; the window can be interacted with just like
  normal.
• Should we be trying to get the WM_NC events inside this window
  instead of tunneling normal events through to the parent window as NC
  events? I'm not sure on this one at all. 😄

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[beviu]

I bet there's a way to get it working without having to recreate the
window every time.

I hope too. I have not found a way to do it unfortunately.

I tried briefly to do the resizing thing, and yeah -- I was having
some Z order issues and couldn't even get the window to reliably paint
(i tried making it red so that i could see what was happening)
Weirdly, adding a WM_PAINT handler made the window totally
hit-testable even when it was resized... perhaps there's something
interesting here?

I have tried adding a WM_PAINT handler in the drag window that does:

case WM_PAINT:
{
    PAINTSTRUCT ps;
    BeginPaint(window, &ps);
    EndPaint(window, &ps);
    return 0;
}

And using MoveWindow to move the drag window in _RecreateDragBarWindow instead of recreating.

But it doesn't work.

The drag region is still hit testable when the application has been
fullscreened; maybe we should suppress it?

Fixed in c375958.

It doesn't feel incorrect; the window can be interacted with just like
normal.

Yes.

Should we be trying to get the WM_NC events inside this window
instead of tunneling normal events through to the parent window as NC
events? I'm not sure on this one at all. 😄

I think that in the drag window proc, either:

• We tunnel the normal events through to the parent window as NC
  events so that DefWindowProc will be called with the window parent HWND and will handle the dragging/resizing for us.
• We could call DefWindowProc directly with the window parent HWND instead of SendMessage.
• We could try to handle resizing/dragging ourselves by sending WM_SYSCOMMAND with SC_MOVE/SC_SIZE. It's probably what DefWindowProc already does.

But I think that in all cases, we have to tunnel something to the parent window: initially, I tried to send WM_SYSCOMMAND events with SC_MOVE to the drag bar window and what happened was that when I clicked on the drag bar, it would drag the drag bar instead of the whole window. So I made it send the message to the parent window instead. I'm no expert at this.

[beviu]

Wow so now (maybe before too?) the drag bar is broken in Release mode but works in Debug. This is bad. I don't know why.

[beviu]

BTW I'm not sure how to use the spell checker bot. I ran the command to accept the changes but it created the .github/actions/spell-check/whitelist/c3759587195744a3a2e01e2be4ee44ef1f6ab100.txt file and I don't see any file with the same weird name in that directory so I don't really understand.

[jsoref]

@greg904 The filename it suggests is not specific to this project (it's based on a git sha) -- When there's only one file, it just suggests that file, but when there are multiple, it doesn't really know which to pick.

It would be better if you picked one of the existing files. A future run of the checker (when you merge again) would include a bit better advice. You could put your changes into the whitelist file, or maybe the api file (when it arrives). Or wherever a reviewer suggests.

[DHowett-MSFT]

@greg904 I love this pull request so much that I pulled it into a testing branch over here on our repo and made a couple changes. I got it working without recreating the window, in both debug and release mode. I'm going to submit a pull request from that branch and you will, of course, get full credit on the merge. :)

[DHowett]

#5485

[beviu]

Ok so I should probably close this PR then. It's cool that you found a way to make it work. When I saw that it didn't work in Release mode I was mad 🤣, I thought it was going to be impossible to debug because I don't even understand how some parts of the code I wrote work.

[ghost]

🎉This issue was addressed in #5485, which has now been successfully released as Windows Terminal Release Candidate v0.11.1251.0 (1.0rc1).:tada:

Handy links:

• Release Notes
• Store Download