[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [PATCH] Fixing the KMix applet
From:       Christian Esken <c.esken () cityweb ! de>
Date:       2003-11-23 19:44:18
[Download RAW message or body]

On Sunday 23 November 2003 18:20, Andras Mantia wrote:
> On Sunday 23 November 2003 18:39, Christian Esken wrote:
> > >3. After restarting KDE or kicker the mixer applet was very wide, there
> > > was
> >
> > Nice. These fix ugly bugs.
> >
> > >-       updateSize(true);
> > >+       updateSize(false);
> >
> > Why do you need to change this?
>
> To fix the above problem.
>
> > And are you aware that kmixerwidget.cpp is not only part of the applet,
> > but of the main application itself? This change could screw up resizing
> > the main window (especially after enabling/disabling the "Advanced"
> > checkbox).
>
> No... Well, changing true to false solves the applet is too wide problem.
> It may be not the best solution.

It's the best solution. I have not correctly looked at the sources. The code 
above is *explicitely* not executed on the main window.

>
> > > -      emit updateLayout();
> > > +      QTimer::singleShot(0, m_mixerwidget, SLOT(updateSize()));
> >
> > I think this might OK, but why is it neccesary? And wouldn't it then be
> > neccesary for MixDeviceWidget::setStereoLinked() too?
>
> I don't know. I just looked at the functions involved in channel
> hiding/showing. If the updateSize() call is not delayed, the hide()/show()
> in the MixDeviceWidget::setDisabled() may not be executed when the
> KMixerWidget::updateSize() is called, and the layout() width will be still
> wrong. This causes the bug that the last channel is not visible even after
> you enable it, only if you hide another channel. Restoring the other
> channel hides the last one again, as the minimum width is too small. Enable
> the debug line in KMixerWidget::updateSize() and you will see what does the
> layout()->minimumSize().width() contain after hiding/showing channels.
>
> Now I looked at the setStereoLinked() and yes, it seems that it needs to be
> done also there. Or use the same trick as with
> KKMixApplet::triggerUpdateLayout / KMixApplet::updateLayoutNow.

Acknowledged. The patch is good. I have applied the patch now (except the 
strange "-void , +void" patch lines).

Only one part of the patch is not good: The channel hiding/showing. It would 
not work if there are channels with the same name. The real  patch is much 
more complicated. Please see it attach and comment.

Helio around? An OK from you to commit?

Chris


["kmix-hideShowMixer.diff" (text/x-diff)]

Index: kmixerwidget.cpp
===================================================================
RCS file: /home/kde/kdemultimedia/kmix/kmixerwidget.cpp,v
retrieving revision 1.57
diff -u -u -r1.57 kmixerwidget.cpp
--- kmixerwidget.cpp	1 Nov 2003 15:25:51 -0000	1.57
+++ kmixerwidget.cpp	23 Nov 2003 19:38:26 -0000
@@ -250,7 +250,7 @@
 		m_balanceSlider = 0;
 	}
 
-	updateSize(true);
+	updateSize(false);
 	// we have to expliciteley set the size, as
 }
 
