drowe67 / freedv-gui

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

1.9.5 left side of main window widgets vanish when resizing main window height, this did not happen in b5d73 before release. #610

Closed barjac closed 10 months ago

barjac commented 11 months ago

A bug was introduced just before release of 1.9.5 that breaks the re-arrangement of the widgets on the left side of the screen. They are removed from view so that stats can not be monitored nor the level meter. I run the main window at down to half screen height, so this is a major problem. I am seeing the same issue in x86_64 and aarch64.

barjac commented 11 months ago

Original effect of reducing window height:

Screenshot_20231119_161245

Now in 1.9.5 release:

Screenshot_20231119_161431

tmiw commented 11 months ago

I can duplicate using the following steps:

  1. Resize window to have everything visible without wrapping.
  2. Shrink window to force the Level indicator to wrap to the right.
  3. Restart FreeDV.

Anyway, it looks like https://github.com/drowe67/freedv-gui/pull/611 fixes this for me. Let me know if that fixes it for you too and I can go ahead and merge/close.

barjac commented 11 months ago

I think this needs to be re-opened :(

There is another issue with recovering the screen layout between sessions.

If the program is closed like this (the layout is not on the edge of a state change):

Screenshot_20231120_172601

It recovers like this after re-starting with the stats chopped in half:

Screenshot_20231120_172628

tmiw commented 11 months ago

Reopening due to above request. I can't seem to duplicate this on Ubuntu 22.04, though. This might be because the local version of FreeDV that I built is set to build in its own copy of wxWidgets 3.2 (vs. using the distro-provided one, which IIRC is still 3.0). If you can confirm the wxWidgets version you're using, I can investigate further.

barjac commented 11 months ago

It's 3.2 here. I just reproduced it also on my laptop. When the window is shrunk from the bottom up until the level gauge is on its own to the left of the scope, it then vanishes on restart. The libwx* .so's currently required by freedv are: libwx_baseu-3.2.so.0()(64bit) libwx_baseu-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_aui-3.2.so.0()(64bit) libwx_gtk3u_aui-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_core-3.2.so.0()(64bit) libwx_gtk3u_core-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_propgrid-3.2.so.0()(64bit) libwx_gtk3u_propgrid-3.2.so.0(WXU_3.2)(64bit)

barjac commented 11 months ago

Screenshot_20231121_104724

That's running #a5217 on the laptop (sorry I chopped the header bar off the image).

barjac commented 11 months ago

Better still watch this: http://mtf.duckdns.org/pub/linux/barjac/test/out.ogv

Tyrbiter commented 11 months ago

Fedora 39 has wxWidgets 3.2.4 now and I am not seeing this behaviour after https://github.com/drowe67/freedv-gui/pull/611 in my local 1.9.6-devel build.

I currently have the experimental features turned off but I am still seeing the stats box, I assume this is intentional at present.

barjac commented 11 months ago

Fedora 39 has wxWidgets 3.2.4 now and I am not seeing this behaviour after #611 in my local 1.9.6-devel build.

What does: rpm -q --requires freedv|grep libwx say in Fedora 39?

Why would you expect not to see the stats box? Surely that is not being removed? It is essential for keeping track of frequency errors etc. I agree that there is wasted space in various widgets but that is certainly not one of them.

Tyrbiter commented 11 months ago

I see this in F39 @barjac

$ rpm -q --requires freedv|grep libwx libwx_baseu-3.2.so.0()(64bit) libwx_baseu-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_aui-3.2.so.0()(64bit) libwx_gtk3u_aui-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_core-3.2.so.0()(64bit) libwx_gtk3u_core-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_propgrid-3.2.so.0()(64bit) libwx_gtk3u_propgrid-3.2.so.0(WXU_3.2)(64bit)

the actual .so files have *.so.0.2.2 which suggests to me that there are only bug fixes in anything later than wxWidgets-3.2.2, certainly some fixes affect Wayland if you use that.

As for the stats box, there has been discussion about its value for new users, it may display confusing information while trying to get things working, hence the idea of hiding it unless another setting is enabled. Currently using the experimental features control is one suggested way to do this, but I reckon a little more granularity is needed.

As yet no decision has been made to make this the default. Note that some of the values shown in the stats box are also available in the graph tabs in the main window.

tmiw commented 11 months ago

I just tried again on Fedora 39 (after running dnf update as it had been a while) and I'm not seeing the issue there, either. I did compile from source instead of generating RPMs, though. This is what it looks like for me immediately after shrinking the window and restarting:

Screenshot 2023-11-21 at 2 29 41 PM

EDIT: dnf info wxGTK shows the following for me:

mooneer@fedora:~/freedv-gui/build_linux$ dnf info wxGTK
Last metadata expiration check: 0:00:28 ago on Tue 21 Nov 2023 02:33:22 PM PST.
Installed Packages
Name         : wxGTK
Version      : 3.2.2.1
Release      : 5.fc39
Architecture : x86_64
Size         : 17 M
Source       : wxGTK-3.2.2.1-5.fc39.src.rpm
Repository   : @System
From repo    : fedora
Summary      : GTK port of the wxWidgets GUI library
URL          : https://www.wxwidgets.org/
License      : wxWidgets
Description  : wxWidgets is the GTK port of the C++ cross-platform wxWidgets
             : GUI library, offering classes for all common GUI controls as well
             : as a comprehensive set of helper classes for most common
             : application tasks, ranging from networking to HTML display and
             : image manipulation.

As for the stats box, there has been discussion about its value for new users, it may display confusing information while trying to get things working, hence the idea of hiding it unless another setting is enabled. Currently using the experimental features control is one suggested way to do this, but I reckon a little more granularity is needed.

As yet no decision has been made to make this the default. Note that some of the values shown in the stats box are also available in the graph tabs in the main window.

Yeah, there's not supposed to be a way to hide the Stats box right now, but an option to do so is under consideration.

barjac commented 11 months ago

My vote would be to leave it alone. I am regularly checking FreqOff and would much prefer it visible on screen without having to navigate elsewhere. We don't all use SDR transceivers :) There is lots of wasted space in the GUI widgets like the level and SNR gauges and the sliders. Can these not be tightened up so the whole GUI is more compact or did GTK3 make this impossible? Other packages now suffer from similar issues. e.g. xnec2c which is far worse now than it was with gtk2. I had long discussions with the developer over that and ended up with no real resolution.

Tyrbiter commented 11 months ago

My vote would be to leave it alone. I am regularly checking FreqOff and would much prefer it visible on screen without having to navigate elsewhere. We don't all use SDR transceivers :)

Perhaps a marker could be placed below the spectrum display to show what the offset is as seen by the modem. Or maybe the offset could be numerical and placed so it's visible on frequently viewed main window graph tabs.

There is lots of wasted space in the GUI widgets like the level and SNR gauges and the sliders. Can these not be tightened up so the whole GUI is more compact or did GTK3 make this impossible? Other packages now suffer from similar issues. e.g. xnec2c which is far worse now than it was with gtk2. I had long discussions with the developer over that and ended up with no real resolution.

GTK4 is going to be the new game in town soon, but I think a lot of this is down to limitations of wxWidgets and the possible different ways of creating a tiled display area. @tmiw is probably a lot more knowledgeable about that than I am.

The aim is purely to improve the situation for a new user who doesn't yet understand what all the values mean, it's not going to disappear just be part of potential advanced user options.

barjac commented 11 months ago

The aim is purely to improve the situation for a new user who doesn't yet understand what all the values mean, it's not going to disappear just be part of potential advanced user options.

He will never ever question what they mean if they are not in view from day one. By dumbing down the display you end up dumbing down the users. Who actually proposed that idea? Why exactly? I don't fully understand them all but I do try to read up and find out! This is supposed to be a hobby for self training.

tmiw commented 11 months ago

He will never ever question what they mean if they are not in view from day one. By dumbing down the display you end up dumbing down the users. Who actually proposed that idea? Why exactly? I don't fully understand them all but I do try to read up and find out! This is supposed to be a hobby for self training.

I typically use a couple of the most popular ham applications as a benchmark for what's expected by hams (for example, I've looked at WSJT-X fairly frequently as of late to see what they do). AFAIK the closest we get to a "detail" view of what's going on in that app is the RX SNR and the waterfall. Of course, FT8/FT4/etc. are significantly different than something like FreeDV, so some stuff in the app will be different, too.

