drowe67 / freedv-gui

GUI Application for FreeDV – open source digital voice for HF radio
https://freedv.org/
GNU Lesser General Public License v2.1
206 stars 52 forks source link

undefined layouts when re-sizing main window on Ubuntu Linux #133

Closed drowe67 closed 3 years ago

drowe67 commented 3 years ago

While testing 304de7875 I noticed an odd screen layout:

Screenshot from 2021-05-25 07-19-44

It can also be reduced in size right down to zero. Quite a few GTK warnings being issued in stderr.

@tmiw - 607b827 seems OK, so this issue might have appeared with #121?

tmiw commented 3 years ago

@drowe67, oh yeah, I switched the sizers used by the left and right panels to allow them to take horizontal space when the display resolution isn't tall enough to allow the "normal" UI. The default window size for the main window should allow it to still look the same otherwise, though.

Anyway, maybe some sort of minimum size should still be enforced and not allow resizing down to zero?

drowe67 commented 3 years ago

Yes it seemed to make a good choice of moving some buttons to the RHS with a "wide" window. However after re-sizing a few times I found many "illegal" arrangements, for example the one above has a zero size center panel, and reducing the entire Window down to zero size.

I guess this could be handled by setting a minimum for the outside window, or perhaps a minimum for other GUI elements.

tmiw commented 3 years ago

Apparently the GTK warnings are "normal":

http://wxwidgets.10942.n7.nabble.com/GTK-warning-and-critical-error-overkill-td95569.html https://stackoverflow.com/questions/61486307/supressing-warnings-in-production-for-wxwidgets-application

(among other sources)

The justification seems to be that a) they don't seem to impact execution and b) users don't typically start GUI apps from the terminal. If there aren't any issues with FreeDV other than the warnings on the console, we probably don't need to suppress them, especially since I haven't been able to get a minimum size to stick for the various plots (nor does the minimum size behavior for the main window do 100% of what we'd like it to do).

drowe67 commented 3 years ago

Yes I agree it's just a minor annoyance to have the warnings, a bit like ALSA. I guess we could filter them using pipes and grep if we really needed to, e.g. if they were obscuring other warnings.

As a fall back, the previous behaivour like https://github.com/drowe67/freedv-gui/commit/607b827ef5d8fc789f500c0b9ebe5aa7300c76ce is OK with me.

tmiw commented 3 years ago

I'd say leave as is unless there are bugs in FreeDV itself as a result of the controls moving horizontally (e.g. crashes or other unwanted behavior triggered by resizing). If that's the case, we can revert back to 607b827's behavior. 👍

drowe67 commented 3 years ago

@tmiw sure - the control moving horizontally is fine.

However as per the OP I'm seeing many configurations that are not viable, e.g. the screen shot above with no central pane, and being able to minimise the entire freedv-gui window down to zero size. Can you reproduce these problems? These seem like fairly serious GUI layout issues to me.

Sorry if I'm missing something here ....

tmiw commented 3 years ago

I might not have been clear before. The point I was trying to make was that as long as the application behaves properly when resized back to a valid configuration, it's probably fine. I'm not sure anyone would willingly resize the app down to 0x0 (or anything where some of the controls would be invisible/clipped), anyway. A minimum vertical size will probably make those scenarios less likely as well, though not 100% eliminated; there doesn't seem to be a way in wxWidgets to have multiple minimum widths and heights without allowing the resize to happen first.

That said, I did get this message while continuously resizing and playing the 700D test file. I suspect we can add more checking for valid sizes inside PlotWaterfall to avoid this one.

Screen Shot 2021-05-27 at 10 42 30 AM
drowe67 commented 3 years ago

Hi @tmiw - thanks I understand your thoughts now. Yes I agree people wouldn't intentionally resize to zero. I tried a few other applications (e.g. Audacity) and they allow resizing to small, but not zero sizes, and all of the GUI elements are in place, although some are cropped.

I feel having "valid" GUI layouts (in particular the centre pane) is more important than the benefit of the horizontal re-arrangement. Rather than adding more code to deal with corner cases I would prefer to design the GUI layout so it can't get into (what I call) invalid layouts - as per the previous designs like https://github.com/drowe67/freedv-gui/commit/607b827ef5d8fc789f500c0b9ebe5aa7300c76ce

So this one is perhaps a difference of opinion, and that's cool :slightly_smiling_face: Perhaps we could see what end users think on the next release? If no one seems to care then perhaps it's a non-issue.

tmiw commented 3 years ago

@drowe67, so yeah, see the PR I just made. We can give that a shot and if it causes issues, we can go back to the old behavior.

tmiw commented 3 years ago

Since the PR seems to be working okay in our testing, should we close this issue? Or wait until we get runtime by others on an upcoming build?