Made the Screen preflet's behavior consistent with the other preflets.

* Moved Apply button into controls box (resolution, etc.), so its meaning is more obvious.
* Revert now resets the whole screen settings for all workspaces to the configuration at the start of the preflet.
* In confirmation window, renamed Revert to Undo because that's what it actually does (it has nothing to do with the Revert in the main dialog).
* Disabled Defaults button (for now) since it doesn't use the screen's preferred settings and added a few TODOs.
* This should fix bug #321.
Note: Unfortunately, it won't work correctly on R5 because BScreen can only access the active workspace, it seems (locally, I used activate_workspace() for testing the preflet). Also, Haiku doesn't yet support setting screen preferences for each workspace individually, so I couldn't test it.
I hope my code isn't too terrible. :)


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@20096 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/preferences/screen/AlertView.cpp

@@ -51,8 +51,8 @@ AlertView::AlertView(BRect frame, char *name)
 	keepButton->ResizeToPreferred();
 	AddChild(keepButton);
 
-	BButton* button = new BButton(rect, "revert", "Revert",
+	BButton* button = new BButton(rect, "undo", "Undo",
-		new BMessage(BUTTON_REVERT_MSG), B_FOLLOW_RIGHT | B_FOLLOW_BOTTOM);
+		new BMessage(SET_INITIAL_MODE_MSG), B_FOLLOW_RIGHT | B_FOLLOW_BOTTOM);
 	button->ResizeToPreferred();
 	AddChild(button);
 
@@ -107,7 +107,7 @@ void
 AlertView::Pulse()
 {
 	if (--fSeconds == 0)
-		Window()->PostMessage(BUTTON_REVERT_MSG);
+		Window()->PostMessage(SET_INITIAL_MODE_MSG);
 	else
 		UpdateCountdownView();
 }
@@ -117,7 +117,7 @@ void
 AlertView::KeyDown(const char* bytes, int32 numBytes)
 {
 	if (numBytes == 1 && bytes[0] == B_ESCAPE)
-		Window()->PostMessage(BUTTON_REVERT_MSG);
+		Window()->PostMessage(SET_INITIAL_MODE_MSG);
 }
 
 

src/preferences/screen/AlertWindow.cpp

@@ -17,7 +17,7 @@
 
 
 AlertWindow::AlertWindow(BMessenger target)
