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

Document the use of ResizableLib in mpc-hc #31

Open
adipose opened this issue Jun 28, 2024 · 8 comments
Open

Document the use of ResizableLib in mpc-hc #31

adipose opened this issue Jun 28, 2024 · 8 comments

Comments

@adipose
Copy link

adipose commented Jun 28, 2024

Feel free to close this, I'm trying to document the changes we've internally made to a copy of your lib from 2014.

  1. CResizableDialog inherits from CCmdUIDialog instead of CDialog.
  2. CResizableDialog::OnSize has an extra Invalidate() call for some redraw bugs. Have to track down the cause.
  3. CResizableDialog uses __super instead of CDialog (you could do the same without causing any changes, but for us it calls the correct parent method)
  4. CResizableState ReadState and WriteState to support custom storage ids.
  5. ResizableWndState adds an ability to set the show command to SW_HIDE
  6. ResizableWndState updated to use different signature to CResizableState methods
@adipose
Copy link
Author

adipose commented Jun 28, 2024

Merging your latest to mpc-hc here: clsid2/mpc-hc#2885

Note, mpc-hc only uses about 26 files from resizablelib. I'm not sure how many of the newer files were in the original version that was adopted by mpc-hc!

@ppescher
Copy link
Owner

Thanks, I will have a look at your changes and see what can be included.
It's always nice to see this library being used in real projects!
Especially since I'm a user of MPC-HC ;-)

@adipose
Copy link
Author

adipose commented Jun 28, 2024

Thanks, I will have a look at your changes and see what can be included. It's always nice to see this library being used in real projects! Especially since I'm a user of MPC-HC ;-)

Awesome, thank you! Good to know you are a customer! I have always thought resizelib was an old abandoned project so it's nice to meet you.

@adipose
Copy link
Author

adipose commented Jun 28, 2024

CCmdUIDialog derives from CDialog. It has the following changes:

DefWindowProc sends a single message WM_KICKIDLE during WM_INITDIALOG

WM_KICKIDLE handler added, which calls UpdateDialogControls(this, false); (see https://deweymao.github.io/c/c++/2018/02/24/wm_kickidle_for_updating_mfc_dialog_controls.html for an explanation of the basic handler)

OnInitMenuPopup that seems identical to the one suggested here:

https://learn.microsoft.com/en-us/troubleshoot/developer/visualstudio/cpp/libraries/cannot-change-state-menu-item

This particular code seems like it's essentially fixing a bug/shortcoming of CDialog and menus.

Possible solutions for mpc-hc:

  1. Implement CResizableCMDUIDialog the same way you implemented CResizableDialog but deriving from CCmdUIDialog
  2. Derive CResizableCMDUIDialog from CResizableDialog and add the same features from CCmdUIDialog
  3. You add CCmdUIDialog or equivalent to ResizeLib and create a CResizableCMDUIDialog
  4. We continue to merge your code to ours but change CDialog to CResizableDialog. Preferably if you could change CDialog:: to __super:: it makes this merging easier.

@adipose
Copy link
Author

adipose commented Jul 11, 2024

@ppescher

Hi, I have another question. Would you consider adding #ifdefs to the codebase that identify MPC-HC specific code? It might be easier, specifically for CCmdUIDialog. Although...I do think CCmdUIDialog is a natural upgrade to CDialog so including it in your project wouldn't be a terrible idea, either!

@ppescher
Copy link
Owner

Possible solutions for mpc-hc:

1. Implement `CResizableCMDUIDialog` the same way you implemented `CResizableDialog` but deriving from `CCmdUIDialog`

2. Derive `CResizableCMDUIDialog` from `CResizableDialog` and add the same features from `CCmdUIDialog`

3. You add `CCmdUIDialog` or equivalent to ResizeLib and create a `CResizableCMDUIDialog`

4. We continue to merge your code to ours but change `CDialog` to `CResizableDialog`.  Preferably if you could change `CDialog::` to `__super::` it makes this merging easier.

Could you not change CCmdUIDialog to derive from CResizableDialog? Does it break things?

@adipose
Copy link
Author

adipose commented Jul 11, 2024

Possible solutions for mpc-hc:

1. Implement `CResizableCMDUIDialog` the same way you implemented `CResizableDialog` but deriving from `CCmdUIDialog`

2. Derive `CResizableCMDUIDialog` from `CResizableDialog` and add the same features from `CCmdUIDialog`

3. You add `CCmdUIDialog` or equivalent to ResizeLib and create a `CResizableCMDUIDialog`

4. We continue to merge your code to ours but change `CDialog` to `CResizableDialog`.  Preferably if you could change `CDialog::` to `__super::` it makes this merging easier.

Could you not change CCmdUIDialog to derive from CResizableDialog? Does it break things?

I probably could. The trick would be if we have any non-resizing cases that derive from it. I count one, the CFavoriteAddDlg, which perhaps I could make be resizable.

@adipose
Copy link
Author

adipose commented Sep 9, 2024

@ppescher , I did try deriving classes from CResizableDialog. The result was they became resizable, where previously they had not been. So at that point the issue becomes: they resize but are not designed to resize, so it just increases the window size without moving widgets anywhere.

I'm not sure if the library supports the concept of something that derives from CResizableDialog without resizing...not sure there is much point, other than code reuse.

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

No branches or pull requests

2 participants