andrew-bibb / cmst

QT GUI for Connman
174 stars 37 forks source link

Tray Icon too small for HiDPI screens. #256

Open replica9000 opened 2 years ago

replica9000 commented 2 years ago

It seems the tray icon is fixed at 22x22, so it doesn't scale with the rest of the tray icons on HiDPI screens.

controlbox.cpp, line 2337

QPixmap pxm = prelimicon.pixmap(prelimicon.actualSize(QSize(22,22)) );

After resizing the raw_images to 48x48, and setting the code to 44,44, it looks much better for HiDPI.

andrew-bibb commented 2 years ago

Thank for reporting this. I've been knocking off a few of the older issues these past two weeks and looks like this one has now percolated to the top of the "to do" list. Been looking at it and I see there is a fairly new, or new to me, section in the QT documentation about icons and high DPI screens. Still digesting that document but once I do hopefully this issue will have a simple solution.

andrew-bibb commented 2 years ago

The line you reference is not actually setting an icon size, it is supposed to return a pixmap scaled appropriately for high dpi screens for an icon that would look 22x22 on a normal screen. I've been through this code several times and I think it is correct. What I have done though is enable two Application Attributes in main.cpp that were not set before. The first one not being set I think was the likely cause if I'm interpreting the documentation correctly. I have no way of checking this, and it is entirely possible I've done nothing to address the issue, so if you could check it I'd appreciate it.

replica9000 commented 2 years ago

I just compiled cmst 2022.01.16-1 with QT 5.15.2. When I start the app, It crashes with this error, which I believe is actually a QT5 bug. qt.qpa.xcb: xcb_shm_create_segment() can't be called for size 17178820624, maximumallowed size is 4294967295

I can start the app with QT_AUTO_SCREEN_SCALE_FACTOR=0 QT_SCALE_FACTOR=1, however, the tray icon and other icons remain very small on HiDPI.

scrot-2022-01-17-072329-2

andrew-bibb commented 2 years ago

Thank you for testing. I'll keep plugging away at it, but without owning a high DPI monitor this is likely to take a while.

Just to confirm, it is both the tray icon and the internal icons? Up to this point I've only been looking at the tray icon, if it is both there is an entirely different .cpp file I should also look in.

From the screenshot it appears you are using the internal icon set we provide. Can you try it with an icon theme (preferences tab on the right hand side, check Use Icon Theme then fill in the theme name in the box). It is a start option so you'll need to exit CMST then restart to see the changes. I'm curious to see if it is only a problem for the built in icons or if it also effects a mainstream icon set.

replica9000 commented 2 years ago

External icon sets are also small.

andrew-bibb commented 2 years ago

I just made and uploaded two minor changes. The biggest is I removed the line I think caused you to need the environmental variable set. Setting that undid the line in the code and may have also had other effects. Basically it said to ignore anything dealing with high DPI. I've left in the automatic scaling of pixmaps variable to see if that works now.

If it does not I'm fairly sure I've come up with a way to force scale them. I'd prefer to have QT do it automatically but if not I'll add the code.

replica9000 commented 2 years ago

Just compiled the changes. The app will now run without the environmental variables, however the icons are still small.

andrew-bibb commented 2 years ago

Thank you for testing. I'm going to put a way for the user to scale the icons, probably a command line option.

andrew-bibb commented 2 years ago

I think there are several issues going on. One is that all of the internal icons were limited to 24x24 size. No matter what we asked for in size we never were going to get anything larger than that. I've rebuilt all the internal icons to 128x128 size. There may still be an issue with external sets being limited, but want to try the internals before I start working on the externals.

Also the problem seems to be affecting icons, labels containing pixmaps, and toolbuttons. If this upload fixed anything it may have only fixed some of these, although I did try to get all three. For the labels containing pixmaps I changed the pixmap size from fixed (generally 16x16) to a percentage of the label height. From your screen shot it appeared that the labels were the proper height so figured I'd use that as a base. For toolbuttons I changed the size policy of each one I could find. For the icons I'm hoping having sizes not limited by 24x24 will cure that. Lot of hope going on here but lets see how it looks.

Thanks. A-

replica9000 commented 2 years ago