Tyrbiter commented 11 months ago

He will never ever question what they mean if they are not in view from day one. By dumbing down the display you end up dumbing down the users. Who actually proposed that idea? Why exactly? I don't fully understand them all but I do try to read up and find out! This is supposed to be a hobby for self training.

Some people just don't work that way @barjac however much we would like it to happen. The suggestion was made in the light of several comments from various mailing lists and on Discord. No decision has been made on this, naturally your input will be taken into account too. If a change is made this will be documented in the user manual, which we hope users read ;-)

barjac commented 11 months ago

I have built a package of master at #ef6d3 for Mageia 10 (cauldron) which has wxGTK 3.2.4. In a new system running Mageia 10 it is behaving exactly as it is in the video I posted above. I notice that when the Stats are above Level then on restart it appears to be Level that is controlling the widget width in that 'column' which is causing the stats to be squashed to match the level. Maybe some sort of precedence is not being considered? In this case the widgets in a column should adopt the width of the widest widget not the narrowest, which is what appears to be happening here:

Screenshot_20231124_222125

Could the Stats widget be missing some 'minimum width' parameter, it only seems to affect that widget. I can understand that Level may disappear when the window height is taken to silly extremes but not the Stats truncation.

I notice a lot of compiler warnings mainly about non-void functions not returning a value, no idea if one of these could be involved. Build log:

ef6d3_log.txt

tmiw commented 10 months ago

I'm still not fully sure why the Stats box isn't rendering properly so I went ahead and forced the widths of all the boxes on the left to be the same: https://github.com/drowe67/freedv-gui/pull/613. Hopefully that helps, or at least makes stuff look more consistent?

barjac commented 10 months ago

OK progress :) This does resolve the issue in both Mageia 9 using GTK-wx 3.2.1 and in Cauldron using 3.2.4. However it does cause even more re-arrangement when reducing the window height to less than just over half screen. I have not yet tested on a wide screen.

If the SNR and Level widgets could be turned through 90deg so the gauges are horizontal and the widget widths kept as they are now, then there would be no need for any rearrangement of the left side until the window height was down to maybe 1/3 screen height or less. IMO slightly shorter gauges would not be a problem visually or functionally.

barjac commented 10 months ago

Ah! No, on a wide screen this is not going to work. Leaving only a moderate height for the reporter the 'scope' is shrunk to nothing. By making all the gauges and sliders horizontal on both sides could be the answer to reducing all the wasted real estate.

Screenshot_20231125_110936

tmiw commented 10 months ago

I shrank each of the boxes a tiny bit to remove some whitespace, which results in the following (simulating a 1920x1080 display):

Screenshot 2023-11-25 at 2 03 15 PM

Of course, my test setup may not 100% simulate what you're seeing, so I don't know if shrinking the widths of each box by ~50px is enough.

Re: making each slider horizontal instead of vertical -- I'm not sure if we can maintain the existing range/precision if we do so. That would work around #303, though, so there's that.

barjac commented 10 months ago

Re: making each slider horizontal instead of vertical -- I'm not sure if we can maintain the existing range/precision if we do so. That would work around #303, though, so there's that.

There may be no need to reduce the lengths of the bars in the widgets so precision would not be affected when turning through 90 deg. It's the height of these widgets that forces the re-arrangement. By turning them they can be very low (height wise) so that they will not force new columns to be added as the window height is reduced. If there are no new columns created then the 'scope' widget will not need to reduce width.

tmiw commented 10 months ago

I haven't checked this in yet but with the sliders all horizontal, here's the smallest I can shrink to before boxes get truncated:

Screenshot 2023-11-26 at 7 26 21 AM

and here's the shortest without boxes repositioning themselves:

Screenshot 2023-11-26 at 7 29 02 AM

Anyway, I'm not fully sure that makes much of a difference vs. the screenshot I posted above, though with the same caveats about my environment.

barjac commented 10 months ago

I wonder if you could point me to those changes or let me have a patch against master to try here?

tmiw commented 10 months ago

I wonder if you could point me to those changes or let me have a patch against master to try here?

They should be in #613 now.

barjac commented 10 months ago

Yes! This is just fine for me and would work well on all three machines that I am testing.

I also think that this improves the whole GUI when in full screen.

The two sliders on the right need to have the right end equivalent to the top as it is in the current layout or it will be non-intuitive to most users.

I will add images of all screens with the main window reduced in height to the point where only two columns are visible on both sides in a 'before and after' display.

EDIT: As may be seen below there is more room below the main window in all cases for the reporter which will then hold more station lines.

The most striking improvement is on the Lenovo laptop where the 'scope' was compressed to nothing well above half screen as shown earlier. It now has a usable 'scope' with the window at half screen height and 3 widget columns on each side.

barjac commented 10 months ago

Here is the Lenovo laptop with a 1280x960 screen. Previous:

Screenshot_20231126_200451

Proposed: Screenshot_20231126_200829

barjac commented 10 months ago

Desktop with 1280x1024 screen:

Current at #87efd:

Screenshot_20231126_205036

Proposed at #981ab:

Screenshot_20231126_205407

barjac commented 10 months ago

Raspberry Pi 4b aarch64 with 1440x900 screen (from HP laptop :)

Current:

screen05

Proposed at #981ab:

screen04

barjac commented 10 months ago

A practical use case: Raspberry Pi monitoring a Kiwi SDR, where with this layout there is room for a good number of stations in the reporter while also displaying the SDR output.

screen07

The screenshot was taken with the SDR on 60m at night so ignore the QRM. Also note who was transmitting :)

tmiw commented 10 months ago

Try the latest from the previously mentioned PR. The sliders on the right hand side should work a bit more logically now given the new layout.

barjac commented 10 months ago

Yes that is more logical. Could 'Level' lose the empty space above the gauge?

When shrinking the window height, Level creates a new column, then Record creates another. Could the widget order be changed so that a maximum of 3 columns is ever created?

