Konqueror throbber size -- too small and doesn't resize

I find in: , line 3762

int size = style()->pixelMetric(QStyle::PM_SmallIconSize, NULL,
m_paAnimatedLogo);

appears to set the size to "SmallIconSize" rather than choosing a size
based the pixel size of the menu bar which seems to be dependent on the
pixel size of menuFont.

, I only made 16x16, 22x22, & 32x32 throbbers. IIRC, someone
reversed the direction of rotation. I can make larger ones if needed.

It might also be possible to make an SVG which could be rendered to the
exact size that is needed. That would be simple since the PNGs are made
with InkScape.

--
JRT

David Faure wrote:
Wednesday 04 June 2008, James Richard Tyrer wrote:
>I find in: , line 3762
>>
>int size = style()->pixelMetric(QStyle::PM_SmallIconSize, NULL,
> m_paAnimatedLogo);
>>
>appears to set the size to "SmallIconSize" rather than choosing a size
>based the pixel size of the menu bar which seems to be dependent on the
>pixel size of menuFont.

Yeah, but the real problem seems to be the margin inside the QToolButton.
I tried int size = menuBar()->height() and obviously this was too big, but it
shows that the button contains a lot of inner padding -- much more than when
a QToolButton is put into a toolbar. This has to come from the widget style I think?
I see no special logic for this inside QToolButton. Cc'ing Casper who might have an
idea about this problem: how to get a toolbutton without inner margins, and what
would be the right icon size for a toolbutton that is put into a menubar?

I tried m_paAnimatedLogo->setContentsMargins(0, 0, 0, 0); but it has no effect,
I think the style is changing the contents margins later on

See commit 817650

I tried:

int size = menuBar()->height();

and the throbber is a little two large, but I think I fixed your problem
-- no boarder; if fills the whole button.

Is it possible to get the height of the Menu font rather than the height
of the menu or is it necessary to use QStyle::PM_MenuBarVMargin to
reduce it?

--
JRT

This is not going very well.

With 12 point type, the Menu bar is 30 pixels tall. However, if I use a
throbber icon that is larger than 20 pixels, the Menu bar gets taller.

And using the pixel height of the font:

int size = QFontInfo(KGlobalSettings::menuFont()).pixelSize() ;

seems to have problems as well since the value is only 17 (which is
correct).

So, I have the attached diff that is only going to help those with high
resolution screens.

This leaves me with questions.

17 pixels is correct for 12 points @ 100 DPI but where does the extra 3
pixels come from and is there a way to obtain this value?

Is there any way to use a throbber that is larger than Qt seems to think
will fit in the Menu bar?

See screen shot which shows a 20x20 throbber (I simply set size = 20 to
try it). There is clearly room for a larger throbber -- 22x22 would
look K.

--
JRT

Index: konqmainwindow.cpp

konqmainwindow.cpp (revision 818056)
konqmainwindow.cpp (working copy)
@@ -3695,7 +3695,17 @@ void KonqMainWindow::initActions()
m_paAnimatedLogo->setFocusPolicy(Qt::NoFocus);
m_paAnimatedLogo->setToolButtonStyle(Qt::ToolButtonI);

- int size = style()->pixelMetric(QStyle::PM_SmallIconSize, NULL, m_paAnimatedLogo);
+ int size = QFontInfo(KGlobalSettings::menuFont()).pixelSize() ;
+
+ if ( size < 22 )
+ size = 16;
+ else if ( size < 32 )
+ size = 22;
+ else if ( size < 48 )
+ size = 32;
+ else if ( size < 64 )
+ size = 48;
+
m_paAnimatedLogo->setIconSize(QSize(size, size));
m_paAnimatedLogo->setIcons("process-working-kde");

James Richard Tyrer wrote:
This is not going very well.

With 12 point type, the Menu bar is 30 pixels tall. However, if I
use a throbber icon that is larger than 20 pixels, the Menu bar gets
taller.

And using the pixel height of the font:

int size = QFontInfo(KGlobalSettings::menuFont()).pixelSize() ;

seems to have problems as well since the value is only 17 (which is
correct).

So, I have the attached diff that is only going to help those with
high resolution screens.

This leaves me with questions.

17 pixels is correct for 12 points @ 100 DPI but where does the extra
3 pixels come from and is there a way to obtain this value?

Is there any way to use a throbber that is larger than Qt seems to
think will fit in the Menu bar?

See screen shot which shows a 20x20 throbber (I simply set size = 20
to try it). There is clearly room for a larger throbber -- 22x22
would look K.

Well, I looked through the Qt dox and the attached patch should work.
Unfortunately, there appears to be a bug in Qt. Still, it works better
than what we have now, and will improve if the Qt bug is fixed.

Should I commit this?

--
JRT



Index: konqmainwindow.cpp

konqmainwindow.cpp (revision 818274)
konqmainwindow.cpp (working copy)
@@ -3695,7 +3695,17 @@ void KonqMainWindow::initActions()
m_paAnimatedLogo->setFocusPolicy(Qt::NoFocus);
m_paAnimatedLogo->setToolButtonStyle(Qt::ToolButtonI);

