wxSmith Feature request: Option to disable calls to Fit() and SetSizeHints()

[lesnewell]

I use wxSmith to generate layouts but often want to add extra controls or hide controls outside wxSmith's automatically generated code. The problem I have is that for every sizer wxSmith adds this code:   
   theSizer->Fit(theWindow);
   theSizer->SetSizeHints(theWindow);

This locks window sizes causing layout issues if you later hide controls or add extra controls. I am currently manually deleting these calls but if I make any changes to the layout wxSmith puts them back. It's easy to forget to go back and edit the code. It would be really handy to have an option to not put these calls in.

As an aside, is there any need to call Fit and SetSizeHints for every sizer?. None of the wxWidgets samples do this. They only do it for the top level window. Looking at the changelog it appears these calls were added back in 2005 but there is no explanation of why.

[Miguel Gimenez]

You can remove that code from src\plugins\contrib\wxSmith\wxwidgets\wxscontainer.cpp (lines 214 to 217) and recompile C::B

[lesnewell]

Thanks. Yes, I saw that code. The problem is that taking it out would affect all windows in my application. I don't add/hide controls on all windows. Making it optional for a window gives the choice.
I'm sure I'm not the only one caught by this. The big problem is that when you do add/remove controls it is very unpredictable what will happen. Sometimes it works, other times it does not. It took me an embarassingly long to figure out why my layouts sometimes went wrong in strange ways. Without those calls the windows lay out predictably.

[oBFusCATed]

Do you want to have control if wxsmith would generate the fit/layout for the top level windows?
Probably this was done this way because of misunderstanding of wx or wx-bug which are no longer present.
I doubt we'll know the exact reason, so I think we should try to modify wxsmith to generate idiomatic wx style code and see what happens.

I suppose we could separate the fit code in a separate section, so the user can just delete it or comment it out.
Something like this:

Code

// * wxsmith
main code
// * wxsmith

// *wxsmith-layout
the layout code
// *wxsmith-layout

With this scheme the user would be able to decide on a case by case basis if he/she wants the layout code.

[lesnewell]

Do you want to have control if wxsmith would generate the fit/layout for the top level windows?

Ideally. For example I quite often create panels in wxSmith which are then used in multiple windows. Calling Fit/Layout in them is redundant.

I suppose we could separate the fit code in a separate section, so the user can just delete it or comment it out.

That sounds like a good idea. How would it cope with existing code? Currently all of the creation code is marked with
//(*Initialize(myClass)
//*)

To be honest I'd assumed such an option would be a check box in  the top level window's options in the wxSmith designer. I prefer your suggestion though.

[oBFusCATed]

I'm not sure the top level window has any properties anywhere.
I'm not too versed in wxSmith's internals, so I might miss something.

Preserving compatibility would be troublesome, unfortunately, you're correct.
Probably we'll have to use something like "// InitializeNoLayout(myClass)" and this would work for new code only or if the user manually changes from "//* Initialize(myClass)" to "//* IntializeNoLayout(myClass)"

This way it might not require adding "// InitializeLayout(myClass)".
Also I'm not really sure which is better to do.
Someone using wxSmith regularly should do some experimentation with both and tell us the results.

Other ideas are welcome of course and mostly patches, the probability for me finding time to work on this is close to 0!
But I can find time to review and apply patches.

[lesnewell]

My knowledge of the internals of wxSmith is pretty much zero but I'll take a look. I downloaded the CodeBlocks source but for some reason my built executable won't run. I get 'This application was unable to start correctly (0xc000007b)'. As far as I can figure out it appears to be a 32 bit/64 bit mixup somewhere but I have yet to figure out where.

[oBFusCATed]

You have to run update31.bat or update31_64.bat.
Also you have to copy the wxwidgets dll in the correct folder.

[lesnewell]

Thanks. I'll give that a try tomorrow.

I think I'll look into where options like 'use parent from argument' are implemented. Adding a 'do not lay out' check box there may be a better bet.

[lesnewell]

