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

Support child windows in Windows #2042

Closed
wants to merge 26 commits into from

Conversation

sjoshid
Copy link
Contributor

@sjoshid sjoshid commented Nov 14, 2021

This PR adds support for child (with type WS_CHILD) windows in Windows.
Main difference between a tooltip window and child window is that the child:

  • Moves along with the parent and,
  • Cannot leave parent's client area.

ChildWindow

UPDATE

After discussion on Zulip, it was decided to let dropdowns leave parent boundary.

SubWindow

@jneem
Copy link
Collaborator

jneem commented Nov 14, 2021

Sounds good! I'm a bit confused about the comment though (and please bear in mind that I know almost nothing about the win32 API).

  • which part of this PR deals with the child-following-parent behavior? Is that something automatic when you use WS_CLIPCHILDREN?
  • what do you mean by "cannot leave the parent's client area," and where is that implemented? Is the dropdown allowed to "stick out" past the edge of the parent window? What happens if the dropdown is bigger than the parent?

I know at some point we discussed having children follow parents, but I don't remember where it was or what the outcome of the discussion was. But it's a decision that should be made intentionally and then documented. Right now, the only documented difference between WindowLevel::Tooltip and WindowLevel::DropDown is that they have different z-order. That makes me reluctant to introduce platform-specific changes without at least thinking about the cross-platform story.

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

which part of this PR deals with the child-following-parent behavior? Is that something automatic when you use WS_CLIPCHILDREN?

No. It's the WS_CHILD flag that is passed when creating the sub/child window. Windows created with this flag are only visible in parent's client and they always follow their parent.

The only thing that sticks out in this PR is the flag WS_CLIPCHILDREN. In Windows, a paint message (specifically WM_PAINT) is generated in response to update regions. If parent/child update regions overlap, then a paint message is sent to parent first and then to all its children allowing children to re-paint themselves in case parent painted on top of them in first paint message. This flag basically tells Windows NOT to send us the second paint message. And in druid land this makes sense because if child needs to be painted/re-painted, we invalidate it separately which in turn would generate child specific WM_PAINT messages. Paint message for child will NOT generate another paint message for its parent.

TBH all this information is dispersed. But the meat of it is here.

what do you mean by "cannot leave the parent's client area," and where is that implemented? Is the dropdown allowed to "stick out" past the edge of the parent window? What happens if the dropdown is bigger than the parent?

All of the heavy lifting is does by Windows. Children are clipped if they do stick out.
image

That makes me reluctant to introduce platform-specific changes without at least thinking about the cross-platform story.

That is good point. I honestly dont know how children are implemented in other platforms. Or if that concept even exists. That's why this PR has only Windows specific changes. I think it is fine to introduce something that is platform specific without changing shell API.

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

I also forgot to include documentation for this new sub-window. But now I guess that'll depend on whether the the rationale behind this PR is accepted.

@jneem
Copy link
Collaborator

jneem commented Nov 15, 2021

I see, thanks for the explanation.

The mental model I have for a "window" (based on linux, and I think it also applies to mac) is that it's basically a region that handles events and draws itself. The OS/window manager/compositor is in charge of positioning the windows, which includes giving them a z-order and figuring out which parts of which windows are visible. This makes WS_CLIPCHILDREN pretty confusing to me, because it goes against this mental model: I expect the parent to draw itself, the child to draw itself, and the OS to be in charge of presenting those drawings on screen.

