Bug 195186 - OSD font uses absolute point size. should be relative
Summary: OSD font uses absolute point size. should be relative
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Notifications (show other bugs)
Version: 2.3.2
Platform: unspecified Linux
: NOR normal with 21 votes (vote)
Target Milestone: 2.4.0
Assignee: Amarok Developers
URL:
Keywords:
: 197282 219994 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-06-04 11:18 UTC by Gandalf Lechner
Modified: 2011-01-15 10:49 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.4.1


Attachments
screenshot (54.24 KB, image/jpeg)
2009-06-04 13:48 UTC, Gandalf Lechner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gandalf Lechner 2009-06-04 11:18:14 UTC
Version:           2.1 (using 4.2.4 (KDE 4.2.4), Kubuntu packages)
Compiler:          cc
OS:                Linux (x86_64) release 2.6.28-12-generic

It would be nice if the font type and font size used for the OSD could be adjusted by the user. I think it was like this in the 1.x series. In the 2.x series, one can still edit the amarokrc config file to adjust the OSD settings, but a more convenient solution within the OSD configuration dialog would be nicer.
Comment 1 Gandalf Lechner 2009-06-04 13:48:31 UTC
Created attachment 34262 [details]
screenshot

PS: The OSD would look even better if the album cover would scale according to the font size: If you use a big font size, a small album cover doesn't fit well. In many cases, the available cover image is quite big, and in that case it could be shown in a size adapted to the font size.

See attached screenshot for an example with a big font and somewhat too small cover.
Comment 2 Martin Sandsmark 2009-06-20 17:14:28 UTC
*** Bug 197282 has been marked as a duplicate of this bug. ***
Comment 3 Mike Bridge 2009-10-06 16:47:20 UTC
The OSD takes up 1/8th of my (10.1" netbook) screen!
Comment 4 Mark Kretschmann 2009-10-07 11:19:49 UTC
Actually this is a bug in Amarok, I've just noticed. We are setting a fixed point size in the OSD:

    QFont f = font();
    f.setPointSize( 16 );

It should use a relative size instead. I will fix this.
Comment 5 Mark Kretschmann 2009-10-07 12:07:53 UTC
I've fixed it. Please test.


commit 0098ad409e62d0ab9eef3909cf0f621bedf31124
Author: Mark Kretschmann <kretschmann@kde.org>
Date:   Wed Oct 7 12:01:50 2009 +0200

    Use relative font size for OSD, usable with all display resolutions.

    Never hardcode font sizes! It makes netbook users very unhappy.

    BUG: 195186
Comment 6 Mike Bridge 2009-10-09 20:59:42 UTC
I'm sorry Mark, I've got amarok-nightly (20091008+svn0e3f8489b19ea258d1eb993cee8a8e1867b56c69-0neon1) and I don't see a change/don't know what i'm doing..
Comment 7 Myriam Schweingruber 2009-12-07 11:29:59 UTC
Indeed, that is not fixed in 2.2.1 while it should be in there.
I also can't find any indication in the ChangeLog that this was ever fixed...
Comment 8 Mikko C. 2009-12-24 20:36:58 UTC
*** Bug 219994 has been marked as a duplicate of this bug. ***
Comment 9 Soren Harward 2010-05-10 02:35:58 UTC
Mark "fixed" this in 3ecf082b .  The OSD font size is set as "normal + 8pt".  It may be better off as "normal * [percentage]".  In which case, widgets/Osd.cpp:91 should be changed.
Comment 10 Andrew Gaydenko 2010-09-23 02:36:48 UTC
I'm agree, the bug isn't fixed (as well as all duplicates). All user's claims are related to having an opportunity to *set* OSD font size and colors, rather to have predefined (precalculated) one this or that common way. There is a killer use case wich shows obvious that the problem still exists. I mean LIRCing. Currently at 2-3 meters from display I need binoculars to understend OSD. Do you? :-)

Current OSD implementation makes OSD unusable for such use cases. 

And, well, I don't mean "relative font size", I mean "absolute and customizable by end user ". Somebody has 19" display in 3 meters distance, somebody other has the same dislay in 5 meters distance. You see, it is impossible to have the only way for all use cases.

Please, reopen the issue.

BTW, I see another way which can be more preferred by developers (and is acceptable from my POV) - just add a checkbox option "Full Screen Size" in OSD customixation dialog. Hey, is it interesting? :-)
Comment 11 Myriam Schweingruber 2010-09-23 15:49:29 UTC
Reopening based on comment #10. 
Andrew, which exact Amarok version are you using?
Comment 12 Andrew Gaydenko 2010-09-23 15:57:43 UTC
Myriam, I use Amarok 2.3.2 from official Kubuntu Maverick testing (with backports enabled).
Comment 13 Soren Harward 2010-09-24 14:11:46 UTC
(In reply to comment #10)
> Currently at 2-3 meters from display I need binoculars to understend
OSD. Do you? :-)

