-
Notifications
You must be signed in to change notification settings - Fork 28
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
allow initial state SW_HIDE #32
Conversation
I cannot reproduce a hidden dialog, even if I unconditionally set |
See clsid2/mpc-hc@4dbe739 for how the code used to be.
We would create the window and immediately hide it. But the resizelib code would show it again, I believe. |
Here is why I had done it originally. The |
I don't think it does, I tried this pattern under the debugger:
|
We want to restore the state, so that when we do show the dialog, it has the same size as the last run. |
There is a way to initially hide the dialog (properly), but we weren't using it. https://stackoverflow.com/questions/39771342/show-modelless-dialog-initially-hidden |
If the dialog is not initially hidden, the library code does not change it (makes it visible when it already is). |
Overriding So I think there is no need to change ResizableLib. |
Here is what was happening:
This in turn calls Because we want to load "rectangle only," it activates the code in
I believe we were calling the SW_HIDE before because it was forcing it to pop up during init. My change allowed us to force it to hide, even if we use the "rectange only" code. All of this happens inside the |
Not the only way! The way it works today is I do not experience initial flash under today's mpc-hc. I used to when we had the extra hide command. |
I just reproduced the base case with a simple MFC dialog project. Note, if we do not call the SW_SHOW, the new dialog we just created does not display. If, however, we derive this from a resizablelib class, and have it "restore" the size, it will automatically show, unless we go to extra efforts to stop it, like overriding
|
Never needed this kind of tricks. Could initial flash be prevented with |
The flash is simply because it shows it against our wishes and we have to hide it after. The rectonly flag forces it to show and there is no way to tell it otherwise. We can intercept the call and stop it, but that is a bit inefficient when you could just pass the correct hide flag when that's what you want. |
Too late time caused misreading MS docs; no show flags were in description of other method. However, your test code raises questions.
What prevents use of the clear and simple way with removing WS_VISIBLE flag in resource file or in window class registration? |
|
OK, so I have a complete example here of the problem in a simple class.
Note, this is the main dialog of the app:
The |
Github says this code is about 20 year old:
|
I think I misunderstood when I calculated 2014. The copyright said 2015 when that code arrived to mpc-hc, but prior to that, mpc-hc was actually using a version with a copyright of 2002. And when that code was merged in 2018, it appears the SW_SHOWNORMAL was swapped for SW_HIDE. |
Yes, the code base is that old... I think the first release I published was back in 2000 and it was only CReziableDialog. Later on it became ResizableLib. Anyway, back to the topic... I will try to reproduce the issue and see if I come up with a nice solution. There is no problem for me to add some parameter, together with |
The line appeared in 1.4a, and upgrading from 1.3 was non-trivial, but all settled down in the end.
Maybe complete on your disk, but others have to recreate while not knowing what is the goal. And I still would love to get an answer for this:
A protected member for changing restored visibility in one very specific case? |
I forgot this thread was on a PR ...
I'm not sure removing
I agree that the protected member is kind of "inelegant". I mean either add an additional function argument after I just took the ResizableDialog demo and converted the first dialog to non-modal, but showing it only after the second one is dismissed:
And also add this to close the app when the non-modal dialog is closed:
I tried the proposed fix, I tried overriding I can succeed in hiding the dialog only by overriding I think there must be some stupid side-effect which is re-setting the visibility of the dialog during the creation phase. But to confirm that I would have to try to make it non-resizable first and see the "normal" behavior. |
Actually I was inaccurate... If I don't call But, if I call it, I have to force |
I said I had a complete example not that I uploaded it ;) Anyway it's just a standard dialog based MFC project generated by visual studio. Then I added resizablelib, added a resizable derived dialog class, set it to enablesaverestore, and added it to the main dialog class as member. I can upload the whole project but it's literally just enablesaverestore + create that makes it show. I can upload the whole project if you like but maybe a mod to the demo dialog is more useful? |
I think I observed before that showCmd=0 just shows it. My notes above had some errors about how the code used to be. But I do recall I had to explicitly add the hide rather than ignore showCmd. Edit: can't be, 0=HIDE. |
It seems if I postpone the call to But I think it's better if I make a PR here and let you test this solution first. As of now So far so good, and this solution actually makes sense to me, but I want to test it a bit more before I place a commit in the master branch. |
So a hidden window, I guess, will always get |
After reading the code more, I think I understand the motivation between connecting the show command to the The problem, then, is that the "default" of It occurs to me that a messy workaround would be to alter the data of the state, set the showCmd to whatever you need it to be, and then avoid |
So there are 2 issues here:
The proposed solution handles the first case. |
Maybe this is the root of issues. Dialog usage commonly follows the pattern: create - show - destroy. The existing implementation of Save and Load routines does not literally execute what the name suggests, because values are changed while saving and while loading (MS have added its own part too). |
In fact My proposal does not change the current meaning of that function, which is to "enable" the automatic save/restore. Only the restore phase would be moved to a more suitable place, that is the first time the window is made visible. This change should not break anything. |
My view on number 2 is it is mostly a non-issue. The visibility of the window itself may or may not be something the application wants to store/restore. For mpc-hc, for example, the dialog(s) in question here would always start hidden, regardless of the last run of the application. We have other dialogs that we do remember visibility, and we handle those with other internal settings. Even if resizablelib had the facility to restore that visibility, how can it be sure when is the right time to do so? Simply because the dialog is created and initialized, doesn't necessarily mean it is time to pop up. If such a need really exists, I would suggest a call such as |
... and most probably never ever shown. Why create at all? |
The reasons you mentioned before, there was a design choice where things are connected to the dialog, shown or otherwise. I was not involved in those choices but it's possible to re-design, of course. The ability to show and hide windows was clearly a design choice by microsoft with a variety of advantages and conveniences, which I don't pretend to be an expert on. The ability to create a window and show it later is also considered a valid use case. So, it's reasonable that resizeablelib would support it, even though you are right that mpc-hc could have designed around it. |
I also think it is perfectly fine to create/initialize a window and then show it later. There might be many reasons for doing it that way. And I agree that this library should not alter the normal behavior of windows and dialogs, just because you made them resizable. |
There would be nothing to discuss should there be no difference between a dialog and a vanilla window, and MS would not have to follow Apple's example and create distinct API calls.
Sometimes people choose non-optimal paths or do things just for fun. Then their reasons might poorly align with the library design goals, and you have to make updates. :) |
I created a new branch and PR #35 with the proposed solution and updated demo project I used for testing. I would appreciate if you could test this too in your own projects. Thank you |
In testing, it seems to solve all the mpc-hc issues around window visibility. I removed our custom code and this seems to work. |
First proposed patch. This is used to allow resizelib to work with an initially hidden dialog.
In
20142002,wp.showCmd = SW_SHOWNORMAL
did not exist, but it still showed the dialog initially.In order to work with latest code, I put that into an else-block so it doesn't override
SW_HIDE
.