Bug 169333 - display setting revert to prev configuration useless
Summary: display setting revert to prev configuration useless
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: kcm_randr (show other bugs)
Version: unspecified
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Gustavo Pichorim Boiko
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-18 04:30 UTC by sycao
Modified: 2008-11-13 17:14 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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().