- int size = style()->pixelMetric(QStyle::PM_SmallIconSize, NULL, m_paAnimatedLogo);
+ int size = (menuBar()->contentsRect()).height();
+
+ if ( size < 22 )
+ size = 16;
+ else if ( size < 32 )
+ size = 22;
+ else if ( size < 48 )
+ size = 32;
+ else if ( size < 64 )
+ size = 48;
+
m_paAnimatedLogo->setIconSize(QSize(size, size));
m_paAnimatedLogo->setIcons("process-working-kde");

David Faure wrote:
Monday 09 June 2008, David Faure wrote:
>return menuBar()->height() - margin - stylePadding;

Ah - this is the problem. The layouting hasn't been done yet, so we can't ask
for menuBar()->height(), it's the default 30 pixels like any QWidget before it's shown
I'll redo this differently.

After reading the dox, I tried this a different way. See attached.
IIUC, Qt will compute the size of the contents rectangle in the MenuBar
so we don't have to do it.

I have gotten rid of the hard coded values.

However, this doesn't work exactly right. :-(

With 12 pt type @ 100 DPI (17 pixel font size) the menu bar is 30 pixels
high. and that appears to be the value which:

(menuBar()->contentsRect()).height();

returns. Since this is 30 pixels, I don't know why I am getting 30 pixels.

So, I tried different font sizes for the Menu font and it does not
change the throbber size. So, the problem is probably as you said that
the layout hasn't been done yet and the 30 pixels is probably the
default height.

IIUC, the problem is that the operations are not in the proper order.

At this point, some help would be appreciated.

--
JRT


Index: konqmainwindow.cpp

konqmainwindow.cpp (revision 818585)
konqmainwindow.cpp (working copy)
@@ -3695,7 +3695,17 @@ void KonqMainWindow::initActions()
m_paAnimatedLogo->setFocusPolicy(Qt::NoFocus);
m_paAnimatedLogo->setToolButtonStyle(Qt::ToolButtonI);

- int size = style()->pixelMetric(QStyle::PM_SmallIconSize, NULL, m_paAnimatedLogo);
+ int size = (menuBar()->contentsRect()).height();
+
+ if ( size < KIconLoader::SizeSmallMedium )
+ size = KIconLoader::SizeSmall;
+ else if ( size < KIconLoader::SizeMedium )
+ size = KIconLoader::SizeSmallMedium;
+ else if ( size < KIconLoader::SizeLarge )
+ size = KIconLoader::SizeMedium ;
+ else if ( size < KIconLoader::SizeHuge )
+ size = KIconLoader::SizeLarge;
+
m_paAnimatedLogo->setIconSize(QSize(size, size));
m_paAnimatedLogo->setIcons("process-working-kde");

David Faure wrote:
Wednesday 11 June 2008, James Richard Tyrer wrote:
>I don't know why I am getting 30 pixels.
>>
>So, I tried different font sizes for the Menu font and it does not
>change the throbber size. So, the problem is probably as you said that
>the layout hasn't been done yet and the 30 pixels is probably the
>default height.

Yes. Please read the code in svn, I fixed all this already. But fredrikh made
me realize that setting an icon size to 18x18 or 19x19 was a bad idea,
the pixmap gets scaled, so now I understand your if()s that round to a known
icon size. I'll merge this in, but I'll also force the button size so that it gets
vertically centered, this is what was bothering me most.

I was having build problems so didn't get to try this till now. It
doesn't seem to work for me. I get a 21x21 icon with a 12 pt. Menu font
and the same with a 24 pt

Yes, the icons sizes should be set to one of the standard values as my
patch does. We have KDE throbber icons in 16x16, 22x22, 32x32 & 48x48.
So, somewhere after 48 we should go to variable size. The code in my
patch:

+ if ( size < KIconLoader::SizeSmallMedium )
+ size = KIconLoader::SizeSmall;
+ else if ( size < KIconLoader::SizeMedium )
+ size = KIconLoader::SizeSmallMedium;
+ else if ( size < KIconLoader::SizeLarge )
+ size = KIconLoader::SizeMedium ;
+ else if ( size < KIconLoader::SizeHuge )
+ size = KIconLoader::SizeLarge;

does this for sizes greater than or equal to "SizeHuge" -- that is,
"SizeHuge" or larger drops through the If-Else structure unchanged. I
don't suppose that we really need to worry about a throbber larger than
48x48 but I was asked to also make an SVG which I have put on my to-do list.

Your code is rather complex. Should we consider it a bug in Qt that
using: "(menuBar()->contentsRect()).height()" doesn't provide us with
the correct number?

Thanks for working on this. It seemed like it would be simple when I
started.

--
JRT

David Faure wrote:
Thursday 12 June 2008, James Richard Tyrer wrote:

>does this for sizes greater than or equal to "SizeHuge"

Duh! I think that that should be:

does this for sizes *less* than or equal to "SizeHuge"

--
JRT