Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#170 closed defect (wontfix)

QIcon size selection different Windows vs. OS/2

Reported by: rudi Owned by:
Priority: major Milestone: Qt 4.6.3
Component: QtGui Version: 4.6.2
Severity: low Keywords:
Cc:

Description

As you can see in the attachments, the "back" / "forward" buttons in the OS/2 version of the Arora browser are much smaller than in the windows version. The reason for that is that the PNG images for those buttons are available in the resolutions 16,32 and 128px while the "reload" image is present in 24 and 32px. The default size of the toolbar buttons in OS/2 is 24px, for which "back" and "forward" don't have an exact match. However, while our implementation chooses the next smaller pixmap, the Windows version downscales the higher one.

Even though the Windows source is also of version 4.6.2, there
is different code in gui/image/qicon.cpp:

OS/2:

161	    int b = area(pb->size);
162	
163	    // prefer the largest among smaller
164	    int res = qMax(a,b);
165	    if (res > s) {
166	        // fallback to the smallest among larger
167	        res = qMin(a,b);
168	    }

Windows:

    int b = area(pb->size);
    int res = a;
    if (qMin(a,b) >= s)
        res = qMin(a,b);
    else
        res = qMax(a,b);

Attachments (6)

history_bunttons_win.jpg (5.5 KB) - added by rudi 9 years ago.
history_buttons_os2.jpg (6.2 KB) - added by rudi 9 years ago.
filedialog_style_inconsistent.jpg (8.0 KB) - added by rudi 9 years ago.
file dialog using new size match policy
filedialog_style_consistent.jpg (9.4 KB) - added by rudi 9 years ago.
file dialog with patch in qcommonstyles
filedialog_style_windows.jpg (8.7 KB) - added by rudi 9 years ago.
file dialog on Windows
history_buttons_os2_new.jpg (8.7 KB) - added by rudi 9 years ago.
Arora history buttons using new size policy

Download all attachments as: .zip

Change History (15)

Changed 9 years ago by rudi

Changed 9 years ago by rudi

comment:1 Changed 9 years ago by rudi

Qbviously Nokia has changed the policy for finding the best size match for an icon. 4.6.3 does contain the same code that I quited from "Windows". The strange thing is, that even 4.5.2 does have this code. I'm really wondering at what version level our "qicon.cpp" really is. Could it be, that this was forgotten when we moved from 4.5.1 to 4.6.2 ?

comment:2 Changed 9 years ago by rudi

Applying the new size match policy makes Arora look fine. However, it introduces an inconsistency in the file dialog. See attached pictures. I tried to avoid this by adding the following to gui/styles/qcommonstyle.cpp:

Index: qcommonstyle.cpp
===================================================================
--- qcommonstyle.cpp    (revision 758)
+++ qcommonstyle.cpp    (working copy)
@@ -6001,6 +6001,13 @@
             static QIcon::State states[] = { QIcon::Off, QIcon::On };
             int small = pixelMetric(PM_SmallIconSize);
             int large = pixelMetric(PM_LargeIconSize);