No luck :(

With the previous builds, I also tried doubling the internal icon sizes to see if that helped. I also get this error when trying certain external icon sets, such as Adwaita, though the icons still appear.

qt.svg: Invalid path data; path truncated.

scrot-2022-01-23-204535-2

andrew-bibb commented 2 years ago

Comparing the screen shots side by side I actually think the icons in the Wifi tab of the second screen shot did increase in size. I think that part may have been solved. The icon in the "Whats This" toolbutton is still obviously wrong as is the tray icon size.

I just uploaded a fix to add larger sizes for system theme icons, such as Adwaita. No need to test that, I'm fairly confident I got it. I originally wrote the program to max out at 24x24 icons, both internal and system theme. Over the weekend I had increased the internal ones to 128x128 but had not touched the system theme ones. That is now been fixed.

Would it be possible for you to screen shot the first(Status) tab? I'm particularly interested in the icons in the Global properties section. The first one for "State" is a label, the one below it for "Offline Mode" is a toolbutton. The icon sizes for each are set differently and I'd like to see how each renders on your system.

My fallback position is to provide an option to scale the icons by a factor of 2. I actually had all that code written over the weekend which is when I discovered that I had originally limited everything to 24x24 max size. I pulled all that new code out thinking that redoing the icons to larger size might solve it. Looking at the screen show above I think that may have been a partial solution, obviously not a complete one.

replica9000 commented 2 years ago

Screenshot of the 2022-01-25 build.

scrot-2022-01-25-200246-2

The below screenshot is the 2021-12-05 version of cmst, with the modified code in the first post (now using the larger icons from your recent versions)

scrot-2022-01-25-200830-2

andrew-bibb commented 2 years ago

So if I'm interpreting the screen shots correctly the systemtray icon size has been solved with a combination of having larger size icons built in, plus adding the QT setting: setAttribute(Qt::AA_UseHighDpiPixmaps), and using the original code from December for the tray icon. The original code was the "actualSize" function way up in your first post, I removed that after one of the attempts failed. Just want to verify that because it sort of all makes sense and I want to make sure that "making sense" part is not coloring my analysis. I'm also looking at the CMST greenish icon in the window bar and that is rendered correctly. That is another clue that may help.

No matter what however I see that the toolbuttons plus the labels are still all too small. All of sizes of those are handled by the internal layouts in the UI. My hypothesis is that I somewhere put in a fixed size for those or the QT layout engine is not rendering them correctly. I used the QT-Designer program to build the UI's and there is a ton hidden behind the scenes when you use that. Going to take me a bit to go into the UI designer program and possibly code generated from it (and there is a lot of code generated from it) to see if the problem is there.

As an aside the red and green off and on buttons in the first screen shot are also too small, but I kind of like them better at that size. May change them when this is all done.

Thanks for sticking with me because without a high DPI monitor there is no way I could do this myself.

replica9000 commented 2 years ago

I don't mind testing. I wonder if you could use xrandr to fake HiDPI resolution for testing.

andrew-bibb commented 2 years ago

Just uploaded a handful of commits. Keep the larger icons and tried to roll back to the first change so the tray icon hopefully looks to be the size in your second screen shot. Also tried to use similar code to size the "What's This" tool button lower left corner plus the "State" label and "Offline Mode" toolbutton on the first tab. Did find a bunch of 16x16 sizes on these items buried from QDesigner and I tried to scale them similar to the way the tray icon is.

Have not touched the icons in the wifi tray, want to wait to see if the fixes above work. If they do there are several more in the UI I need to work on plus the ones in the tableviews.

replica9000 commented 2 years ago

It looks like the icons are still the same size. Also, resizing the app only resizes the "State" area instead of the Technologies and Services sections under the first tab.

2022-02-06-135330_3840x2160_scrot-2

andrew-bibb commented 2 years ago

The sizing issue is just me messing around in QDesigner. I just need to define (or redefine) some expanding policies.

In your post 12 days ago you state The below screenshot is the 2021-12-05 version of cmst, with the modified code in the first post (now using the larger icons from your recent versions) Can you specify exactly which of my posts is "first post". I thought I had it and had reverted the changes back to that. Think I am going to concentrate on the tray icon, from the screen shot 12 days ago it was fixed once. If I can get back to that I'll try to move on to the other ones.

replica9000 commented 2 years ago

My opening post. I modified this line of the code in the 2021-12-05 version. I doubled the value from 22 to 44, and doubled the size of the icons too. I hadn't looked at any of the other code for the other icon sizes.

QPixmap pxm = prelimicon.pixmap(prelimicon.actualSize(QSize(22,22)) );

Edit: That change only affects the internal tray icons. External icon sets still appear small.

andrew-bibb commented 2 years ago

OK, now I understand, thought you meant my first post where I changed something. It has become clear now that I am going to need to put in a control to scale icon sizes. Either having QT do it automatically is beyond my abilities or maybe it is not possible without starting with High DPI icons. At least it is a way forward and something I know I can do.

By the way, already fixed (and uploaded the fix) for the rescale issue.

andrew-bibb commented 2 years ago

Well if you are game I've got a partial attempt at a fix. There is a new command line option, capital I, that takes a number between 1 and 3 for an icon scale. It also is available in the Start Up Options section of the Preferences tab, although if you use that you will need to exit then restart CMST as the options are only read on startup.

Right now the only icon scaled is the tray icon. The 22x22 size is multiplied by whatever number you supply for the option with 1.0 being the default. If the tray icon works I'll add this to all the other artwork in the program plus all the manpage and other help files.

replica9000 commented 2 years ago

The new option works great!

andrew-bibb commented 2 years ago

Just uploaded code to change the other artwork (excepting the green and red on/off buttons). All these other pieces of artwork should now be linked to the commandline option -I.

replica9000 commented 2 years ago

The changes look good!

scrot-2022-02-12-145325-2 scrot-2022-02-12-145327-2

andrew-bibb commented 2 years ago

Thank you for testing and staying with me on this. I still think some future QT upgrade will make all this superfluous, but for now it is good. I'll probably close this issue in a day or so.

Not sure if you are aware of this or not but the internal icon set you are using can be colorized if you want. Over in the preferences tab in the upper left corner in the interface section. Just mentioning it in case you want to make them a bit more visible.

replica9000 commented 2 years ago

No problem. Let me know if you need me to test in the future.