@@ -382,38 +382,41 @@
 {
 	QStringList output, input, sw;
 	QMap<QString, bool> state;
-   int n=0;
+	QMap<QString, int> id;
+	QMap<QString, QString> name;
 
    m_toggleMixerChannels->popupMenu()->clear();
 
    for (Channel *chn=m_channels.first(); chn!=0; chn=m_channels.next())
    {
+		QString uniqueName;
+		uniqueName.append(chn->dev->mixDevice()->num()).append(chn->dev->name());
 		if( chn->dev->isSwitch() )
-			sw << chn->dev->name();
+			sw << uniqueName;
 		else if ( chn->dev->isRecsrc() )
-			input << chn->dev->name();
+			input << uniqueName;
 		else
-			output << chn->dev->name();
+			output << uniqueName;
 
-		state[ chn->dev->name() ] = !chn->dev->isDisabled();
+		state[ uniqueName ] = !chn->dev->isDisabled();
+		id[ uniqueName ] = chn->dev->mixDevice()->num();
+		name[ uniqueName ] = chn->dev->name();
    }
 
 	// Output
 	m_toggleMixerChannels->popupMenu()->insertTitle(  SmallIcon(  "kmix" ), i18n( "Output" ) );
 	for ( QStringList::Iterator it = output.begin(); it != output.end(); ++it )
 	{
-		m_toggleMixerChannels->popupMenu()->insertItem( *it, n );
-		m_toggleMixerChannels->popupMenu()->setItemChecked( n, state[ *it ] );
-		n++;
+		m_toggleMixerChannels->popupMenu()->insertItem( name[ *it ], id[ *it ] );
+		m_toggleMixerChannels->popupMenu()->setItemChecked( id[ *it ], state[ *it ] );
 	}
 
 	// Input
 	m_toggleMixerChannels->popupMenu()->insertTitle(  SmallIcon(  "kmix" ), i18n( "Input" ) );
 	for ( QStringList::Iterator it = input.begin(); it != input.end(); ++it )
 	{
-		m_toggleMixerChannels->popupMenu()->insertItem( *it, n );
-		m_toggleMixerChannels->popupMenu()->setItemChecked( n, state[ *it ] );
-		n++;
+		m_toggleMixerChannels->popupMenu()->insertItem( name[ *it ], id[ *it ] );
+		m_toggleMixerChannels->popupMenu()->setItemChecked( id[ *it ], state[ *it ] );
 	}
 
 	if( ! m_small )
@@ -422,9 +425,8 @@
 		m_toggleMixerChannels->popupMenu()->insertTitle(  SmallIcon(  "kmix" ), i18n( "Switches" ) );
 		for ( QStringList::Iterator it = sw.begin(); it != sw.end(); ++it )
 		{
-			m_toggleMixerChannels->popupMenu()->insertItem( *it, n );
-			m_toggleMixerChannels->popupMenu()->setItemChecked( n, state[ *it ] );
-			n++;
+			m_toggleMixerChannels->popupMenu()->insertItem( name[ *it ], id[ *it ] );
+			m_toggleMixerChannels->popupMenu()->setItemChecked( id[ *it ], state[ *it ] );
 		}
 	}
 }
@@ -438,7 +440,6 @@
    Channel *chn = m_channels.at(id);
    if(!chn)
       return;
-
    bool gotCheck = m_toggleMixerChannels->popupMenu()->isItemChecked(id);
 
    chn->dev->setDisabled(gotCheck);
Index: mixdevicewidget.cpp
===================================================================
RCS file: /home/kde/kdemultimedia/kmix/mixdevicewidget.cpp,v
retrieving revision 1.62
diff -u -u -r1.62 mixdevicewidget.cpp
--- mixdevicewidget.cpp	12 Oct 2003 15:50:22 -0000	1.62
+++ mixdevicewidget.cpp	23 Nov 2003 19:38:26 -0000
@@ -432,7 +432,8 @@
       value ? slider->hide() : slider->show();
 
    layout()->activate();
-   emit updateLayout();
+   QTimer::singleShot(0, m_mixerwidget, SLOT(updateSize()));
+   //emit updateLayout();
 }
 
 
Index: mixdevicewidget.h
===================================================================
RCS file: /home/kde/kdemultimedia/kmix/mixdevicewidget.h,v
retrieving revision 1.21
diff -u -u -r1.21 mixdevicewidget.h
--- mixdevicewidget.h	28 Sep 2003 17:26:27 -0000	1.21
+++ mixdevicewidget.h	23 Nov 2003 19:38:26 -0000
@@ -70,6 +70,7 @@
 		bool hasMute() const;
       bool isStereoLinked() const { return m_linked; };
       bool isLabeled() const;
+      MixDevice* mixDevice() { return m_mixdevice; };
 
       void setStereoLinked( bool value );
       void setLabeled( bool value );



[prev in list] [next in list] [prev in thread] [next in thread]