+
+            QPixmap     p = icon.pixmap(large);
+            if (p.width() == 16) {
+                icon.addPixmap(p.scaled(large, large,
+                    Qt::KeepAspectRatio, Qt::SmoothTransformation));
+            }
+
             for (size_t m = 0; m < sizeof(modes)/sizeof(modes[0]); ++m) {
                 for (size_t s = 0; s < sizeof(states)/sizeof(states[0]); ++s) {
                     QIcon::Mode mm = modes[m];

Changed 9 years ago by rudi

file dialog using new size match policy

Changed 9 years ago by rudi

file dialog with patch in qcommonstyles

Changed 9 years ago by rudi

file dialog on Windows

Changed 9 years ago by rudi

Arora history buttons using new size policy

comment:3 Changed 9 years ago by dmik

  • Milestone changed from Qt Enhanced to Qt 4.6.3

comment:4 Changed 9 years ago by dmik

  • Resolution set to wontfix
  • Status changed from new to closed

What are you talking about is actually my patch (see r261) as I considered it as a vendor bug, so no surprise you can't find it in the original sources from Nokia. A bug not only because of the misalignment with what docs says but also because I think that a blurred icon is much worse than an icon with some empty field around (and yes, the file dialog was one of the examples of the bogus behavior). In any case, this problem should be fixed at the application level by providing an icon of the correct size (or at least making sure that all icons have the same set of sizes in which case all icons will have the empty field around them which will also be acceptable).

Thanks for raising this up anyway.

comment:5 follow-up: Changed 9 years ago by rudi

Actually, in case of the file dialog I find the blurred icons not that bad... The difference in size even makes it easier for the user to distinguish between the "favorite folder list" and the "folder contents".

Personally I don't think we should change the size policy to something other than what is done in Windows and Linux. Even if we believe, that the original implementation is wrong.

Of course it would be the best to fix this in the application, but I guess it's going to be hard to convince the authors to do so if they don't see this problem on any other platform the app is run on.

comment:6 in reply to: ↑ 5 Changed 9 years ago by dmik

Replying to rudi:

The difference in size even makes it easier for the user to distinguish between the "favorite folder list" and the "folder contents".

They are bigger just by accident. It's a side effect. Side effects are evil because they are generally unpredictable. If you want the icons to be bigger, make them bigger by writing the proper code and providing the proper icon resolutions.

Of course it would be the best to fix this in the application, but I guess it's going to be hard to convince the authors to do so if they don't see this problem on any other platform the app is run on.

I expect people to share common sense :) If they don't, I don't care. But in case of Arora (or any other opensource software), you can simply fix it yourself. I guess the authors will easily accept this fix even if they would not do it themselves.

BTW, provided that Nokia ever accepts our patches, the fix will go cross-platform which eliminates this argument.

comment:7 Changed 9 years ago by Rudi

They are bigger just by accident. It's a side effect.

I'm not so sure about that.

BTW, provided that Nokia ever accepts our patches, the fix will go cross-platform which eliminates this argument.

The docs say "The result might be smaller than requested, but never larger.". But that does not neccessarily imply that the largest among the smaller is selected. It could just mean that icons are never upscaled in case no larger image exists. If a larger image is there, smooth downscaling might be done by design, not by accident.

comment:8 Changed 9 years ago by dmik

Well, OK, I'm not fully correct regarding the matter of the "accident". They ask for some certain icon size (24 px I believe) for the side bar but they don't enforce it: If the icon returned is smaller than that (which is pretty legal according to QIcon docs), this smaller icon will be drawn w/o normalizing it (e.g. centering or scaling at worst). So they don't actually care if there is a misalignment among the side bar items and if 24px is actually any bigger than the normal icon size in the file list to the right. These are all side effects and the result is still accidental, depending on the real icon sizes the platform has for various types of URLs.

I also agree that both approaches (the original and mine) are valid according to the documentation. So, for compatibility reasons (with "broken" applications like Arora) I reverted r261 in r763.

Then, in r764 I fixed QCommonStyle::standardIconImplementation() to make sure that both the small and lagre standard icon are present in the stock icons (which is actually an improved version of what you suggest in comment:2).

Finally, in r765 I fixed QFileDialog (its private QSideBar class actually) to use the item size hint to set a slightly bigger item height in the side bar compared to the normal height of items in the file list view (instead of enforcing the item icon size) -- this approach is much cleaner because it avoids the ugly icon scaling that I so dislike. More over, this height increase is now dynamic (which was actually an original Qt devs' TODO as you can see from the source).

Hope everyone is happy now :)

comment:9 Changed 9 years ago by rudi

Yep. Everthing else shoud be done in a style plugin (once should I say if) we ever have one...

Note: See TracTickets for help on using tickets.