The running problem was libstdc++-6.dll. A lot of applications install this file (The CodeBlocks + minGW package installs 3 copies!) and I had a mix of 32 bit and 64 bit versions on the path. CodeBlocks was picking the wrong one. I don't normally use MinGW so I haven't come across the problem before.

As expected simply removing Fit+Layout from wxscontainer.cpp messes up windows that don't have Fit + Layout elsewhere in the code.
From a quick glance at the wxSmith code, I think adding a top level option shouldn't be too bad. What are your thoughts on panels? In my opinion panels created in wxSmith should never have Fit and Layout because they are always going to be contained by another window.
 

[oBFusCATed]

The major problem with wxSmith is that there is no button "regenerate all ui in the project/workspace", so it is hard to change such changes to the code-gen.
I'm pretty sure this would break things for existing users, but I'm not sure if it is a concern.
If we can generate more idiomatic wxwidgets code and it works as expected for new projects then I'm fine to change it.
The changes to the wxsmith code would be visible in the diffs, so people could review them and either fix them or report them.

The second point would require us to have a clear explanation why having Fit+Layout for panels is detrimental.
Do you have an example or link to documentation?

[ollydbg]

My knowledge of the internals of wxSmith is pretty much zero but I'll take a look. I downloaded the CodeBlocks source but for some reason my built executable won't run. I get 'This application was unable to start correctly (0xc000007b)'. As far as I can figure out it appears to be a 32 bit/64 bit mixup somewhere but I have yet to figure out where.

Told us how you build the C::B(compiler, wx version and steps), I think we can help you to solve this issue.

[lesnewell]

I dug into the code and added a 'Lay out the window' property to wxFrames and wxDialogs (maybe needs a more descriptive name). If this is checked only one SetSizeHints() or Layout() call is generated for the top level sizer. If it is not checked there is no SetSizeHints() or Layout().
wxPanels never have SetSizeHints() or Layout() because they cannot be top level windows.

I'm pretty sure this would break things for existing users, but I'm not sure if it is a concern.

As far as I am concerned, breaking existing projects is not acceptable. I'm testing on an existing project to make sure it doesn't. If 'Lay out the window' is enabled (the default) windows should behave pretty much the same as they do now. The generated code will change slightly when you open the wxs file but that often happens when a new feature is added to wxSmith.

The second point would require us to have a clear explanation why having Fit+Layout for panels is detrimental.
Do you have an example or link to documentation?

wxSizer::SetSizeHints https://docs.wxwidgets.org/3.0/classwx_sizer.html#abc460cd0e2bb3bde72142fdb434bc546 lays out the window then fixes the window's minimum and maximum sizes based on the current layout. To quote the wxWidgets docs 'This only makes sense when window is actually a wxTopLevelWindow such as a wxFrame or a wxDialog'.

Currently SetSizeHints is called for every window containing a sizer. This effectively fixes the size of every window.  While not recommended, this is not a problem until you change the contents of any child windows. Now when you call Layout() on the top level sizer it cannot resize child windows to fit their updated control layout.

Panels by definition are always child windows so should never use SetSizeHints.

[lesnewell]

Here is a patch. I tested it on a pretty big project with about 60 windows managed by wxSmith. Some windows have layout disabled. The generated code all looks correct. I haven't tested every window in the application but every one I have tested so far looks as it should.

[lesnewell]

Told us how you build the C::B(compiler, wx version and steps), I think we can help you to solve this issue.

I followed the instructions here http://wiki.codeblocks.org/index.php/Installing_Code::Blocks_from_source_on_Windows. I Installed the CodeBlocks 20.03 with MinGW package and used the recommended wx3.1.4 source. Using 64 bit build.

As I mentioned in a previous post the issue is that there are loads of copies of libstdc++-6.dll on my machine, both 32 bit and 64 bit. I have many open source applications installed and apparently many were built with MinGW. CodeBlocks even installs 3 copies.

I am currently getting around the problem by running CodeBlocks from a command prompt with PATH just set to point to MinGW and the wx dlls.

[BlueHazzard]

As I mentioned in a previous post the issue is that there are loads of copies of libstdc++-6.dll on my machine, both 32 bit and 64 bit. I have many open source applications installed and apparently many were built with MinGW. CodeBlocks even installs 3 copies.

