Re: Panel icon sizing bug in 4.4
> There appears to be a bug, or at least a change in behavior for panel icon > sizing in KDE 4.4 (specifically Debian 4.4.3-2). In previous versions of > KDE (e.g., 4.3), panel icons would resize with the panel up to at least > 48x48. However, as of KDE 4.4 panel icons appear to be restricted to 32x32 > in size. As far as I know, it is intentional, that is, it is not a bug. >From my point of view, this is something that we can make configurable (there already is a configuration page in system settings > application appearance > icons > advanced, but the size setting is disabled) This, however, can't be done in 4.5 since we are in the feature freeze. For the time being, the 'cap' could be changed if the rest of the ppl here agree. Cheerio. -- You know, there are many people in the country today who, through no fault of their own, are sane. Some of them were born sane. Some of them became sane later in their lives... -- Monty Python's Flying Circus ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 23, 2010 09:53:15 Mike Kasick wrote: > Hi folks, > > This issue was first raised on the KDE community forums [1], for which > my reply is fifth in the thread. It was recommended that I post the > issue here. > I recommend that you search bko first next time. :) https://bugs.kde.org/show_bug.cgi?id=226039 https://bugs.kde.org/show_bug.cgi?id=201619 and both redirect to https://bugs.kde.org/show_bug.cgi?id=193756 basically, WONTFIX. 177166 looks like it's a dup too... then there's https://bugs.kde.org/show_bug.cgi?id=168579 which was the reason for this change in the first place. one of the bug reports mentioned that KDE's default icon size should be respected, although it's not clear whether that was implemented. if that setting is not followed, then you have found a bug, and patches are welcome. :) -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On 23/05/10 18:08, Ivan Čukić wrote: > > As far as I know, it is intentional, that is, it is not a bug. > > From my point of view, this is something that we can make configurable (there > already is a configuration page in system settings> application appearance> > icons> advanced, *but the size setting is disabled*) My exact response to the highlighted part was to scream "What?!? You WHAT?!?". I went to check this and found that is it true. There is a setting there, it is set to 32 pixels, and unlike the other categories (Desktop, Toolbar, Main Toolbar, etc) is is explicitly disabled. Why would you do that? Why would you set an arbitrary size limit and then explicitly disable the option to change it, while still leaving it there to mock the user? What reason was there? > This, however, can't be done in 4.5 since we are in the feature freeze. Which means it's going to be (to my mind) broken for at least another nine months. Joy. > For the time being, the 'cap' could be changed if the rest of the ppl here > agree. Is this going to be an especially complicated thing if it does get fixed/changed (take your pick)? My question about is quite genuine, I don't see why the option was disabled in the first place, and I certainly can't see how enabling it would be a major and disruptive change. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 23, 2010 10:27:26 Rhapsody wrote: > On 23/05/10 18:08, Ivan Čukić wrote: > > As far as I know, it is intentional, that is, it is not a bug. > > > > From my point of view, this is something that we can make configurable > > (there already is a configuration page in system settings> application > > appearance> icons> advanced, *but the size setting is disabled*) > > My exact response to the highlighted part was to scream "What?!? You > WHAT?!?". I went to check this and found that is it true. There is a > setting there, it is set to 32 pixels, and unlike the other categories > (Desktop, Toolbar, Main Toolbar, etc) is is explicitly disabled. > > Why would you do that? Why would you set an arbitrary size limit and > then explicitly disable the option to change it, while still leaving it > there to mock the user? What reason was there? sounds like a bug to me. and if it's a bug, it can be fixed (and might even be a trivial fix). it's probably worth checking whether there was a reason, though. Ivan, since you've already found the config option, do you know who put it there (or, who disabled it)? -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 23, 2010 11:02:33 Chani wrote: > On May 23, 2010 10:27:26 Rhapsody wrote: > > On 23/05/10 18:08, Ivan Čukić wrote: > > > As far as I know, it is intentional, that is, it is not a bug. > > > > > > From my point of view, this is something that we can make configurable > > > (there already is a configuration page in system settings> application > > > appearance> icons> advanced, *but the size setting is disabled*) > > wait a sec... it's not greyed out for me. I can change the icon size. however, X randomly crashed before I could test whether plasma follows it. now I'm pissed off at my computer, so I'm gonna go do something else. -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
> wait a sec... it's not greyed out for me. I can change the icon size. > however, X randomly crashed before I could test whether plasma follows it. > now I'm pissed off at my computer, so I'm gonna go do something else. Strange, for the panel icon, it was disabled (for me) since I can remember :) Cheerio, Ivan -- Sanity is the trademark of a weak mind. -- Mark Harrold ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Sunday 23 May 2010, Chani wrote: > On May 23, 2010 10:27:26 Rhapsody wrote: > > On 23/05/10 18:08, Ivan Čukić wrote: > > > As far as I know, it is intentional, that is, it is not a bug. > > > > > > From my point of view, this is something that we can make configurable > > > (there already is a configuration page in system settings> application > > > appearance> icons> advanced, *but the size setting is disabled*) > > > > My exact response to the highlighted part was to scream "What?!? You > > WHAT?!?". I went to check this and found that is it true. There is a > > setting there, it is set to 32 pixels, and unlike the other categories > > (Desktop, Toolbar, Main Toolbar, etc) is is explicitly disabled. > > > > Why would you do that? Why would you set an arbitrary size limit and > > then explicitly disable the option to change it, while still leaving it > > there to mock the user? What reason was there? > > sounds like a bug to me. > and if it's a bug, it can be fixed (and might even be a trivial fix). > > it's probably worth checking whether there was a reason, though. Ivan, > since you've already found the config option, do you know who put it there > (or, who disabled it)? so, the reason was that vertical panels on widescreens are usually huge to fit the taskbar, if icons would be unlimited they would quite useless, like kickoff using half of the screen. now yes, it should follow that setting, no it doesn't, never followed it in kde4 and should do before 4.5 cheers, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 23, 2010 13:08:57 Marco Martin wrote: > On Sunday 23 May 2010, Chani wrote: > > On May 23, 2010 10:27:26 Rhapsody wrote: > > > On 23/05/10 18:08, Ivan Čukić wrote: > > > > As far as I know, it is intentional, that is, it is not a bug. > > > > > > > > From my point of view, this is something that we can make > > > > configurable (there already is a configuration page in system > > > > settings> application appearance> icons> advanced, *but the size > > > > setting is disabled*) > > > > > > My exact response to the highlighted part was to scream "What?!? You > > > WHAT?!?". I went to check this and found that is it true. There is a > > > setting there, it is set to 32 pixels, and unlike the other categories > > > (Desktop, Toolbar, Main Toolbar, etc) is is explicitly disabled. > > > > > > Why would you do that? Why would you set an arbitrary size limit and > > > then explicitly disable the option to change it, while still leaving it > > > there to mock the user? What reason was there? > > > > sounds like a bug to me. > > and if it's a bug, it can be fixed (and might even be a trivial fix). > > > > it's probably worth checking whether there was a reason, though. Ivan, > > since you've already found the config option, do you know who put it > > there (or, who disabled it)? > > so, > the reason was that vertical panels on widescreens are usually huge to fit > the taskbar, if icons would be unlimited they would quite useless, like > kickoff using half of the screen. > now yes, it should follow that setting, no it doesn't, never followed it in > kde4 and should do before 4.5 > not the reason for the panel thing (that was already explained enough times) - the reason for the default icon size setting in the KCM being disabled on ivan's computer. -- This message brought to you by eevil bananas and the number 3. www.chani3.com signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Sun, May 23, 2010 at 10:28:00AM -0700, Chani wrote:
> one of the bug reports mentioned that KDE's default icon size should be
> respected, although it's not clear whether that was implemented.
It wasn't.
> if that setting is not followed, then you have found a bug, and patches
> are welcome. :)
OK, attached are three fairly trivial patches.
The first patch, against kdebase-runtime (both Debian 4.4.3-2 and KDE 4.5
SVN trunk) reenables the ability to change the "Panel" icon size in
kcm_icons. It also disables the "Animate Icons" checkbox as that checkbox
is disabled for most of the other icon types, and appers to be obsolete in
KDE 4.
The second and third patches do the same thing, but the second applies to
Debian 4.4.3-2 while the third applies to KDE 4.5 SVN trunk. Both patches,
against kdelibs, changes IconWidget::sizeHint to return
IconSize(KIconLoader::Panel) instead of KIconLoader::SizeMedium when no
preferred icon size has already been set (4.5 only). Note that
KIconLoader::Panel defaults to KIconLoader::SizeMedium, so there's no
change unless someone modifies the setting in the Appearance dialog.
I've tested the first two patches against Debian 4.4.3-2 and they resolve
the problem, allowing the user to configure the capped panel icon size.
plasma-desktop has to be restarted for a new capped icon size to take
effect, since IconWidget::sizeHint is only queried on applet
initialization. The effect is also immediate (without plasma-desktop
restart) on any new applet added to the panel.
I've not tested the patches against KDE SVN trunk, I've just ensured they
apply cleanly. Unfortunately I'm not setup to build and test trunk right
now, if someone else can try these patches that would be great. If you do
a partial build, the modified output files are kcm_icons.so and
libplasma.so.3.0.0.
So actually, I haven't confirmed that the icon size cap problem exists in
KDE 4.5. I suspect it does, since it's definitely not user configurable
right now. It looks like a bunch of code was added to IconWidget::sizeHint
to allow individual applets to set a preferred icon size. However the
"icon" and "kickoff" applets appear not to touch this.
Are the first two patches fair game for KDE 4.4.4? I'd really love to see
a user configurable panel icon size in Debian for accesibility purposes,
and 4.5 is going to hit too late for squeeze.
I'm assuming the first and third patches are appropriate for KDE 4.5.
Again, they're fairly trivial, but need to be tested.
Thanks everyone!
diff --git a/kcontrol/icons/icons.cpp b/kcontrol/icons/icons.cpp
--- a/kcontrol/icons/icons.cpp
+++ b/kcontrol/icons/icons.cpp
@@ -409,10 +409,10 @@
return;
mUsage = index;
-if ( mUsage == KIconLoader::Panel || mUsage == KIconLoader::LastGroup )
+if ( mUsage == KIconLoader::LastGroup )
{
mpSizeBox->setEnabled(false);
- mpAnimatedCheck->setEnabled( mUsage == KIconLoader::Panel );
+ mpAnimatedCheck->setEnabled(false);
}
else
{
diff --git a/plasma/widgets/iconwidget.cpp b/plasma/widgets/iconwidget.cpp
--- a/plasma/widgets/iconwidget.cpp
+++ b/plasma/widgets/iconwidget.cpp
@@ -604,7 +604,7 @@
QSizeF IconWidget::sizeHint(Qt::SizeHint which, const QSizeF & constraint) const
{
if (which == Qt::PreferredSize) {
-return sizeFromIconSize(KIconLoader::SizeMedium);
+return sizeFromIconSize(IconSize(KIconLoader::Panel));
} else if (which == Qt::MinimumSize) {
return sizeFromIconSize(KIconLoader::SizeSmall);
} else {
diff --git a/plasma/widgets/iconwidget.cpp b/plasma/widgets/iconwidget.cpp
--- a/plasma/widgets/iconwidget.cpp
+++ b/plasma/widgets/iconwidget.cpp
@@ -674,7 +674,7 @@
if (d->preferredIconSize.isValid()) {
return sizeFromIconSize(qMax(d->preferredIconSize.height(), d->preferredIconSize.height()));
}
-int iconSize = KIconLoader::SizeMedium;
+int iconSize = IconSize(KIconLoader::Panel);
if (d->iconSvg) {
QSizeF oldSize = d->iconSvg->size();
d->iconSvg->resize();
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Monday 24 May 2010, Mike Kasick wrote: > On Sun, May 23, 2010 at 10:28:00AM -0700, Chani wrote: > > one of the bug reports mentioned that KDE's default icon size should be > > respected, although it's not clear whether that was implemented. > > It wasn't. > > > if that setting is not followed, then you have found a bug, and patches > > are welcome. :) > > OK, attached are three fairly trivial patches. > > The first patch, against kdebase-runtime (both Debian 4.4.3-2 and KDE 4.5 > SVN trunk) reenables the ability to change the "Panel" icon size in > kcm_icons. It also disables the "Animate Icons" checkbox as that checkbox > is disabled for most of the other icon types, and appers to be obsolete in > KDE 4. > > The second and third patches do the same thing, but the second applies to > Debian 4.4.3-2 while the third applies to KDE 4.5 SVN trunk. Both patches, > against kdelibs, changes IconWidget::sizeHint to return > IconSize(KIconLoader::Panel) instead of KIconLoader::SizeMedium when no > preferred icon size has already been set (4.5 only). Note that > KIconLoader::Panel defaults to KIconLoader::SizeMedium, so there's no > change unless someone modifies the setting in the Appearance dialog. > > I've tested the first two patches against Debian 4.4.3-2 and they resolve > the problem, allowing the user to configure the capped panel icon size. > plasma-desktop has to be restarted for a new capped icon size to take > effect, since IconWidget::sizeHint is only queried on applet > initialization. The effect is also immediate (without plasma-desktop > restart) on any new applet added to the panel. > > I've not tested the patches against KDE SVN trunk, I've just ensured they > apply cleanly. Unfortunately I'm not setup to build and test trunk right > now, if someone else can try these patches that would be great. If you do > a partial build, the modified output files are kcm_icons.so and > libplasma.so.3.0.0. > > So actually, I haven't confirmed that the icon size cap problem exists in > KDE 4.5. I suspect it does, since it's definitely not user configurable > right now. It looks like a bunch of code was added to IconWidget::sizeHint > to allow individual applets to set a preferred icon size. However the > "icon" and "kickoff" applets appear not to touch this. > > Are the first two patches fair game for KDE 4.4.4? I'd really love to see > a user configurable panel icon size in Debian for accesibility purposes, > and 4.5 is going to hit too late for squeeze. > > I'm assuming the first and third patches are appropriate for KDE 4.5. > Again, they're fairly trivial, but need to be tested. > > Thanks everyone! thanks for those. they should go in trunk for sure. the first at least. probably it can be made harmless enough for trunk. about the second and the third, it's not the proper place to do that iconwidget is used really everywhere, not only on panels but also in layouts of applets that can be anywhere else in popups etc. What should be modified i think is the size hint of the Icon applet, in workspace, that is explicitly used here and should have a sizehint of Panel in vertical/horizontal formfactors and Desktop in Planar Cheers, Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Mon, May 24, 2010 at 10:56:53AM +0200, Marco Martin wrote:
> What should be modified i think is the size hint of the Icon applet, in
> workspace, that is explicitly used here
> and should have a sizehint of Panel in vertical/horizontal formfactors and
> Desktop in Planar
OK. Attached is a patch against KDE 4.5 trunk, which I've compiled and
tested.
It does two things, (i) reenable the setting of the Panel icon size (the
kcm_icons patch I've submitted previously), and (ii) add sizeHint methods
to MenuLauncherApplet (the simple/old-style kickoff), IconApplet, and
PopupApplet--the last with the intention of fixing LauncherApplet (the
new-style kickoff) and any other popup-based applets that use an icon when
in "small form".
Seems to do the trick on my end. Your thoughts?
diff --git a/kdebase/runtime/kcontrol/icons/icons.cpp b/kdebase/runtime/kcontrol/icons/icons.cpp
--- a/kdebase/runtime/kcontrol/icons/icons.cpp
+++ b/kdebase/runtime/kcontrol/icons/icons.cpp
@@ -409,10 +409,10 @@
return;
mUsage = index;
-if ( mUsage == KIconLoader::Panel || mUsage == KIconLoader::LastGroup )
+if ( mUsage == KIconLoader::LastGroup )
{
mpSizeBox->setEnabled(false);
- mpAnimatedCheck->setEnabled( mUsage == KIconLoader::Panel );
+ mpAnimatedCheck->setEnabled(false);
}
else
{
diff --git a/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp b/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp
--- a/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp
+++ b/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp
@@ -787,4 +787,24 @@
return d->actions;
}
+QSizeF MenuLauncherApplet::sizeHint(Qt::SizeHint which, const QSizeF &constraint) const
+{
+if (which == Qt::PreferredSize) {
+ int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSize(KIconLoader::Desktop);
+return QSizeF(iconSize, iconSize);
+case Plasma::Horizontal:
+case Plasma::Vertical:
+iconSize = IconSize(KIconLoader::Panel);
+return QSizeF(iconSize, iconSize);
+}
+}
+
+return Plasma::Applet::sizeHint(which, constraint);
+}
+
#include "simpleapplet.moc"
diff --git a/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h b/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h
--- a/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h
+++ b/kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h
@@ -125,6 +125,7 @@
* Create a configuration dialog.
*/
void createConfigurationInterface(KConfigDialog *parent);
+QSizeF sizeHint(Qt::SizeHint which, const QSizeF &constraint = QSizeF()) const;
private Q_SLOTS:
/// Configuration-dialog accepted.
diff --git a/kdebase/workspace/plasma/generic/applets/icon/icon.cpp b/kdebase/workspace/plasma/generic/applets/icon/icon.cpp
--- a/kdebase/workspace/plasma/generic/applets/icon/icon.cpp
+++ b/kdebase/workspace/plasma/generic/applets/icon/icon.cpp
@@ -457,5 +457,25 @@
}
}
+QSizeF IconApplet::sizeHint(Qt::SizeHint which, const QSizeF &constraint) const
+{
+if (which == Qt::PreferredSize) {
+ int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSize(KIconLoader::Desktop);
+return QSizeF(iconSize, iconSize);
+case Plasma::Horizontal:
+case Plasma::Vertical:
+iconSize = IconSize(KIconLoader::Panel);
+return QSizeF(iconSize, iconSize);
+}
+}
+
+return Plasma::Applet::sizeHint(which, constraint);
+}
+
#include "icon.moc"
diff --git a/kdebase/workspace/plasma/generic/applets/icon/icon.h b/kdebase/workspace/plasma/generic/applets/icon/icon.h
--- a/kdebase/workspace/plasma/generic/applets/icon/icon.h
+++ b/kdebase/workspace/plasma/generic/applets/icon/icon.h
@@ -57,6 +57,7 @@
void dropEvent(QGraphicsSceneDragDropEvent *event);
void saveState(KConfigGroup &cg) const;
void showConfigurationInterface();
+QSizeF sizeHint(Qt::SizeHint which, const QSizeF &constraint = QSizeF()) const;
private slots:
void acceptedPropertiesDialog();
diff --git a/kdelibs/plasma/popupapplet.cpp b/kdelibs/plasma/popupapplet.cpp
--- a/kdelibs/plasma/popupapplet.cpp
+++ b/kdelibs/plasma/popupapplet.cpp
@@ -856,6 +856,26 @@
dialog->move(pos);
}
+QSizeF PopupApplet::sizeHint(Qt::SizeHint which, const QSizeF &constraint) const
+{
+if (which == Qt::PreferredSize) {
+ int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSize(KIconLoader::Desktop);
+return QSizeF(iconSize, iconSize);
+case Plasma::Horiz
Re: Panel icon sizing bug in 4.4
This will be needed in Plasma::PopupApplet as well. I'll can do it when this gets accepted. Cheerio -- While you were hanging yourself on someone else's words Dying to believe in what you heard I was staring straight into the shining sun ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
> Seems to do the trick on my end. Your thoughts? popupapplet:866 When on desktop, popup applet is not an icon, so it should return Plasma::Applet::sizeHint(...) Other things seem ok Cheerio, Ivan -- The bleeding hearts and artists, Make their stand. -- Pink Floyd ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
> Seems to do the trick on my end. Your thoughts? p.s. Do you have SVN account? Cheerio, Ivan -- You know, there are many people in the country today who, through no fault of their own, are sane. Some of them were born sane. Some of them became sane later in their lives... -- Monty Python's Flying Circus ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 23, 2010, Rhapsody wrote: > On 23/05/10 18:08, Ivan Čukić wrote: > > As far as I know, it is intentional, that is, it is not a bug. > > > > From my point of view, this is something that we can make configurable > > (there already is a configuration page in system settings> application > > appearance> icons> advanced, *but the size setting is disabled*) > > My exact response to the highlighted part was to scream "What?!? You discuss things constructively with facts. keep the posturing and hysterics for the user forums and blog comments. this mailing list is a productivity zone, and i do not tolerate it straying from that. thank you for your understanding and cooperation. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 24, 2010, Mike Kasick wrote:
> On Mon, May 24, 2010 at 10:56:53AM +0200, Marco Martin wrote:
> > What should be modified i think is the size hint of the Icon applet, in
> > workspace, that is explicitly used here
> > and should have a sizehint of Panel in vertical/horizontal formfactors
> > and Desktop in Planar
>
> OK. Attached is a patch against KDE 4.5 trunk, which I've compiled and
> tested.
>
> It does two things, (i) reenable the setting of the Panel icon size (the
> kcm_icons patch I've submitted previously),
personally, i'd just remove the animate checkbox from that control panel
altogether. otherwise, this looks fine.
> and (ii) add sizeHint methods
> to MenuLauncherApplet (the simple/old-style kickoff), IconApplet, and
the patch to applets/icon/icon.h looks ok.
kickoff is a popup applet should be handled by PopupApplet and so shouldn't be
patched for this.
the PopupApplet patch needs work, however. in PopupApplet::sizeHint, it should
probably be something like:
if (!d->icon) {
return Applet::sizeHint(which, constraint);
}
switch (formFactor()) {
}
that should work properly.
finally, both the Icon applet and the PopupApplet class will need to connect
to KGlobalSettings::self()->iconChanged(int group) and when the group that is
changed is the Panel or Desktop groups, then an updateGeometry() call will
need to be made. this signal should be connected to a Q_PRIVATE_SLOT in the
PopupApplet case.
thanks for the patches, especially revising them in response to feedback; i
think the easiest thing to do from here will be for ivan and i to triage them
into svn and get them commited for beta2. cheers :)
--
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43
KDE core developer sponsored by Qt Development Frameworks
signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Tue, May 25, 2010 at 10:20:46AM -0700, Aaron J. Seigo wrote:
> kickoff is a popup applet should be handled by PopupApplet and so shouldn't
> be
> patched for this.
The "new-style" kickoff is indeed, I don't think the patches touch
the regular LauncherApplet. The "old-style" kickoff (better name for
this? simpleapplet? Class MenuLauncherApplet anyways.) isn't a
PopupApplet hence why I added the code there.
> the PopupApplet patch needs work, however. in PopupApplet::sizeHint, it
> should
> probably be something like:
>
> if (!d->icon) {
> return Applet::sizeHint(which, constraint);
> }
So actually, I wasn't quite sure if I had to do anything more when
converting iconSize to a QSizeF. In IconWidget and elsewhere
IconWidget::sizeFromIconSize is used (e.g., icons.cpp:63).
Unfortunately I couldn't use m_icon->sizeFromIconSize in IconApplet (or
d->icon elsewhere) since sizeHint is called the first time before these are
initialized, and I didn't want to muck with the initialization order. I
checked on my build, and it appears IconWidget::sizeFromIconSize doesn't
actually change the size for these particular icons though, so it's
equivalent to QSizeF(iconSize, iconSize).
Anyways, what I didn't know was if sizeHint is only queried once, or if it
gets queried a couple of times. If it's only queried once, it needs to
return the "right" size even when the respective IconWidgets are
uninitialized. Perhaps that's related to the iconChanged bit?
> thanks for the patches, especially revising them in response to feedback;
No problem, wanted to do it right anyways.
> think the easiest thing to do from here will be for ivan and i to triage
> them into svn and get them commited for beta2. cheers :)
Is there a chance this will make it into 4.4.4? If not, I might want to
grab a copy of the final patch set and see if I can get it slipped into
Debian, since they won't ship 4.5 for the next stable release.
Figure there's likely to be a single SVN commit I can grab? Otherwise I
can take a look at the commit log and grab out the relevant pieces.
Thanks everyone!
P.S.: While it's on my mind, I think I see another bug in
IconWidget::sizeHint (kdelibs/plasma/widgets/iconwidget.cpp:675,691,696).
The three return lines in this method all take "qMax(...height(), ...height()",
I suspect that's supposed to be "qMax(...width(), ...height()".
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On May 25, 2010, Mike Kasick wrote:
> On Tue, May 25, 2010 at 10:20:46AM -0700, Aaron J. Seigo wrote:
> > kickoff is a popup applet should be handled by PopupApplet and so
> > shouldn't be patched for this.
>
> The "new-style" kickoff is indeed, I don't think the patches touch
> the regular LauncherApplet. The "old-style" kickoff (better name for
> this? simpleapplet? Class MenuLauncherApplet anyways.) isn't a
> PopupApplet hence why I added the code there.
>
> > the PopupApplet patch needs work, however. in PopupApplet::sizeHint, it
> > should probably be something like:
> >
> > if (!d->icon) {
> >
> > return Applet::sizeHint(which, constraint);
> >
> > }
>
> So actually, I wasn't quite sure if I had to do anything more when
> converting iconSize to a QSizeF. In IconWidget and elsewhere
> IconWidget::sizeFromIconSize is used (e.g., icons.cpp:63).
good question; sizeFromIconSize basically answers the question, "with the
given text for this icon, how bit does the whole QGraphicsWidget need to be to
show both the text and the icon at that size"? in these cases no text is being
shown, so we don't really care.
> Is there a chance this will make it into 4.4.4? If not, I might want to
i highly doubt it, sorry :(
> Figure there's likely to be a single SVN commit I can grab? Otherwise I
> can take a look at the commit log and grab out the relevant pieces.
there was one commit per set of files changed; so one for PopupApplet in
libplasma, one for kickoff, one for lancelot (iirc?) and one for the control
panel.
> P.S.: While it's on my mind, I think I see another bug in
> IconWidget::sizeHint (kdelibs/plasma/widgets/iconwidget.cpp:675,691,696).
> The three return lines in this method all take "qMax(...height(),
> ...height()", I suspect that's supposed to be "qMax(...width(),
> ...height()".
yes, looks like a typo. nice catch.
--
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43
KDE core developer sponsored by Qt Development Frameworks
signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Tue, May 25, 2010 at 12:38:44PM -0700, Aaron J. Seigo wrote:
> there was one commit per set of files changed; so one for PopupApplet in
> libplasma, one for kickoff, one for lancelot (iirc?) and one for the control
> panel.
I tested these out, they work great!
One more issue: the "simple" Application Launcher Menu (which I prefer to
kickoff) needs to be fixed too. Attached is a patch based on the Icon
applet changes that fixes the MenuLauncher applet too.
Thanks!
Index: workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp
===
--- workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp (revision 1130567)
+++ workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp (working copy)
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -371,6 +372,9 @@
}
constraintsEvent(Plasma::ImmutableConstraint);
+
+connect(KGlobalSettings::self(), SIGNAL(iconChanged(int)),
+this, SLOT(iconSizeChanged(int)));
}
void MenuLauncherApplet::constraintsEvent(Plasma::Constraints constraints)
@@ -787,4 +791,34 @@
return d->actions;
}
+void MenuLauncherApplet::iconSizeChanged(int group)
+{
+if (group == KIconLoader::Desktop || group == KIconLoader::Panel) {
+updateGeometry();
+}
+}
+
+QSizeF MenuLauncherApplet::sizeHint(Qt::SizeHint which, const QSizeF & constraint) const
+{
+if (which == Qt::PreferredSize) {
+int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSize(KIconLoader::Desktop);
+break;
+
+case Plasma::Horizontal:
+case Plasma::Vertical:
+iconSize = IconSize(KIconLoader::Panel);
+break;
+}
+
+return QSizeF(iconSize, iconSize);
+}
+
+return Plasma::Applet::sizeHint(which, constraint);
+}
+
#include "simpleapplet.moc"
Index: workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h
===
--- workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h (revision 1130567)
+++ workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h (working copy)
@@ -125,6 +125,7 @@
* Create a configuration dialog.
*/
void createConfigurationInterface(KConfigDialog *parent);
+QSizeF sizeHint(Qt::SizeHint which, const QSizeF & constraint = QSizeF()) const;
private Q_SLOTS:
/// Configuration-dialog accepted.
@@ -133,6 +134,7 @@
void toggleMenu(bool pressed = true);
/// An action within the menu got triggered.
void actionTriggered(QAction *action);
+void iconSizeChanged(int group);
private:
class Private;
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
> One more issue: the "simple" Application Launcher Menu (which I prefer to > kickoff) needs to be fixed too. Attached is a patch based on the Icon > applet changes that fixes the MenuLauncher applet too. Sorry, my bad, forgot to commit the changes... it should be ok now. Cheerio, Ivan -- While you were hanging yourself on someone else's words Dying to believe in what you heard I was staring straight into the shining sun -- Pink Floyd ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Sun, May 23, 2010 at 9:29 PM, Mike Kasick wrote: > On Sun, May 23, 2010 at 10:28:00AM -0700, Chani wrote: > >> one of the bug reports mentioned that KDE's default icon size should be >> respected, although it's not clear whether that was implemented. > > It wasn't. > >> if that setting is not followed, then you have found a bug, and patches >> are welcome. :) > > OK, attached are three fairly trivial patches. > > The first patch, against kdebase-runtime (both Debian 4.4.3-2 and KDE 4.5 > SVN trunk) reenables the ability to change the "Panel" icon size in > kcm_icons. It also disables the "Animate Icons" checkbox as that checkbox > is disabled for most of the other icon types, and appers to be obsolete in > KDE 4. > > The second and third patches do the same thing, but the second applies to > Debian 4.4.3-2 while the third applies to KDE 4.5 SVN trunk. Both patches, > against kdelibs, changes IconWidget::sizeHint to return > IconSize(KIconLoader::Panel) instead of KIconLoader::SizeMedium when no > preferred icon size has already been set (4.5 only). Note that > KIconLoader::Panel defaults to KIconLoader::SizeMedium, so there's no > change unless someone modifies the setting in the Appearance dialog. > > I've tested the first two patches against Debian 4.4.3-2 and they resolve > the problem, allowing the user to configure the capped panel icon size. > plasma-desktop has to be restarted for a new capped icon size to take > effect, since IconWidget::sizeHint is only queried on applet > initialization. The effect is also immediate (without plasma-desktop > restart) on any new applet added to the panel. > > I've not tested the patches against KDE SVN trunk, I've just ensured they > apply cleanly. Unfortunately I'm not setup to build and test trunk right > now, if someone else can try these patches that would be great. If you do > a partial build, the modified output files are kcm_icons.so and > libplasma.so.3.0.0. > > So actually, I haven't confirmed that the icon size cap problem exists in > KDE 4.5. I suspect it does, since it's definitely not user configurable > right now. It looks like a bunch of code was added to IconWidget::sizeHint > to allow individual applets to set a preferred icon size. However the > "icon" and "kickoff" applets appear not to touch this. > > Are the first two patches fair game for KDE 4.4.4? I'd really love to see > a user configurable panel icon size in Debian for accesibility purposes, > and 4.5 is going to hit too late for squeeze. > > I'm assuming the first and third patches are appropriate for KDE 4.5. > Again, they're fairly trivial, but need to be tested. > > Thanks everyone! I apologize of this is a silly question, or already discussed, but if the problem is the width of vertical panels, why don't we just have the size hint depend on whether the panel is vertical or horizontal? So for horizontal panels it resizes freely, but for vertical panels it only goes up to the maximum size specified? Wouldn't that fix the problems with the vertical panel while not making the horizontal panel any more difficult to use? -Todd ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On June 2, 2010, todd rme wrote: > So for horizontal panels it resizes freely, but for vertical panels it > only goes up to the maximum size specified? Wouldn't that fix the because that doesn't address the issue that some people want LARGE icons in a horizontal panel and others don't. the difference between this and when the exact same approach was written into kicker is that i'm not doing it on my own this time. which is nice (if a bit more verbose :) -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On Tue, May 25, 2010 at 12:38:44PM -0700, Aaron J. Seigo wrote:
> > Is there a chance this will make it into 4.4.4? If not, I might want to
>
> i highly doubt it, sorry :(
Thanks for your effors in getting this into trunk. The latest revisions
(with MenuLauncherApplet::sizeHint defined const) work great!
I've gone through SVN and cherry-picked the patches for each applet and
backported them to the KDE 4.4 SVN branch--keeping just the changes
relevant to iconSizeChanged/sizeHint as well as setting of the Panel icon
size in kcm_icons. Attached is a single, backported, tested patch against
4.4/KDE.
Would it be possible to commit this to the 4.4 branch? I'd love to see
this in Debian, and KDE 4.4.5 is the path of least resistance on that.
Thanks!
Index: kdebase/runtime/kcontrol/icons/icons.cpp
===
--- kdebase/runtime/kcontrol/icons/icons.cpp (revision 1134187)
+++ kdebase/runtime/kcontrol/icons/icons.cpp (working copy)
@@ -409,10 +409,10 @@
return;
mUsage = index;
-if ( mUsage == KIconLoader::Panel || mUsage == KIconLoader::LastGroup )
+if ( mUsage == KIconLoader::LastGroup )
{
mpSizeBox->setEnabled(false);
- mpAnimatedCheck->setEnabled( mUsage == KIconLoader::Panel );
+mpAnimatedCheck->setEnabled(false);
}
else
{
Index: kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp
===
--- kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp (revision 1134187)
+++ kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp (working copy)
@@ -371,6 +371,9 @@
}
constraintsEvent(Plasma::ImmutableConstraint);
+
+connect(KGlobalSettings::self(), SIGNAL(iconChanged(int)),
+this, SLOT(iconSizeChanged(int)));
}
void MenuLauncherApplet::constraintsEvent(Plasma::Constraints constraints)
@@ -787,4 +790,35 @@
return d->actions;
}
+void MenuLauncherApplet::iconSizeChanged(int group)
+{
+if (group == KIconLoader::Desktop || group == KIconLoader::Panel) {
+updateGeometry();
+}
+}
+
+QSizeF MenuLauncherApplet::sizeHint(Qt::SizeHint which, const QSizeF & constraint) const
+{
+if (which == Qt::PreferredSize) {
+int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSize(KIconLoader::Desktop);
+break;
+
+case Plasma::Horizontal:
+case Plasma::Vertical:
+iconSize = IconSize(KIconLoader::Panel);
+break;
+}
+
+return QSizeF(iconSize, iconSize);
+}
+
+return Plasma::Applet::sizeHint(which, constraint);
+}
+
+
#include "simpleapplet.moc"
Index: kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h
===
--- kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h (revision 1134187)
+++ kdebase/workspace/plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.h (working copy)
@@ -126,6 +126,8 @@
*/
void createConfigurationInterface(KConfigDialog *parent);
+QSizeF sizeHint(Qt::SizeHint which, const QSizeF & constraint = QSizeF()) const;
+
private Q_SLOTS:
/// Configuration-dialog accepted.
void configAccepted();
@@ -133,6 +135,8 @@
void toggleMenu(bool pressed = true);
/// An action within the menu got triggered.
void actionTriggered(QAction *action);
+/// Icon size setting changed
+void iconSizeChanged(int group);
private:
class Private;
Index: kdebase/workspace/plasma/generic/applets/icon/icon.cpp
===
--- kdebase/workspace/plasma/generic/applets/icon/icon.cpp (revision 1134187)
+++ kdebase/workspace/plasma/generic/applets/icon/icon.cpp (working copy)
@@ -27,6 +27,7 @@
#include
#include
+#include
#include
#include
#include
@@ -84,6 +85,9 @@
setDisplayLines(2);
registerAsDragHandle(m_icon);
setAspectRatioMode(Plasma::ConstrainedSquare);
+
+connect(KGlobalSettings::self(), SIGNAL(iconChanged(int)),
+this, SLOT(iconSizeChanged(int)));
}
IconApplet::~IconApplet()
@@ -110,6 +114,13 @@
}
}
+void IconApplet::iconSizeChanged(int group)
+{
+if (group == KIconLoader::Desktop || group == KIconLoader::Panel) {
+updateGeometry();
+}
+}
+
void IconApplet::setUrl(const KUrl& url)
{
m_url = KIO::NetAccess::mostLocalUrl(url, 0);
@@ -252,6 +263,29 @@
}
}
+QSizeF IconApplet::sizeHint(Qt::SizeHint which, const QSizeF & constraint) const
+{
+if (which == Qt::PreferredSize) {
+int iconSize;
+
+switch (formFactor()) {
+case Plasma::Planar:
+case Plasma::MediaCenter:
+iconSize = IconSi
Re: Panel icon sizing bug in 4.4
On Thu, Jun 03, 2010 at 07:50:24PM -0400, Mike Kasick wrote: > I've gone through SVN and cherry-picked the patches for each applet and > backported them to the KDE 4.4 SVN branch--keeping just the changes > relevant to iconSizeChanged/sizeHint as well as setting of the Panel icon > size in kcm_icons. Attached is a single, backported, tested patch against > 4.4/KDE. > > Would it be possible to commit this to the 4.4 branch? I'd love to see > this in Debian, and KDE 4.4.5 is the path of least resistance on that. Sorry, is there a more appropriate list to discuss the 4.4 branch? Thanks! ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Panel icon sizing bug in 4.4
On June 3, 2010, Mike Kasick wrote: > Would it be possible to commit this to the 4.4 branch? I'd love to see i think most of us are stupidly busy as it is right now getting 4.5 out. if you can test these changes against 4.4 and confirm they work properly, then please commit. at least i don't have any time right now to be working on the 4.4 branch. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