Anyway, so my main opinion here (and I'm happy to get push-back, also cc'ing @cmyr who may have thoughts) is that we do not want to clip the children when they stick out: as far as I understood it (and bearing in mind that I'm working in the mental model above, which might not be the best one) the entire point of spawning new windows was to allow them to stick out if they're too big. For example, if we only wanted druid to support tooltips that stay in the main window, we'd just use a druid widget instead of bothering with all the portability issues that platform windows bring.

I have fewer opinions about the child-window-follows-parent thing, mainly just that whatever we decide on should be consistent across platforms and documented.

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

This makes WS_CLIPCHILDREN pretty confusing to me

WS_CLIPCHILDREN is a window style and window styles are used abundantly in win32 platform code.

Every platform is different. If it's easier to do thing X in one platform should we not do it because it is unavailable in others? Should we ignore it for the sake of platform consistencies?
I would like to emphasize that we can take up a platform specific feature iff it doesnt break shell API contract.

FYI, Im not trying to be difficult. Dont get me wrong. I truly appreciate all your responses

@cmyr
Copy link
Member

cmyr commented Nov 15, 2021

I am also confused by the "child cannot leave parent's client area". If this is intended to be used mainly for things like combo-boxes, I would expect that the child would, in some circumstances, need to draw outside of the parent's frame. Is this not a convention on windows?

@jneem
Copy link
Collaborator

jneem commented Nov 15, 2021

This makes WS_CLIPCHILDREN pretty confusing to me

WS_CLIPCHILDREN is a window style and window styles are used abundantly in win32 platform code.

Sorry, I didn't express myself clearly. I wasn't trying to say that anything is wrong with WS_CLIPCHILDREN. I was just trying to explain why I need so much help understanding this: because it doesn't match my internal mental model.

Also, I appreciate your responses and I don't think you're being difficult at all. In fact, I think this sort of discussion is exactly what we need in order to have druid-shell behave sanely across platforms.

Anyway, I think the key here is to identify:

  1. what kind of behavior can be implemented (glitch-free and with reasonable performance) across platforms, and
  2. what is the use-case in druid.

My concern about windows-leaving-the-parent-rect was motivated by point 2, because my understanding behind the motivation for child windows was that sometimes we want dropdowns or tooltips or dialogs to be able to extend past the parent. For example, here is how firefox behaves on my system if it has a very small window:
Screenshot_2021-11-14_22-55-13

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

@cmyr

I am also confused by the "child cannot leave parent's client area". If this is intended to be used mainly for things like combo-boxes, I would expect that the child would, in some circumstances, need to draw outside of the parent's frame. Is this not a convention on windows?

What Im trying to say is if a window might lay completely/partially outside parent, then it is tooltip. Not a child.
This PR allows users to pick what behavior they want. If they want a window to not cross parent boundary AND move along with parent then they have to use WS_CHILD. If not, they can use tooltip (ie a WS_POPUP).

It falls down to how end users utilize these windows.
In Intellij (Ctrl + Q) shows me a tooltip. But it would have been better if it was a child so it doesnt obscure the app behind it.
IntellijSubWindow

@jneem
That is an instance of a tooltip. Firefox also has a child instance. Eg:
image

Suggestions dropdown does not cross parent. It must be a child. They very well could have made it a tooltip.
Again, it goes back to what end users want.

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

All this also raises another question: can we not have a tooltip that follows the parent when it is dragged? This will remove the need to have a WS_CHILD. But I think redrawing/re-painting_ wont be as smooth. I might be wrong.

Another question: what about modals? They definitely should move with parent and lay within parent. So they cannot be tooltips.

@jneem
Copy link
Collaborator

jneem commented Nov 15, 2021

Thanks for the example, that definitely helps me. (I do think there is some possibility for cross-platform terminology issues, because at least the way that the word "tooltip" is used in WindowLevel -- which I think was inspired by the mac terminology -- it only refers to the z-order, and not to any other aspect of the window behavior.)

So as far as I understand it:

  1. there is a use-case for things that are restricted by the parent window. (I hesitate to use the word "clipped" because I think the real desired behavior involves them being aware of the size constraints and adapting appropriately.)
  2. there is a use-case for things that are not restricted by the parent window.

In both cases, it may be desirable to have them follow the parent when dragged, but maybe in the second case it can't be made smooth on Windows?

I agree that it would be nice for modal dialogs to move with the parent, although I'm not sure about them being restricted to it. Here's an example of a file dialog that doesn't fit in my firefox window, and I think I like the fact that it doesn't get constrained or clipped: Screenshot_2021-11-15_12-14-50

Anyway, my main question is: for case (1) above, is it necessary to have them backed by a child window from the operating system? Like, if I want a dropdown-like widget in druid and I want it to always remain within the bounds of the parent window, couldn't I just implement it as a widget?

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 15, 2021

for case (1) above, is it necessary to have them backed by a child window from the operating system? Like, if I want a dropdown-like widget in druid and I want it to always remain within the bounds of the parent window, couldn't I just implement it as a widget?

I guess you can. I say guess because I dont know if z-order indexing is implemented in druid today. And also it will get complicated real quick (not saying we shouldnt do something just because it is complicated). Like you said (and Manmeet) in Zulip topic:
image

Would that complex logic not apply to your widget dropdown?

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 16, 2021

Just out of curiosity, I did a search on child windows in gtk and macos.
I didnt find much in gtk at all. Leading me to believe it either doesnt exists or is difficult to implement.
But in macos there is such concept. And from the description it is exactly what Windows has.

So, for gtk, we might have to this the widget way like you are suggesting. And if it done with widgets, then it will be platform independent which will address your platform consistency concern.

Is that the point you are trying to make?

ForLoveOfCats and others added 2 commits November 18, 2021 11:26
This completes the `Code` parsing and also adds F13 - F24 to the `Key`
parsing code.
@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 21, 2021

@cmyr what are you thoughts on this?
There have been many attempts in the past but what I want to ask you is not addressed anywhere.

There are two categories of sub windows. Ones that are unbounded (maybe drawn outside parent) and ones that are bounded (lie wholly within parent). Context menus are a good example. Here's example from Firefox and VSCode respectively:

image
image

Two things to keep in mind in VSCode image:

  1. VSCode is painting context on the left because there isnt enough space on right. (Normally it is painted right)
  2. The context follows the parent (like in issue description). I havent attached a gif but trust me on this.

Should druid support both types of contexts? I ask because if you say yes, then we'd have to support two types of sub-windows. Eg., WindowLevel::bounded::Dropdown and WindowLevel::unbounded::Dropdown.

@CryZe
Copy link
Contributor

CryZe commented Nov 22, 2021

VSCode doesn't actually do any of this intentionally. They are highly limited by electron / the web model that doesn't allow them to have proper child windows / drop downs, so they have to bind it to the parent window. This isn't intentional.

@sjoshid
Copy link
Contributor Author

sjoshid commented Nov 22, 2021

Users requesting something similar is a fair request.

* Replace test-env-log by test-log

The crate was renamed and the latest release of test-env-log now raises
deprecation warnings.

* Upgrade tracing-subscriber
zhiburt and others added 6 commits November 26, 2021 09:19
* Expand `log_to_console` doc comment

Note that it  panics when a global logger was already set

* Reduce repetition.

(as suggested by @PoignardAzur)

Co-authored-by: jneem <[email protected]>
- x11::Application::xkb_context is not needed because we create keymap
  at start of Application::run()

- x11::clipload: time and selection atom are not needed for INCR
@sjoshid sjoshid mentioned this pull request Dec 13, 2021
@sjoshid
Copy link
Contributor Author

sjoshid commented Dec 13, 2021

I some how managed to F this branch up. :(
Please see #2087.

@sjoshid sjoshid closed this Dec 13, 2021
@jneem jneem mentioned this pull request Dec 16, 2022
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.