I am currently getting around the problem by running CodeBlocks from a command prompt with PATH just set to point to MinGW and the wx dlls.

generally it is a bad idea to have all this in path...
I keep my compiler instillations all out of path.... If i need a compierl i use a bat file to set up the environment...

[lesnewell]

After more testing I still haven't found any cases where this patch breaks existing projects and layouts. Should I submit it as a ticket?

[oBFusCATed]

Yep, I'm trying to find some time to push it. I've taken a brief look, but that is it.
The biggest problem for me is that the default seems wrong and the name of the field doesn't have a tooltip with a better explanation.

[lesnewell]

I know the time problem very well.

As far as I know none of the options have a tool tip. At least I don't see them here. Adding tool tip functionality to the framework is likely to be quite a bit of work.
Maybe it's worth renaming the option. If you have any suggestions for a better name I'll happily change it. Admittedly that would break the projects I've used this on but I'mm sure I can get around that with some judicious wxs file editing.


'Lay out window' should default to enabled because we normally want the window to be laid out automatically. This also maintains compatibility with older projects.

[oBFusCATed]

Are you removing the code for the per sizer fit/layout calls? Is this unconditional?

[lesnewell]

Yes it unconditionally removes the calls per sizer. I looked back at the wx2.8.11 samples and they only call SetSizeHints once for the top level window so I can't see it being needed for backwards compatibility with old versions of wx.

[oBFusCATed]

We don't care for wx2.x in wxSmith as far as I'm concerned.

[lesnewell]

The point I was trying to make is that the recommended use for SetSizeHints does not appear to have changed for a long time. If the current behaviour was put in due to a bug in wx I guess it was probably fixed a long time ago.

[oBFusCATed]

I'm testing this patch.
Am I supposed to see this new property for wxFrames?
Are they supposed to be laid out?

I think, I've implemented the tooltip stuff, but I have to find where is the property to test it.

[oBFusCATed]

OK, I'm not sure if this is a problem or side effect.
I have a wxFrame test project.
If I use the code generated by the patched cb the min size of the window is not set, so I can shrink it smaller then the content.
I have "Default min size" checked.
I'm always confused by this min size stuff, so I don't know if this is a regression or not but it is a change in behaviour.

Any comments about it? Do you have wxFrame based windows in your app?

[oBFusCATed]

Ah it seems you're not using flTopLevel, so the property is never visible...

[oBFusCATed]

And my simple change to add tooltips works fine.
So I'll need a good explanation what this property does before pushing this fix.

[lesnewell]

Ah it seems you're not using flTopLevel, so the property is never visible...

Huh. I just checked the patch and it looks like I messed up somewhere. There are some files missing from the patch. That explains why the property was missing from your frame.

I'll have another go tomorrow.

If I use the code generated by the patched cb the min size of the window is not set, so I can shrink it smaller then the content.
I have "Default min size" checked.

SetSizeHints should automatically set min and max sizes. Is SetSizeHints being called?

[lesnewell]

If I use the code generated by the patched cb the min size of the window is not set, so I can shrink it smaller then the content.

I just tested a frame and with 'Lay out the window' turned off you can resize the frame to any size you want. With it on (the default) you can't resize below the minimum size that fits all controls. That is the expected behaviour.

I have "Default min size" checked.
I'm always confused by this min size stuff, so I don't know if this is a regression or not but it is a change in behaviour.

Size sets the initial size of the window, overriding the layout system.
Min size sets the minimum size of the window. This will override the layout system if the layout could be smaller.
Max size sets the maximum size of the window. This will override the layout system if the layout should be bigger.
Manually setting these sizes is really not a good idea in most cases. SetSizeHints sets these automatically, based on the ideal minimum and maximum sizes reported by the layout system.

I just ran some tests and noticed a bug that has been around for a very long time. As SetSizeHints was called for every sizer, it overrode whatever sizes you entered. This probably explains why you found them confusing. They actually did nothing. I've also been caught out by this a few times.