No, I don't.  And I'm running Amarok on an HTPC with 720P resolution.  The OSD displays just fine because I configured the "Normal" system font to 20pt.  So if the Amarok OSD is completely unreadable for your HTPC use case, that means that your system fonts are not set up correctly.  Having a huge normal font size is just a fact of life with an HTPC, and it's fair to have the onus of proper system configuration be on the user, rather than expecting developers to offer workarounds for improperly configured systems.

I agree that the font size should probably be proportional to, rather than a fixed increase from, the "default" font size, but having an option to set the font size of the OSD (and only the OSD) is an unnecessary kludge.  Amarok, like all other KDE apps, should just use the font sizes that the user has specified, rather than trying to specify its own.
Comment 14 Andrew Gaydenko 2010-09-24 14:54:10 UTC
Soren, you have described another use case. I'm not interested in video at all, and don't need HTPC, and have 19" display. But I love music, have brilliant music hardware, and (besides job) use PC as a audio source with LIRCing. This is a direct and main purpose of audio player - to play a music, isn't it? And Amarok is audio player, I suppose :-)

I have configured Amarok to react on LIRCing only (i.e. OSD is off in configuration dialog). So OSD rises when I push defined button on RC only.
Comment 15 Soren Harward 2010-10-12 01:00:37 UTC
commit d9d27ca96e4a1e8c06aea03c8c4cddcbbd02daf7
Author: Soren Harward <stharward@gmail.com>
Date:   Mon Oct 11 18:57:50 2010 -0400

    Font size of OSD is now configurable
    
    For some reason, the OSD preview in the configuration dialog ignores the
    font scaling parameter.  But it works fine when the real OSD is
    displayed.
    
    BUG: 195186

diff --git a/ChangeLog b/ChangeLog
index 0529ba7..68f7c32 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@ Amarok ChangeLog
 
 VERSION 2.4
   FEATURES:
+    * Size of OSD font is now configurable. (BR 195186)
     * Musicbrainz-based mass tagging UI by Sergey Ivanov.
     * New easy to use table-based UI for Ampache server configuration. (BR 200703)
     * OPML export for podcast subscriptions. (BR 126120)
diff --git a/src/amarokconfig.kcfg b/src/amarokconfig.kcfg
index 4ef752e..46625e7 100644
--- a/src/amarokconfig.kcfg
+++ b/src/amarokconfig.kcfg
@@ -363,10 +363,17 @@
         <default>false</default>
     </entry>
     <entry key="Osd Text Color" type="Color">
-        <label>Font Color for On-Screen Display</label>
+        <label>Font color for OSD</label>
         <whatsthis>The color of the OSD text. The color is specified in RGB, a comma-separated list containing three integers between 0 and 255.</whatsthis>
         <default>#ffff00</default>
     </entry>
+    <entry key="Osd Font Scaling" type="Int">
+        <label>Font scaling for OSD</label>
+        <whatsthis>The scaling multiplier for the OSD font, in percent-of-normal. 100 is &ldquo;normal size&rdquo;.</whatsthis>
+        <default>115</default>
+	<min>0</min>
+	<max>1000</max>
+    </entry>
     <entry key="Osd Duration" type="Int">
         <label>How many milliseconds the text should be displayed for</label>
         <whatsthis>The time in milliseconds to show the OSD. A value of 0 means never hide. The default value is 5000 ms.</whatsthis>
diff --git a/src/configdialog/dialogs/NotificationsConfig.cpp b/src/configdialog/dialogs/NotificationsConfig.cpp
index 662a45c..8648c5c 100644
--- a/src/configdialog/dialogs/NotificationsConfig.cpp
+++ b/src/configdialog/dialogs/NotificationsConfig.cpp
@@ -60,6 +60,8 @@ NotificationsConfig::NotificationsConfig( QWidget* parent )
              m_osdPreview,            SLOT( setVisible( bool ) ) );
     connect( kcfg_OsdUseTranslucency, SIGNAL( toggled( bool ) ),
              m_osdPreview,            SLOT( setTranslucent( bool ) ) );
