Bug 169333

Summary: display setting revert to prev configuration useless
Product: [Applications] systemsettings Reporter: sycao <yinshuiboy>
Component: kcm_randrAssignee: Gustavo Pichorim Boiko <gustavo.boiko>
Status: RESOLVED FIXED    
Severity: normal CC: jpetso, yinshuiboy
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Unlisted Binaries   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description sycao 2008-08-18 04:30:39 UTC
Version:            (using KDE 4.1.0)
OS:                Linux
Installed from:    Unspecified Linux

reproduce:
1. kcmshell4 display 
2. in size&Orientation tab, change size and refresh, click 'apply' button.
3. when a dialog popup, click 'revert to previous configuration'.

expected behaviour:
your change should be reverted to previous.
Comment 1 sycao 2008-08-18 04:40:26 UTC
the problem doesn't show up when using krandrtray, because all changes will trigger 
RandROutput->applyProposed with specific changed argument. but when use kcmshell4 display, 
all changes will trigger m_display->applyProposed with inaccurate changed argument. 

diff -Nur kdebase-workspace-4.1.0-orig/kcontrol/randr/krandrmodule.cpp kdebase-workspace-4.1.0/kcontrol/randr/krandrmodule.cpp
--- kdebase-workspace-4.1.0-orig/kcontrol/randr/krandrmodule.cpp	2008-08-12 15:49:50.000000000 +0000
+++ kdebase-workspace-4.1.0/kcontrol/randr/krandrmodule.cpp	2008-08-14 11:52:31.000000000 +0000
@@ -39,6 +39,11 @@
     : KCModule(KSSFactory::componentData(), parent)
 {
 	m_display = new RandRDisplay();
+     	if ( m_display->needsRefresh() ) 
+	{                                      
+               	m_display->refresh();                                           
+      	}  
+
 	if (!m_display->isValid())
 	{
 		QVBoxLayout *topLayout = new QVBoxLayout(this);
diff -Nur kdebase-workspace-4.1.0-orig/kcontrol/randr/randrconfig.cpp kdebase-workspace-4.1.0/kcontrol/randr/randrconfig.cpp
--- kdebase-workspace-4.1.0-orig/kcontrol/randr/randrconfig.cpp	2008-08-12 15:49:50.000000000 +0000
+++ kdebase-workspace-4.1.0/kcontrol/randr/randrconfig.cpp	2008-08-14 11:34:12.000000000 +0000
@@ -152,12 +152,22 @@
 			output->proposeRect(configuredRect);
 			output->proposeRotation(config->rotation());
 			output->proposeRefreshRate(config->refreshRate());
+
+			int change = 0;
+			if ( output->rect() != configuredRect )
+				change |= RandR::ChangeRect;
+			if ( output->rotation() != config->rotation() )
+				change |= RandR::ChangeRotation;
+			if ( output->refreshRate() != config->refreshRate() )
+				change |= RandR::ChangeRate;
+			output->applyProposed( RandR::ChangeRect, true );
+
 		} else { // user wants to disable this output
 			kDebug() << "Disabling " << output->name();
 			output->slotDisable();
 		}
 	}
-	m_display->applyProposed();
+//	m_display->applyProposed();
 	update();
 }
Comment 2 Jakob Petsovits 2008-11-13 17:10:00 UTC
SVN commit 883800 by jpetso:

* Fix reverting to the previous configuration.
* Ignore even more irrelevant applyProposed() attempts.
* Remove an unused variable (QRect r).
* Some more debug output tweaking.

BUG: 169333


 M  +1 -1      outputconfig.cpp  
 M  +8 -9      randroutput.cpp  
 M  +5 -2      randrscreen.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=883800
Comment 3 Jakob Petsovits 2008-11-13 17:14:21 UTC
m_display->applyProposed() should also result in correct ask/revert cycles, therefore ignoring it in favor of applyProposed() for each output separately is not sufficient. Also, if you do this separately for each output then you'll get a dialog box for each output, which is not intended (and probably the reason why it's all done at once in the display).

I fixed it properly - the outputs didn't propose their original values on output->proposeOriginal() so nothing changed there. By restoring the original values there, it works as it should, also inside of display->applyProposed().