-	: BWindow(BRect(100.0, 100.0, 400.0, 193.0), "Revert",
+	: BWindow(BRect(100.0, 100.0, 400.0, 193.0), "Undo",
 		B_MODAL_WINDOW_LOOK, B_MODAL_APP_WINDOW_FEEL,
 		B_NOT_RESIZABLE | B_NOT_ZOOMABLE, B_ALL_WORKSPACES),
 	fTarget(target)
@@ -43,7 +43,7 @@ AlertWindow::MessageReceived(BMessage *message)
 			PostMessage(B_QUIT_REQUESTED);
 			break;
 
-		case BUTTON_REVERT_MSG:
+		case SET_INITIAL_MODE_MSG:
 			fTarget.SendMessage(SET_INITIAL_MODE_MSG);
 			PostMessage(B_QUIT_REQUESTED);
 			break;

src/preferences/screen/MonitorView.cpp

@@ -18,9 +18,10 @@
 
 
 #ifndef __HAIKU__
-inline bool operator!=(const rgb_color& x, const rgb_color& y)
+inline bool
+operator!=(const rgb_color& x, const rgb_color& y)
 {
-	return (x.red!=y.red || x.blue!=y.blue || x.green!=y.green);
+	return x.red != y.red || x.blue != y.blue || x.green != y.green;
 }
 #endif
 

src/preferences/screen/ScreenMode.cpp

@@ -9,6 +9,8 @@
 
 #include "ScreenMode.h"
 
+#include <InterfaceDefs.h>
+
 #include <stdlib.h>
 #include <stdio.h>
 
@@ -152,7 +154,7 @@ screen_mode::SetTo(display_mode& mode)
 ScreenMode::ScreenMode(BWindow* window)
 	:
 	fWindow(window),
-	fUpdatedMode(false)
+	fUpdatedModes(false)
 {
 	BScreen screen(window);
 	if (screen.GetModeList(&fModeList, &fModeCount) == B_OK) {
@@ -173,12 +175,17 @@ ScreenMode::~ScreenMode()
 
 
 status_t
-ScreenMode::Set(const screen_mode& mode)
+ScreenMode::Set(const screen_mode& mode, int32 workspace)
 {
-	if (!fUpdatedMode)
+	if (!fUpdatedModes)
-		UpdateOriginalMode();
+		UpdateOriginalModes();
 
 	BScreen screen(fWindow);
+	
+	if (workspace == ~0)
+		workspace = current_workspace();
+	
+	// TODO: our app_server doesn't fully support workspaces, yet
 	SetSwapDisplays(&screen, mode.swap_displays);
 	SetUseLaptopPanel(&screen, mode.use_laptop_panel);
 	SetTVStandard(&screen, mode.tv_standard);
@@ -187,20 +194,25 @@ ScreenMode::Set(const screen_mode& mode)
 	if (!GetDisplayMode(mode, displayMode))
 		return B_ENTRY_NOT_FOUND;
 
-	return screen.SetMode(&displayMode, true);
+	return screen.SetMode(workspace, &displayMode, true);
 }
 
 
 status_t
-ScreenMode::Get(screen_mode& mode)
+ScreenMode::Get(screen_mode& mode, int32 workspace) const
 {
 	display_mode displayMode;
 	BScreen screen(fWindow);
-	if (screen.GetMode(&displayMode) != B_OK)
+	
+	if (workspace == ~0)
+		workspace = current_workspace();
+	
+	if (screen.GetMode(workspace, &displayMode) != B_OK)
 		return B_ERROR;
 	
 	mode.SetTo(displayMode);
 	
+	// TODO: our app_server doesn't fully support workspaces, yet
 	if (GetSwapDisplays(&screen, &mode.swap_displays) != B_OK)
 		mode.swap_displays = false;
 	if (GetUseLaptopPanel(&screen, &mode.use_laptop_panel) != B_OK)
@@ -212,32 +224,60 @@ ScreenMode::Get(screen_mode& mode)
 }
 
 
+status_t
+ScreenMode::GetOriginalMode(screen_mode& mode, int32 workspace) const
+{
+	if (workspace == ~0)
+		workspace = current_workspace();
+	else if(workspace > 31)
+		return B_BAD_INDEX;
+	
+	mode = fOriginal[workspace];
+	
+	return B_OK;
+}
+
+
+// this method assumes that you already reverted to the correct number of workspaces
 status_t
 ScreenMode::Revert()
 {
-	if (!fUpdatedMode)
+	if (!fUpdatedModes)
-		return B_OK;
+		return B_ERROR;
 
+	status_t result = B_OK;
 	screen_mode current;
-	if (Get(current) == B_OK && fOriginal == current)
+	for (int32 workspace = 0; workspace < count_workspaces(); workspace++) {
-		return B_OK;
+		if (Get(current, workspace) == B_OK && fOriginal[workspace] == current)
+			continue;
 
 		BScreen screen(fWindow);
-	SetSwapDisplays(&screen, fOriginal.swap_displays);
-	SetUseLaptopPanel(&screen, fOriginal.use_laptop_panel);
-	SetTVStandard(&screen, fOriginal.tv_standard);
 
-	return screen.SetMode(&fOriginalDisplayMode, true);
+		// TODO: our app_server doesn't fully support workspaces, yet
+		if (workspace == current_workspace()) {
+			SetSwapDisplays(&screen, fOriginal[workspace].swap_displays);
+			SetUseLaptopPanel(&screen, fOriginal[workspace].use_laptop_panel);
+			SetTVStandard(&screen, fOriginal[workspace].tv_standard);
+		}
+
+		result = screen.SetMode(workspace, &fOriginalDisplayMode[workspace], true);
+		if (result != B_OK)
+			break;
+	}
+	
+	return result;
 }
 
 
 void
-ScreenMode::UpdateOriginalMode()
+ScreenMode::UpdateOriginalModes()
 {
 	BScreen screen(fWindow);
-	if (screen.GetMode(&fOriginalDisplayMode) == B_OK) {
+	for (int32 workspace = 0; workspace < count_workspaces(); workspace++) {
-		fUpdatedMode = true;
+		if (screen.GetMode(workspace, &fOriginalDisplayMode[workspace]) == B_OK) {
-		Get(fOriginal);
+			Get(fOriginal[workspace], workspace);
+			fUpdatedModes = true;
+		}
 	}
 }
 

src/preferences/screen/ScreenMode.h

@@ -41,11 +41,12 @@ class ScreenMode {
 		ScreenMode(BWindow* window);
 		~ScreenMode();
 
-		status_t Set(const screen_mode& mode);
+		status_t Set(const screen_mode& mode, int32 workspace = ~0);
-		status_t Get(screen_mode& mode);
+		status_t Get(screen_mode& mode, int32 workspace = ~0) const;
-		status_t Revert();
+		status_t GetOriginalMode(screen_mode &mode, int32 workspace = ~0) const;
 
-		void UpdateOriginalMode();
+		status_t Revert();
+		void UpdateOriginalModes();
 
 		bool SupportsColorSpace(const screen_mode& mode, color_space space);
 		status_t GetRefreshLimits(const screen_mode& mode, float& min, float& max);
@@ -60,9 +61,10 @@ class ScreenMode {
 		display_mode*	fModeList;
 		uint32			fModeCount;
 
-		bool			fUpdatedMode;
+		bool			fUpdatedModes;
-		display_mode	fOriginalDisplayMode;
+		// TODO: hard-coded to the current workspace limit
-		screen_mode		fOriginal;
+		display_mode	fOriginalDisplayMode[32];
+		screen_mode		fOriginal[32];
 };
 
 #endif	/* SCREEN_MODE_H */

src/preferences/screen/ScreenWindow.cpp

@@ -214,7 +214,8 @@ ScreenWindow::ScreenWindow(ScreenSettings *settings)
 	fIsVesa(false),
 	fVesaApplied(false),
 	fScreenMode(this),
-	fChangingAllWorkspaces(false)
+	fTempScreenMode(this),
+	fModified(false)
 {
 	BScreen screen(this);
 
@@ -222,7 +223,7 @@ ScreenWindow::ScreenWindow(ScreenSettings *settings)
 	if (screen.GetDeviceInfo(&info) == B_OK && !strcasecmp(info.chipset, "VESA"))
 		fIsVesa = true;
 
-	fScreenMode.Get(fOriginal);
+	UpdateOriginal();
 	fActive = fSelected = fOriginal;
 
 	BView *view = new BView(Bounds(), "ScreenView", B_FOLLOW_ALL, B_WILL_DRAW);
@@ -482,9 +483,10 @@ ScreenWindow::ScreenWindow(ScreenSettings *settings)
 	fBackgroundsButton->SetFontSize(be_plain_font->Size() * 0.9);
 	fScreenBox->AddChild(fBackgroundsButton);
 
-	fDefaultsButton = new BButton(buttonRect, "DefaultsButton", "Defaults",
+	// TODO: we don't support getting the screen's preferred settings
+	/* fDefaultsButton = new BButton(buttonRect, "DefaultsButton", "Defaults",
 		new BMessage(BUTTON_DEFAULTS_MSG));
-	view->AddChild(fDefaultsButton);
+	view->AddChild(fDefaultsButton); */
 
 	fRevertButton = new BButton(buttonRect, "RevertButton", "Revert",
 		new BMessage(BUTTON_REVERT_MSG));
@@ -494,7 +496,7 @@ ScreenWindow::ScreenWindow(ScreenSettings *settings)
 	fApplyButton = new BButton(buttonRect, "ApplyButton", "Apply", 
 		new BMessage(BUTTON_APPLY_MSG));
 	fApplyButton->SetEnabled(false);
-	view->AddChild(fApplyButton);
+	fControlsBox->AddChild(fApplyButton);
 
 	UpdateControls();
 
@@ -765,17 +767,7 @@ ScreenWindow::ScreenChanged(BRect frame, color_space mode)
 void
 ScreenWindow::WorkspaceActivated(int32 workspace, bool state)
 {
-	if (fChangingAllWorkspaces) {
+	fScreenMode.GetOriginalMode(fOriginal, workspace);
-		// we're currently changing all workspaces, so there is no need
-		// to update the interface
-		return;
-	}
-
-	fScreenMode.Get(fOriginal);
-	fScreenMode.UpdateOriginalMode();
-
-	// only override current settings if they have not been changed yet
-	if (fSelected == fActive)
 	UpdateActiveMode();
 	
 	BMessage message(UPDATE_DESKTOP_COLOR_MSG);
@@ -793,19 +785,19 @@ ScreenWindow::MessageReceived(BMessage* message)
 
 		case POP_WORKSPACE_CHANGED_MSG:
 		{
+			// update checkpoint state
 			int32 index;
-			if (message->FindInt32("index", &index) == B_OK)
+			if (message->FindInt32("index", &index) == B_OK) {
 				set_workspace_count(index + 1);
+				CheckApplyEnabled();
+			}
 			break;
 		}
 
 		case POP_RESOLUTION_MSG:
 		{
-			int32 oldWidth = fSelected.width, oldHeight = fSelected.height;
 			message->FindInt32("width", &fSelected.width);
 			message->FindInt32("height", &fSelected.height);
-			if (fSelected.width == oldWidth && fSelected.height == oldHeight)
-				break;
 
 			CheckColorMenu();
 			CheckRefreshMenu();
@@ -813,41 +805,28 @@ ScreenWindow::MessageReceived(BMessage* message)
 			UpdateMonitorView();
 			UpdateRefreshControl();
 
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 		}
 
 		case POP_COLORS_MSG:
 		{
-			color_space old = fSelected.space;
 			message->FindInt32("space", (int32 *)&fSelected.space);
-			if (fSelected.space == old)
-				break;
 
 			BString string;
 			string << fSelected.BitsPerPixel() << " Bits/Pixel";
 			fColorsMenu->Superitem()->SetLabel(string.String());
 
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 		}
 
 		case POP_REFRESH_MSG:
 		{
-			float old = fSelected.refresh;
 			message->FindFloat("refresh", &fSelected.refresh);
 			fOtherRefresh->SetLabel("Other" B_UTF8_ELLIPSIS);
 				// revert "Other…" label - it might have had a refresh rate prefix
 
-			if (fSelected.refresh == old)
-				break;
-
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 		}
@@ -874,13 +853,8 @@ ScreenWindow::MessageReceived(BMessage* message)
 		{
 			// user pressed "done" in "Other…" refresh dialog;
 			// select the refresh rate chosen
-			float old = fSelected.refresh;
 			message->FindFloat("refresh", &fSelected.refresh);
-			if (fSelected.refresh != old)
-				break;
 
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			UpdateRefreshControl();
 			CheckApplyEnabled();
 			break;
@@ -893,8 +867,6 @@ ScreenWindow::MessageReceived(BMessage* message)
 			if (message->FindInt32("mode", &mode) == B_OK)
 				fSelected.combine = (combine_mode)mode;
 
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckResolutionMenu();
 			CheckApplyEnabled();
 			break;
@@ -902,22 +874,16 @@ ScreenWindow::MessageReceived(BMessage* message)
 		
 		case POP_SWAP_DISPLAYS_MSG:
 			message->FindBool("swap", &fSelected.swap_displays);
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 
 		case POP_USE_LAPTOP_PANEL_MSG:
 			message->FindBool("use", &fSelected.use_laptop_panel);
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 
 		case POP_TV_STANDARD_MSG:
 			message->FindInt32("tv_standard", (int32 *)&fSelected.tv_standard);
-			fScreenMode.Get(fOriginal);
-			fScreenMode.UpdateOriginalMode();
 			CheckApplyEnabled();
 			break;
 
@@ -941,61 +907,45 @@ ScreenWindow::MessageReceived(BMessage* message)
 			fSelected.use_laptop_panel = false;
 			fSelected.tv_standard = 0;
 
-			fScreenMode.Get(fOriginal);
+			BMenuItem *item;
-			fScreenMode.UpdateOriginalMode();
+			item = fWorkspaceCountField->Menu()->ItemAt(3);
+			if (item != NULL)
+				item->SetMarked(true);
+
 			UpdateControls();
 			break;
 		}
 
-		case BUTTON_REVERT_MSG:
 		case SET_INITIAL_MODE_MSG:
+			fTempScreenMode.Revert();
+			UpdateActiveMode();
+			break;
+
+		case BUTTON_REVERT_MSG:
+		{
+			fModified = false;
+			BMenuItem *item;
+			item = fWorkspaceCountField->Menu()->ItemAt(fOriginalWorkspaceCount - 1);
+			if (item != NULL)
+				item->SetMarked(true);
+
+			// ScreenMode::Revert() assumes that we first set the correct number
+			// of workspaces
+			set_workspace_count(fOriginalWorkspaceCount);
 			fScreenMode.Revert();
 			UpdateActiveMode();
 			break;
+		}
 
 		case BUTTON_APPLY_MSG:
 			Apply();
 			break;
 
-		case MAKE_INITIAL_MSG: {
+		case MAKE_INITIAL_MSG:
-			// user pressed "keep" in confirmation box;
+			// user pressed "keep" in confirmation dialog
-			// select this mode in dialog and mark it as
+			fModified = true;
-			// previous mode; if "all workspaces" is selected, 
-			// distribute mode to all workspaces 
-
-			// use the mode that has eventually been set and
-			// thus we know to be working; it can differ from 
-			// the mode selected by user due to hardware limitation
-			display_mode newMode;
-			BScreen screen(this);
-			screen.GetMode(&newMode);
-
-			if (fAllWorkspacesItem->IsMarked()) {
-				int32 originatingWorkspace;
-
-				// the original panel activates each workspace in turn;
-				// this is disguisting and there is a SetMode
-				// variant that accepts a workspace id, so let's take
-				// this one
-				originatingWorkspace = current_workspace();
-
-				// well, this "cannot be reverted" message is not
-				// entirely true - at least, you can revert it
-				// for current workspace; to not overwrite original
-				// mode during workspace switch, we use this flag
-				fChangingAllWorkspaces = true;
-
-				for (int32 i = 0; i < count_workspaces(); i++) {
-					if (i != originatingWorkspace)
-						screen.SetMode(i, &newMode, true);
-				}
-
-				fChangingAllWorkspaces = false;
-			}
-
 			UpdateActiveMode();
 			break;
-		}
 		
 		default:
 			BWindow::MessageReceived(message);		
@@ -1045,8 +995,8 @@ ScreenWindow::CanApply() const
 bool
 ScreenWindow::CanRevert() const
 {
-	return (fActive != fOriginal && !fAllWorkspacesItem->IsMarked())
+	return fSelected != fActive || count_workspaces() != fOriginalWorkspaceCount
-			|| fSelected != fActive;
+		|| fModified;
 }
 
 
@@ -1058,6 +1008,15 @@ ScreenWindow::CheckApplyEnabled()
 }
 
 
+void
+ScreenWindow::UpdateOriginal()
+{
+	fOriginalWorkspaceCount = count_workspaces();
+	fScreenMode.Get(fOriginal);
+	fScreenMode.UpdateOriginalModes();
+}
+
+
 void
 ScreenWindow::Apply()
 {
@@ -1072,20 +1031,28 @@ ScreenWindow::Apply()
 		return;
 	}
 	
-	if (fAllWorkspacesItem->IsMarked()) {
+	// make checkpoint, so we can undo these changes
-		BAlert *workspacesAlert = new BAlert("WorkspacesAlert",
+	fTempScreenMode.UpdateOriginalModes();
-			"Change all workspaces? This action cannot be reverted.", "Okay", "Cancel", 
-			NULL, B_WIDTH_AS_USUAL, B_WARNING_ALERT);
-
-		if (workspacesAlert->Go() == 1)
-			return;
-	}
-
 	status_t status = fScreenMode.Set(fSelected);
 	if (status == B_OK) {
+		// use the mode that has eventually been set and
+		// thus we know to be working; it can differ from
+		// the mode selected by user due to hardware limitation
+		display_mode newMode;
+		BScreen screen(this);
+		screen.GetMode(&newMode);
+		
+		if (fAllWorkspacesItem->IsMarked()) {
+			int32 originatingWorkspace = current_workspace();
+			for (int32 i = 0; i < count_workspaces(); i++) {
+				if (i != originatingWorkspace)
+					screen.SetMode(i, &newMode, true);
+			}
+		}
+		
 		fActive = fSelected;
 		
-		// ToDo: only show alert when this is an unknown mode
+		// TODO: only show alert when this is an unknown mode
 		BWindow* window = new AlertWindow(this);
 		window->Show();
 	} else {
@@ -1162,18 +1129,22 @@ ScreenWindow::LayoutControls(uint32 flags)
 	BRect boxFrame = fScreenBox->Frame() | fControlsBox->Frame();
 
 	// layout rest of buttons
-	fDefaultsButton->ResizeToPreferred();
+	// TODO: we don't support getting the screen's preferred settings
-	fDefaultsButton->MoveTo(boxFrame.left, boxFrame.bottom + 8);
+//	fDefaultsButton->ResizeToPreferred();
+//	fDefaultsButton->MoveTo(boxFrame.left, boxFrame.bottom + 8);
 	
 	fRevertButton->ResizeToPreferred();
-	fRevertButton->MoveTo(fDefaultsButton->Frame().right + 10,
+	fRevertButton->MoveTo(boxFrame.left, boxFrame.bottom + 8);
-						  fDefaultsButton->Frame().top);
+//	fRevertButton->MoveTo(fDefaultsButton->Frame().right + 10,
+//						  fDefaultsButton->Frame().top);
 	
 	fApplyButton->ResizeToPreferred();
-	fApplyButton->MoveTo(boxFrame.right - fApplyButton->Bounds().Width(),
+	float resolutionFieldRight = fResolutionField->Frame().right;
-						 fDefaultsButton->Frame().top);
+	fApplyButton->MoveTo(resolutionFieldRight - fApplyButton->Bounds().Width(),
+				fControlsBox->Bounds().bottom - fApplyButton->Bounds().Height()
+					- (fControlsBox->Bounds().right - resolutionFieldRight));
 
-	ResizeTo(boxFrame.right + 10, fDefaultsButton->Frame().bottom + 10);
+	ResizeTo(boxFrame.right + 10, fRevertButton->Frame().bottom + 10);
 }
 
 

src/preferences/screen/ScreenWindow.h

@@ -46,6 +46,7 @@ class ScreenWindow : public BWindow {
 		void UpdateRefreshControl();
 		void UpdateMonitorView();
 		void UpdateControls();
+		void UpdateOriginal();
 
 		void Apply();
 
@@ -93,9 +94,12 @@ class ScreenWindow : public BWindow {
 
 		BButton*		fBackgroundsButton;
 
-		ScreenMode		fScreenMode;
+		ScreenMode		fScreenMode, fTempScreenMode;
-		bool			fChangingAllWorkspaces;
+			// screen modes for all workspaces
+		int32			fOriginalWorkspaceCount;
 		screen_mode		fActive, fSelected, fOriginal;
+			// screen modes for the current workspace
+		bool			fModified;
 };
 
 #endif	/* SCREEN_WINDOW_H */