+    connect( kcfg_OsdFontScaling,     SIGNAL( valueChanged( int ) ),
+             m_osdPreview,            SLOT( setFontScale( int ) ) );
 
     /*
     Amarok::QStringx text = i18n(
@@ -118,6 +120,7 @@ NotificationsConfig::updateSettings()
     AmarokConfig::setOsdAlignment( m_osdPreview->alignment() );
     AmarokConfig::setOsdYOffset( m_osdPreview->y() );
     AmarokConfig::setOsdUseTranslucency( kcfg_OsdUseTranslucency->isChecked() );
+    //AmarokConfig::setOsdTextScaling( m_osdPreview->); //FIXME?
 
     Amarok::OSD::instance()->setEnabled( kcfg_OsdEnabled->isChecked() );
 
diff --git a/src/configdialog/dialogs/NotificationsConfig.ui b/src/configdialog/dialogs/NotificationsConfig.ui
index b7e6cbe..5afef8c 100644
--- a/src/configdialog/dialogs/NotificationsConfig.ui
+++ b/src/configdialog/dialogs/NotificationsConfig.ui
@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>229</width>
-    <height>312</height>
+    <width>598</width>
+    <height>426</height>
    </rect>
   </property>
   <layout class="QGridLayout" name="rootLayout">
@@ -118,7 +118,7 @@
            </item>
           </layout>
          </item>
-         <item row="3" column="0">
+         <item row="5" column="0">
           <widget class="QGroupBox" name="kcfg_OsdUseCustomColors">
            <property name="enabled">
             <bool>true</bool>
@@ -193,7 +193,7 @@
               <property name="text">
                <string/>
               </property>
-              <property name="color" stdset="0">
+              <property name="color">
                <color>
                 <red>255</red>
                 <green>0</green>
@@ -215,6 +215,46 @@
            </property>
           </widget>
          </item>
+         <item row="4" column="0">
+          <layout class="QHBoxLayout" name="horizontalLayout_2">
+           <property name="topMargin">
+            <number>0</number>
+           </property>
+           <item>
+            <widget class="QLabel" name="label">
+             <property name="text">
+              <string>Scale Font:</string>
+             </property>
+            </widget>
+           </item>
+           <item>
+            <widget class="QSpinBox" name="kcfg_OsdFontScaling">
+             <property name="suffix">
+              <string>%</string>
+             </property>
+             <property name="maximum">
+              <number>1000</number>
+             </property>
+             <property name="value">
+              <number>115</number>
+             </property>
+            </widget>
+           </item>
+           <item>
+            <spacer name="horizontalSpacer">
+             <property name="orientation">
+              <enum>Qt::Horizontal</enum>
+             </property>
+             <property name="sizeHint" stdset="0">
+              <size>
+               <width>40</width>
+               <height>20</height>
+              </size>
+             </property>
+            </spacer>
+           </item>
+          </layout>
+         </item>
         </layout>
        </widget>
       </item>
diff --git a/src/widgets/Osd.cpp b/src/widgets/Osd.cpp
index ea43d4e..29ee886 100644
--- a/src/widgets/Osd.cpp
+++ b/src/widgets/Osd.cpp
@@ -62,6 +62,7 @@ OSDWidget::OSDWidget( QWidget *parent, const char *name )
         , m_rating( 0 )
         , m_volume( 0 )
         , m_showVolume( false )
+        , m_fontScale( 1.15 )
 {
     Qt::WindowFlags flags;
     flags = Qt::WindowStaysOnTopHint | Qt::FramelessWindowHint;
@@ -86,11 +87,6 @@ OSDWidget::OSDWidget( QWidget *parent, const char *name )
     m_timer->setSingleShot( true );
     connect( m_timer, SIGNAL( timeout() ), SLOT( hide() ) );
 
-    QFont f = font();
-    f.setFamily( "Arial" );
-    f.setPointSize( f.pointSize() + 8 );
-    setFont( f );
-
     //or crashes, KWindowSystem bug I think, crashes in QWidget::icon()
     kapp->setTopWidget( this );
 
@@ -117,6 +113,10 @@ OSDWidget::show( const QString &text, QImage newImage )
     else
         m_cover = Amarok::icon();
 
+    QFont f = QFont( "sans-serif" );
+    f.setPointSizeF( f.pointSizeF() * m_fontScale );
+    setFont( f );
+
     m_text = text;
     show();
 }
@@ -622,6 +622,7 @@ Amarok::OSD::applySettings()
     setEnabled( AmarokConfig::osdEnabled() );
     setOffset( AmarokConfig::osdYOffset() );
     setScreen( AmarokConfig::osdScreen() );
+    setFontScale( AmarokConfig::osdFontScaling() );
 
     if( AmarokConfig::osdUseCustomColors() )
         setTextColor( AmarokConfig::osdTextColor() );
diff --git a/src/widgets/Osd.h b/src/widgets/Osd.h
index 7d8194f..b80b37e 100644
--- a/src/widgets/Osd.h
+++ b/src/widgets/Osd.h
@@ -78,6 +78,7 @@ class OSDWidget : public QWidget
         void setText( const QString &text ) { m_text = text; }
         void setRating( const short rating ) { if ( isEnabled() ) m_rating = rating; }
         void setTranslucent( bool enabled ) { setWindowOpacity( enabled ? OSD_WINDOW_OPACITY : 1.0 ); }
+        void setFontScale( int scale ) { m_fontScale = static_cast<double>(scale) / 100.0; }
 
     protected:
         virtual ~OSDWidget();
@@ -109,6 +110,7 @@ class OSDWidget : public QWidget
         QImage      m_cover;
         QPixmap     m_scaledCover;
         bool        m_paused;
+        double      m_fontScale;
 };
 
 
@@ -125,8 +127,9 @@ public:
 
 public slots:
     void setTextColor( const QColor &color ) { OSDWidget::setTextColor( color ); doUpdate(); }
-    void setFont( const QFont &font ) { OSDWidget::setFont( font ); doUpdate(); }
+    //void setFont( const QFont &font ) { OSDWidget::setFont( font ); doUpdate(); }
     void setScreen( int screen ) { OSDWidget::setScreen( screen ); doUpdate(); }
+    void setFontScale( int scale ) { OSDWidget::setFontScale( scale ); doUpdate(); }
     void setUseCustomColors( const bool use, const QColor &fg )
     {
         if( use ) OSDWidget::setTextColor( fg );
Comment 16 Andrew Gaydenko 2010-10-12 01:13:08 UTC
Soren, thanks!! Hope the fix will be included into the next A2 release.
Comment 17 Myriam Schweingruber 2010-10-15 13:00:31 UTC
*** Bug 254029 has been marked as a duplicate of this bug. ***
Comment 18 Kevin Funk 2011-01-15 10:31:46 UTC
commit ba66192ba096bce6903abde422b7c919a0bd0edc
branch master
Author: Kevin Funk <krf@electrostorm.net>
Date:   Sat Jan 15 02:25:15 2011 +0100

    Fix OSD preview widget wrt scaling, clean up
    
    This is a follow-up on d9d27ca96e4a1e8c06aea03c8c4cddcbbd02daf7.
    
    CCBUG: 195186
    BUG: 254029

diff --git a/ChangeLog b/ChangeLog
index c7ab729..1d9a0c7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,11 +9,16 @@ VERSION 2.4.1
   CHANGES:
 
   BUGFIXES:
-    * Fixed issue with playlist tooltips that was shown independetly from "Show tooltip" option. (BR 263121)
-    * Fixed issues with multifiles cuesheet, when all tracks get metadata of last track
+    * Fixed 'Scale Font' option in OSD options for OSD preview widget.
+      (BR 254029)
+    * Fixed issue with playlist tooltips that was shown independetly from
+      "Show tooltip" option. (BR 263121)
+    * Fixed issues with multifiles cuesheet, when all tracks get metadata of
+      last track
       in cuesheet, and each file defined in sheet gets all tracks of this sheet.
       (BR 262668) (BR 209341)
-    * Fixed crash when trying to download a full size cover and the server redirects the request. (BR 262902)
+    * Fixed crash when trying to download a full size cover and the server
+      redirects the request. (BR 262902)
     * Fixed issue when breadcrumbs stayed not updated after service insert/remove.
       (BR 262780)
     * Fixed issue with TagDialog that make metadata fields stay editable if multiple
diff --git a/src/widgets/Osd.cpp b/src/widgets/Osd.cpp
index 05c16da..b843f5d 100644
--- a/src/widgets/Osd.cpp
+++ b/src/widgets/Osd.cpp
@@ -63,7 +63,6 @@ OSDWidget::OSDWidget( QWidget *parent, const char *name )
         , m_rating( 0 )
         , m_volume( 0 )
         , m_showVolume( false )
-        , m_fontScale( 1.15 )
 {
     Qt::WindowFlags flags;
     flags = Qt::WindowStaysOnTopHint | Qt::FramelessWindowHint;
@@ -81,6 +80,9 @@ OSDWidget::OSDWidget( QWidget *parent, const char *name )
     setFocusPolicy( Qt::NoFocus );
     unsetColors();
 
+    QFont font("sans-serif");
+    setFont(font);
+
     #ifdef Q_WS_X11
     KWindowSystem::setType( winId(), NET::Notification );
     #endif
@@ -114,10 +116,6 @@ OSDWidget::show( const QString &text, QImage newImage )
     else
         m_cover = Amarok::icon();
 
-    QFont f = QFont( "sans-serif" );
-    f.setPointSizeF( f.pointSizeF() * m_fontScale );
-    setFont( f );
-
     m_text = text;
     show();
 }
@@ -299,8 +297,8 @@ OSDWidget::determineMetrics( const int M )
 void
 OSDWidget::paintEvent( QPaintEvent *e )
 {
-    int M = m_m;
-    QSize size = m_size;
+    const int& M = m_m;
+    const QSize& size = m_size;
 
     QPoint point;
     QRect rect( point, size );
@@ -381,6 +379,7 @@ OSDWidget::paintEvent( QPaintEvent *e )
 
         p.drawImage( rect.topLeft() - QPoint( 5, 5 ), ShadowEngine::makeShadow( pixmap, shadowColor ) );
     }
+    p.setFont( font() );
     p.setPen( palette().color( QPalette::Active, QPalette::WindowText ) );
     //p.setPen( Qt::white ); // This too.
     p.drawText( rect, align, m_text );
diff --git a/src/widgets/Osd.h b/src/widgets/Osd.h
index c261314..bda01df 100644
--- a/src/widgets/Osd.h
+++ b/src/widgets/Osd.h
@@ -21,17 +21,12 @@
 #include "core/meta/Meta.h"
 
 #include <QImage>
-#include <QList>
 #include <QPixmap>
 #include <QString>
 #include <QWidget> //baseclass
 
 #define OSD_WINDOW_OPACITY 0.74
 
-namespace Plasma
-{
-    class PanelSvg;
-}
 
 class OSDWidget : public QWidget
 {
@@ -77,7 +72,17 @@ class OSDWidget : public QWidget
         void setText( const QString &text ) { m_text = text; }
         void setRating( const short rating ) { if ( isEnabled() ) m_rating = rating; }
         void setTranslucent( bool enabled ) { setWindowOpacity( enabled ? OSD_WINDOW_OPACITY : 1.0 ); }
-        void setFontScale( int scale ) { m_fontScale = static_cast<double>(scale) / 100.0; }
+        void setFontScale( int scale ) {
+            double fontScale = static_cast<double>( scale ) / 100.0;
+
+            // update font, reuse old one
+            QFont newFont( font() );
+            newFont.setPointSizeF( defaultPointSize() * fontScale );
+            setFont( newFont );
+        }
+
+        // work-around to get default point size on this platform, Qt API does not offer this directly
+        static inline double defaultPointSize() { return QFont().pointSizeF(); }
 
     protected:
         virtual ~OSDWidget();
@@ -109,7 +114,6 @@ class OSDWidget : public QWidget
         QImage      m_cover;
         QPixmap     m_scaledCover;
         bool        m_paused;
-        double      m_fontScale;
 };
 
 
@@ -120,9 +124,9 @@ class OSDPreviewWidget : public OSDWidget
 public:
     OSDPreviewWidget( QWidget *parent );
 
-    int screen()    { return m_screen; }
-    int alignment() { return m_alignment; }
-    int y()         { return m_y; }
+    int screen() const    { return m_screen; }
+    int alignment() const { return m_alignment; }
+    int y() const         { return m_y; }
 
 public slots:
     void setTextColor( const QColor &color ) { OSDWidget::setTextColor( color ); doUpdate(); }