Maybe if Level was under SNR (as they are the same width like Squelch and TX Atten on the right) then Sync and Audio would re-arrange to a new, narrower column to the left of Stats, which would always have it's own column.

tmiw commented 10 months ago

Could 'Level' lose the empty space above the gauge?

That's for the "Too High" indicator, so I'd lean towards not trying to eliminate that.

When shrinking the window height, Level creates a new column, then Record creates another. Could the widget order be changed so that a maximum of 3 columns is ever created?

How about now?

barjac commented 10 months ago

Could 'Level' lose the empty space above the gauge?

That's for the "Too High" indicator, so I'd lean towards not trying to eliminate that.

Ah yes I forgot about that :(

When shrinking the window height, Level creates a new column, then Record creates another. Could the widget order be changed so that a maximum of 3 columns is ever created?

How about now?

Perfect! I like it, well done! That is working just as I envisaged :)

On the Lenovo wide screen laptop that was struggling, there is now room for a web SDR as well as enough space in the reporter without spoiling the main GUI.

Screenshot_20231130_223212

barjac commented 10 months ago

Damn!! On restart Stats is getting clobbered again. EDIT: It only happens when Stats gets pushed into the third column. It's fine with only two left columns.

Screenshot_20231130_230719

Any slight change in the window height corrects it. If a cure can't be found then I reckon that a hack/workaround could be used to wiggle the main window bottom edge up and down a bit just after the main window is painted :)

tmiw commented 10 months ago

Damn!! On restart Stats is getting clobbered again. EDIT: It only happens when Stats gets pushed into the third column. It's fine with only two left columns.

I'm not sure if what I just pushed will help. If nothing else, it'll keep the y dimension from being shrunk enough to cause the Stats box to be truncated. Worth a try, anyway.

barjac commented 10 months ago

No it does not really help, in fact the previous could be worked around by jiggling the window height but this now cannot :(

This restricts the left side to two columns, such that the height reduction gained by using three columns each side on wide screens is lost.

Record and Stats end up in the same column where previously Stats had its own.

On wide screen laptop with max height compression of main window:

Screenshot_20231203_132510

barjac commented 10 months ago

If this were to be the best that can done until maybe the actual bug is found, be it in wxGTK or freedv, then I think it would be major improvement for the GUI compared to the one in 1.9.5.

Despite being 'less than perfect' for wide screens, it does re-start correctly, looks much tider and reduces wasted space in the GUI.

I would be happy to see this layout in 1.9.6 but even happier with #ec735 if the 'Stats clobber on restart' bug could be fixed :)

tmiw commented 10 months ago

So I removed the minimum height constraint for the window (though I really think there should be something since it's currently possible to shrink the window enough to hide some of the contents of the Stats box*) and implemented the workaround you proposed earlier (make the window bigger by 1px in both X and Y and then resize back to the saved size). This does seem to resolve the issue on macOS and likely other platforms as well.

* Unfortunately I think there might need to be different minimums depending on OS and possibly desktop environment, so I'm not sure if this is even workable. For example, in my environment I apparently need a 450px minimum height to avoid issues on Linux while it only needs to be ~375px on macOS (and for you, 450 might be too strict due to being forced into a two column maximum).

barjac commented 10 months ago

So I removed the minimum height constraint for the window (though I really think there should be something since it's currently possible to shrink the window enough to hide some of the contents of the Stats box*)

That really does not matter. It's possible to break most GUIs in lots of good software if you try hard enough. Give the users some credit - why would they do that? Take pavucontrol as a simple example. The window can be shrunk to a point where it's totally useless - who cares? In fact it is actually a plus! Since I currently only look at Freq Off and above, I can hide the last two items in Stats and gain even more space in the reporter. See screen shot below.

...and implemented the workaround you proposed earlier (make the window bigger by 1px in both X and Y and then resize back to the saved size). This does seem to resolve the issue on macOS and likely other platforms as well.

\o/ Yay - yes it works fine here too in Mageia Linux, although I think only Y needs tweaking not X as well. I'm so glad that works :)

  • Unfortunately I think there might need to be different minimums depending on OS and possibly desktop environment, so I'm not sure if this is even workable. For example, in my environment I apparently need a 450px minimum height to avoid issues on Linux while it only needs to be ~375px on macOS (and for you, 450 might be too strict due to being forced into a two column maximum).

Please forget any minimum height it's up to users to use common sense (they got a license so they must have some) :)

This is again on the Lenovo Thinkpad with wide screen, it recovers perfectly after close and restart.

Screenshot_20231204_203732

barjac commented 10 months ago

Another screen shot of a more typical screen aspect ratio using this build, without the need for minimal height. I think this layout is much cleaner and more compact than the original with the vertical gauges and sliders.

Screenshot_20231204_210625