With the attached patch, size, min size and max size should work correctly. For top level windows with 'Lay out window' selected it gets a little complicated.

• If size, min and max are set to default, SetSizeHints() is used. The layout system is free to decide the optimum window layout and sizes.
• If size is default but min and/or max are not, Fit() is used. This sets size to the optimum decided by the layout system while still obeying min and max size. Depending on min/max size the layout may not fit.
• If size is not default Layout() is used. This makes the layout system try to fit to your defined size. The layout probably will not fit.

The attached patch has these changes:

• The above changes depending on what defaults are used
• The missing files
• Fixed 'Lay out the window' selection not being saved correctly.
• Fixed SetSizer() sometimes being called twice on top level windows

[oBFusCATed]

Thanks, it seems to work.

Now I need a couple of things to finish this of:
1. Descriptive commit message. I can gather one from the topic, but I'd be happier if you can provide one.
2. A real name or nickname. I like to give credit to people who provide patches and svn doesn't allow separation between author and committer name, so I write it down in the commit message.
3. A description for the tooltip for the "Lay out the window" property.

Just a note: You've messed up the load/save of the layout property, but I've fixed it. Re using parent_arg wasn't a good idea.

[lesnewell]

1. Descriptive commit message. I can gather one from the topic, but I'd be happier if you can provide one.

Changes to wxSmith code generation:
Stop SetSizeHints() being used for every window containing a sizer and make laying out the window optional.

2. A real name or nickname. I like to give credit to people who provide patches and svn doesn't allow separation between author and committer name, so I write it down in the commit message.

Les Newell

3. A description for the tooltip for the "Lay out the window" property.

Automatically lay out the window. If disabled you must call Layout(), Fit() or SetSizeHints() in your code.

Just a note: You've messed up the load/save of the layout property, but I've fixed it. Re using parent_arg wasn't a good idea.

Yeah, copy and paste isn't always your friend. The last patch fixes this. If you changed it yourself, please use the name 'layout_window' so it doesn't break a project I'm already using this on.

[oBFusCATed]

Your explanations are too brief for my liking.

[lesnewell]

Ok,  how about this for the commit message
Changes to wxSmith code generation:
Stop SetSizeHints() being used for every window containing a sizer. The wxWidgets documentation recommends SetSizeHints to only be used on top level windows https://docs.wxwidgets.org/3.0/classwx_sizer.html#abc460cd0e2bb3bde72142fdb434bc546
Added 'Lay out window' option for top level windows. If enabled (the default) the wxWidgets layout sytem is called and the window size is set using SetSizeHints(), Fit() or Layout() as appropriate. If disabled no calls are made to the layout system, allowing extra controls to be added/removed outside of wxSmith managed code. Note: after adding,removing or hiding controls you need to call SetSizeHints, Fit() or Layout().

I'm not sure if the tool tip should be much longer. IMHO tool tips should be fairly short. It's likely that this function is only of much use to people who have a reasonable understanding of how windows are laid out. In that case the tip is probably enough as it is.

By the way are you adding tips for all of the other options?

[oBFusCATed]

Thanks this is a lot better.

The changes I plan to commit are in this branch: https://github.com/obfuscated/codeblocks_sf/tree/wxsmith/layout
Please test and verify I'm not missing something.

I have no time or knowledge to add tooltips for all properties, but this is a start to show how it is done.
Patches welcome. Ideally we should add for all of them.

[lesnewell]

I just looked at the code, built and tested it. Everything looks as I would expect. Thanks.

I have no time or knowledge to add tooltips for all properties, but this is a start to show how it is done.
Patches welcome. Ideally we should add for all of them.

Yes, that's quite a bit of work. That's why I wan't to keen on adding tool tips.

[oBFusCATed]

I'll be happy if someone more familiar with wxsmith usage and wxwidgets could provide tooltips for the more often used classes.
The properties related to expand/shape/properties and similar are really confusing and are worth documenting.
Same for the minimal/max/default sizes.
Could you do it now that we have a way to do it?

[lesnewell]

To be honest I'm pretty tied up at the moment. I'm already working flat out on a number of projects for various customers.