ollydbg: you'll have to debug it yourself, because here it works as intended, but I'm running bigger screens than yours (1600x1200 and 1920x1080). The code for compiler settings and debugger settings is pretty much the same. The only difference is that the Compiler settings are created from xrc resource, but debugger settings are created with wxSmith.
I just debug it myself, and please add one line statement to fix this issue.
In the end of function body, in file: src\debuggersettingsdlg.cpp around line 108.
DebuggerSettingsDlg::DebuggerSettingsDlg(wxWindow* parent)
ollydbg: you have to test without CenterToParent and with PlaceWindow(..., pdlCentre, ...) instead.
I just add one line in the end of constructor.
PlaceWindow(this,pdlCentre,true);
Can you change the PlaceWindow line, where ShowModal is called (it is in main.cpp) and remove your PlaceWindow from the constructor?
Ok, I just test it like:
void MainFrame::OnSettingsDebugger(wxCommandEvent& /*event*/)
{
DebuggerSettingsDlg dlg(this);
PlaceWindow(&dlg,pdlCentre,true); //change here
if (dlg.ShowModal() == wxID_OK)
{
CodeBlocksEvent event(cbEVT_SETTINGS_CHANGED);
event.SetInt(cbSettingsType::Debugger);
Manager::Get()->ProcessEvent(event);
}
}
Works fine too.
PS: PlaceWindow(&dlg,pdlCentre,true) does not consider the parent window position, it always set window pos based on the screen.
RFA:
Is this patch OK?
Index: E:/code/cb/cb_debugger_branch/src/src/main.cpp
===================================================================
--- E:/code/cb/cb_debugger_branch/src/src/main.cpp (revision 7791)
+++ E:/code/cb/cb_debugger_branch/src/src/main.cpp (working copy)
@@ -4403,7 +4403,7 @@
bool needRestart = false;
EnvironmentSettingsDlg dlg(this, m_LayoutManager.GetArtProvider());
- PlaceWindow(&dlg);
+ PlaceWindow(&dlg,pdlCentre,true);
if (dlg.ShowModal() == wxID_OK)
{
DoUpdateEditorStyle();
@@ -4431,7 +4431,7 @@
void MainFrame::OnSettingsCompiler(wxCommandEvent& /*event*/)
{
CompilerSettingsDlg dlg(this);
- PlaceWindow(&dlg);
+ PlaceWindow(&dlg,pdlCentre,true);
if (dlg.ShowModal() == wxID_OK)
{
CodeBlocksEvent event(cbEVT_SETTINGS_CHANGED);
@@ -4443,7 +4443,7 @@
void MainFrame::OnSettingsDebugger(wxCommandEvent& /*event*/)
{
DebuggerSettingsDlg dlg(this);
- PlaceWindow(&dlg);
+ PlaceWindow(&dlg,pdlCentre,true);
if (dlg.ShowModal() == wxID_OK)
{
CodeBlocksEvent event(cbEVT_SETTINGS_CHANGED);
I change three places, compiler/debugger/environment dialog setting placement.
EDIT
I see the code in trunk code:
void MainFrame::OnSettingsCompilerDebugger(wxCommandEvent& /*event*/)
{
CompilerSettingsDlg dlg(this);
PlaceWindow(&dlg);
dlg.ShowModal();
}
But the compilerdebugger dialog shows correctly. I'm not sure why this does not works under debugger_branch.
You can debug it, can't you?
If it does work in trunk, then it is a bug in the branch, so your patch is not correct.
The bug should be fixed.
(If you remember Thomas had an old signature, which was 100% true: 'Never fix a bug you don't understand!')
Ok, I debugged it and find the reason:
The trunk code:
CompilerSettingsDlg::CompilerSettingsDlg(wxWindow* parent)
{
......
// make sure everything is laid out properly
GetSizer()->SetSizeHints(this);
CentreOnParent();
}
But the debugger branch, there is no such code in DebuggerSettingsDlg::DebuggerSettingsDlg().
But the debugger branch, it DO have a CentreOnParent(); in the CompilerSettingsDlg::CompilerSettingsDlg().
CompilerSettingsDlg::CompilerSettingsDlg(wxWindow* parent)
{
// make sure everything is laid out properly
GetSizer()->SetSizeHints(this);
CentreOnParent();
}
The solution should be:
Adding CentreOnParent(); in DebuggerSettingsDlg::DebuggerSettingsDlg(), right?
The solution should be:
Adding CentreOnParent(); in DebuggerSettingsDlg::DebuggerSettingsDlg(), right?
It will most likely break the configuration made in "Settings -> View -> Enhanced multi-monitor placement", and if it does, these settings are ignored in trunk already.
I'm not fully understand your meaning, here is the code:
void PlaceWindow(wxTopLevelWindow *w, cbPlaceDialogMode mode, bool enforce)
{
HMONITOR hMonitor;
MONITORINFO mi;
RECT r;
int the_mode;
if (!w)
cbThrow(_T("Passed NULL pointer to PlaceWindow."));
wxWindow* referenceWindow = Manager::Get()->GetAppWindow();
if (!referenceWindow) // no application window available, so this is as good as we can get
referenceWindow = w;
wxRect windowRect = w->GetRect();
ConfigManager *cfg = Manager::Get()->GetConfigManager(_T("app"));
if (!enforce && cfg->ReadBool(_T("/dialog_placement/do_place")) == false)
return;
if (mode == pdlBest)
the_mode = cfg->ReadInt(_T("/dialog_placement/dialog_position"), (int) pdlCentre);
else
the_mode = (int) mode;
static MonitorFromWindow_t MonitorFromWindowProc = (MonitorFromWindow_t) GetProcAddress(GetModuleHandle(_T("user32.dll")), "MonitorFromWindow");
static GetMonitorInfo_t GetMonitorInfoProc = (GetMonitorInfo_t) GetProcAddress(GetModuleHandle(_T("user32.dll")), "GetMonitorInfoA");
int monitorWidth;
int monitorHeight;
if (GetMonitorInfoProc)
{
hMonitor = MonitorFromWindowProc((HWND) referenceWindow->GetHandle(), MONITOR_DEFAULTTONEAREST);
mi.cbSize = sizeof(mi);
GetMonitorInfoProc(hMonitor, &mi);
r = mi.rcWork;
monitorWidth = r.right - r.left;
monitorHeight = r.bottom - r. top;
}
else // Win95, NT4: support only single monitor
{
wxDisplaySize(&monitorWidth, &monitorHeight);
r.left = r.top = 0;
}
switch(the_mode)
{
case pdlCentre:
{
windowRect.x = r.left + (monitorWidth - windowRect.width)/2;
windowRect.y = r.top + (monitorHeight - windowRect.height)/2;
}
break;
case pdlHead:
{
windowRect.x = r.left + (monitorWidth - windowRect.width)/2;
windowRect.y = r.top + (monitorHeight - windowRect.height)/3;
}
break;
case pdlConstrain:
{
int x1 = windowRect.x;
int x2 = windowRect.x + windowRect.width;
int y1 = windowRect.y;
int y2 = windowRect.y + windowRect.height;
if (windowRect.width > monitorWidth) // cannot place without clipping, so centre it
{
x1 = r.left + (monitorWidth - windowRect.width)/2;
x2 = x1 + windowRect.width;
}
else
{
x2 = std::min((int) r.right, windowRect.GetRight());
x1 = std::max(x2 - windowRect.width, (int) r.left);
x2 = x1 + windowRect.width;
}
if (windowRect.height > monitorHeight) // cannot place without clipping, so centre it
{
y1 = r.top + (monitorHeight - windowRect.height)/2;
y2 = y1 + windowRect.height;
}
else
{
y2 = std::min((int) r.bottom, windowRect.GetBottom());
y1 = std::max(y2 - windowRect.height, (int) r.top);
y2 = y1 + windowRect.height;
}
windowRect = wxRect(x1, y1, x2-x1, y2-y1);
}
break;
case pdlClip:
{
int x1 = windowRect.x;
int x2 = windowRect.x + windowRect.width;
int y1 = windowRect.y;
int y2 = windowRect.y + windowRect.height;
x1 = std::max(x1, (int) r.left);
x2 = std::min(x2, (int) r.right);
y1 = std::max(y1, (int) r.top);
y2 = std::min(y2, (int) r.bottom);
windowRect = wxRect(x1, y1, x2-x1, y2-y1);
}
break;
}
w->SetSize(windowRect.x, windowRect.y, windowRect.width, windowRect.height, wxSIZE_ALLOW_MINUS_ONE);
}
I do not check on any multiply-monitor related options, so this function is in fact do nothing and return from:
if (!enforce && cfg->ReadBool(_T("/dialog_placement/do_place")) == false)
return;
So, if you have "Settings -> View -> Enhanced multi-monitor placement" settings check on, this function will go further to adjust the dialog position.
I think this should be th eplace for changes:
The settings dialog, to be clear that it can be used for multimonitor systems and for single monitors, and possible the ability to centre a dialog oin the parent and not only on the screen (Changes in PlaceWindow).
Not fully understand this either. Sorry. :), you mean the dialog SHOULD be placed in the center of parent window or screen?
BTW, notice that the PlaceWindow method uses "pdlBest" as default argument for the method. So unless its really provided/enforced by the caller, the following code:
if (mode == pdlBest)
the_mode = cfg->ReadInt(_T("/dialog_placement/dialog_position"), (int) pdlCentre);
else
the_mode = (int) mode;
So if there is no good reason you should always call PlaceWindow without additional argument.
You mean we should always do like below (call PlaceWindow() with only first parameter)
CompilerSettingsDlg dlg(this);
PlaceWindow(&dlg);
In this case, If I would like place dlg in the center of the screen. I need to check on both:
View->Enhance multi-monitor dialog place and Move to "head up" position. Otherwise, the dialog place badly as my original post in this discussion thread.
That's not intuitive, because I work on a Single monitor Laptop.
I even can't see the buttons of the dialog. This is the same behavior in both Windows and Ubuntu 10.10.
That's why I said its an improper design of the dialog itself then. the constructor should make everything needed/possible to properly position the dialog initially. Then PlaceWindow may adjust it to the users needs in case of multiple monitors.
For the debugger settings pane I see two drawbacks in its contructor:
1.) its wxSmith based, but sets:
SetMinSize(wxSize(600, 600));
SetSize(wxSize(600, 600));
2.) it does not center itself on the parent (if any).
Both should be adjusted using wxSmith. Than you should be happy.
The same applies to all other dialogs that do not behave / position correctly.
@morten:
I see you change in rev7950
===================================================================
--- trunk/src/src/debuggersettingsdlg.cpp (revision 7949)
+++ trunk/src/src/debuggersettingsdlg.cpp (revision 7950)
@@ -60,15 +60,16 @@
m_treebook->SetMinSize(wxSize(600,440));
mainSizer->Add(m_treebook, 1, wxALL|wxEXPAND|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL, 5);
staticLine = new wxStaticLine(this, wxID_ANY, wxDefaultPosition, wxSize(10,-1), wxLI_HORIZONTAL, _T("wxID_ANY"));
- mainSizer->Add(staticLine, 0, wxALL|wxEXPAND|wxALIGN_LEFT|wxALIGN_BOTTOM, 5);
+ mainSizer->Add(staticLine, 0, wxBOTTOM|wxLEFT|wxRIGHT|wxEXPAND|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL, 5);
stdDialogButtons = new wxStdDialogButtonSizer();
stdDialogButtons->AddButton(new wxButton(this, wxID_OK, wxEmptyString));
stdDialogButtons->AddButton(new wxButton(this, wxID_CANCEL, wxEmptyString));
stdDialogButtons->Realize();
- mainSizer->Add(stdDialogButtons, 0, wxALL|wxALIGN_BOTTOM|wxALIGN_CENTER_HORIZONTAL, 5);
+ mainSizer->Add(stdDialogButtons, 0, wxBOTTOM|wxLEFT|wxRIGHT|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL, 5);
SetSizer(mainSizer);
mainSizer->Fit(this);
mainSizer->SetSizeHints(this);
+ Center();
Connect(ID_TREEBOOK,wxEVT_COMMAND_TREEBOOK_PAGE_CHANGED,(wxObjectEventFunction)&DebuggerSettingsDlg::OnPageChanged);
//*)
Please put the Center command in the end of the constructor like:
Index: E:/code/cb/cb_trunk/src/src/debuggersettingsdlg.cpp
===================================================================
--- E:/code/cb/cb_trunk/src/src/debuggersettingsdlg.cpp (revision 7950)
+++ E:/code/cb/cb_trunk/src/src/debuggersettingsdlg.cpp (working copy)
@@ -100,6 +100,8 @@
for (size_t ii = 0; ii < m_treebook->GetPageCount(); ++ii)
m_treebook->ExpandNode(ii);
+
+ Center();
}
DebuggerSettingsDlg::~DebuggerSettingsDlg()
This works OK.
The another issue is: If you search the word "CentreOnParent", you will see many dialog's constructor will call this in the end. As we discussed before, We should be consistent. Either all Center() or all CentreOnParent().
What do you think? (As jens said here: Re: Debugger branch: Placement of Windows (http://forums.codeblocks.org/index.php/topic,15818.msg107377.html#msg107377))
I think Center() is preferred. Right?