Bug 112332

Summary: MiniBar QPushButton repaints (recursively?) if background mode is set to NoBackground
Product: [Applications] kpdf Reporter: Thomas Lübking <thomas.luebking>
Component: generalAssignee: Albert Astals Cid <aacid>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: What i see

Description Thomas Lübking 2005-09-09 22:37:47 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc 3.4.1 
OS:                Linux

if (e.g. by a style) the QPushButton background mode is set to NoBackground, kpdf will instantly repaint the Button in the minibar when loading a document, hang and be unusable. (smells like a live lock?)
Comment 1 Albert Astals Cid 2005-09-09 22:52:38 UTC
You say Version: (using KDE Devel) does that mean trunk or 3.5 branch ?
Also can you tell a style that has that behaviour?
Comment 2 Thomas Lübking 2005-09-10 10:24:11 UTC
3.5 (sorry, didn't see different entries in that combo, if there are any)

i triggered that behaviour with baghira (cvs) and hacked around it by excluding MiniBar parented buttons from beeing NoBackground (polish.cpp, #418)

i also tried to eat events around that (in case you just live lock on palette changes) but that didn't help while setting the background mode to e.g. PaletteBackground changes the color back to button - looks ugly, but doesn't cause problems.

any style could do that to increase performance and reduce flicker if it expects to paint a full qpushbutton (you can however prevent this by  setting the background mode after constructing and therefore style polished the button, if there's no other way to fix that, but 1. "paletteBackgroundColor" is still available after setting the backgroundmode to NoBackground and 2. the live lock may still be a bug ;)

if the backgroundmode is set to NoBackground, the widget's just not erased before paint, i.e. if you don't paint the full widgets rect yourself, you're getting artefacts
Comment 3 Albert Astals Cid 2005-09-10 14:21:33 UTC
Created attachment 12518 [details]
What i see

I don't see any artifact. Maybe it's because i'm using already fixed cvs
version of baghira? Anyway do you have any suggestion on how to fix it?
Comment 4 Thomas Lübking 2005-09-10 15:32:12 UTC
it's not supposed to have any artefacts ;)
i just played teacher and schwadronierte about what NoBackground does.
as the button is fully painted anyway (unhovered by kpdf, hovered by baghira) ther won't occur any problems from missing erase.

to see the problem, comment the 
if (!(w->parent() && w->parent()->inherits("MiniBar")))
line in polish.cpp, then start kpdf and open a document. kpdf will hang and infinitly repaint the Button (it will still load the page - slowly, the GUI runs in its own thread)
Comment 5 Albert Astals Cid 2005-09-10 15:53:52 UTC
Really i do not understand the problem, i mean, if it works with all styles and not with yours, isn't it a bug on your side?
Comment 6 Thomas Lübking 2005-09-10 18:27:19 UTC
no. every style could decide to do the same thing (maybe some do, don't know)
NoBackground is a valid background mode and as usually styles paint the pushbutton, they could come to the idea to use it.
so you should at least ensure that the background mode for your button is usefull (i.e. if you need the PaletteBackground mode, set it after constructing)

also, if NoBackground as background mode causes infinite repaints, there's some unwanted behaviour in the code, i.e. a bug.

it's not a problem for me, as i know about this issue and worked around by excluding this specific button, but other styles may run into the same trap.

i guess the problem is about swapping around in the paint event, you could rely on QEvent::Enter and QEvent::Leave instead of QEvent::MouseMove and QEvent::Paint to swap palette colors
Comment 7 Thomas Lübking 2005-09-10 19:14:14 UTC
ok, got what you wanna do. this implemetation will do the job and don't fear the void background (additionally it won't mess up if the background mode of the button differs from the bg mode of the MiniBar):

// [private widget] a flat qpushbutton that enlights on hover
class HoverButton : public QPushButton
{
    public:
        HoverButton( QWidget * parent );

    protected:
        void paintEvent( QPaintEvent * e );
};

/** HoverButton **/

HoverButton::HoverButton( QWidget * parent )
    : QPushButton( parent )
{
    setMouseTracking( true );
#if KDE_IS_VERSION(3,3,90)
    KAcceleratorManager::setNoAccel( this );
#endif
}

void HoverButton::paintEvent( QPaintEvent * e )
{
   if (hasMouse())
   {
      QPushButton::paintEvent( e );
   }
   else
   {
      QPainter p( this );
      p.fillRect(e->rect(), parentWidget() ? parentWidget()->palette().brush(QPalette::Active, QColorGroup::Background) : paletteBackgroundColor());
      drawButtonLabel( &p );
   }
}
Comment 8 Albert Astals Cid 2005-09-10 20:08:57 UTC
SVN commit 459394 by aacid:

Avoid recursive painting of the minibar buttons. Thanks Thomas for the patch.
BUGS: 112332


 M  +3 -40     minibar.cpp  


--- branches/KDE/3.5/kdegraphics/kpdf/ui/minibar.cpp #459393:459394
@@ -68,12 +68,7 @@
         HoverButton( QWidget * parent );
 
     protected:
-        void mouseMoveEvent( QMouseEvent * e );
-        void mouseReleaseEvent( QMouseEvent * e );
         void paintEvent( QPaintEvent * e );
-
-    private:
-        bool m_hovering;
 };
 
 
@@ -394,7 +389,7 @@
 /** HoverButton **/
 
 HoverButton::HoverButton( QWidget * parent )
-    : QPushButton( parent ), m_hovering( false )
+    : QPushButton( parent )
 {
     setMouseTracking( true );
 #if KDE_IS_VERSION(3,3,90)
@@ -402,48 +397,16 @@
 #endif
 }
 
-void HoverButton::mouseMoveEvent( QMouseEvent * e )
-{
-    // check for mouse hovering
-    const QRect myGeom( 0,0, width(), height() );
-    bool hover = myGeom.contains( e->pos() );
-
-    // if hover state changed update gfx
-    if ( m_hovering != hover )
-    {
-        m_hovering = hover;
-        update();
-    }
-}
-
-void HoverButton::mouseReleaseEvent( QMouseEvent * e )
-{
-    // call default handler
-    QPushButton::mouseReleaseEvent( e );
-
-    // reset hover state when clicking
-    m_hovering = false;
-    update();
-}
-
 void HoverButton::paintEvent( QPaintEvent * e )
 {
-    // always not hovering in disabled state
-    if ( !isEnabled() )
-        m_hovering = false;
-
-    // paint button in different flavours
-    if ( m_hovering )
+    if ( hasMouse() )
     {
-        // if we're hovering the button, draw it using QPushButton style
-        setPaletteBackgroundColor( palette().active().button() );
         QPushButton::paintEvent( e );
     }
     else
     {
-        // custom drawing of unhovered button
         QPainter p( this );
-        setPaletteBackgroundColor( palette().active().background() );
+        p.fillRect(e->rect(), parentWidget() ? parentWidget()->palette().brush(QPalette::Active, QColorGroup::Background) : paletteBackgroundColor());
         drawButtonLabel( &p );
     }
 }