Author Topic: Debugger branch: Placement of Windows  (Read 34453 times)

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Debugger branch: Placement of Windows
« Reply #15 on: January 19, 2012, 09:03:02 am »
So, what should we/I do to fix this problem? Use both functions?
I would try this:
You can call PlaceWindow with pdlCentre as second and true as third parameter to enforce placing the window (more or less) regardless the settings in "Settings -> View" .

But I think we should wait until ollydbg has tested, if it really works without additional tweaking inside the dialog-constructor

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #16 on: January 19, 2012, 09:53:19 am »
But I think we should wait until ollydbg has tested, if it really works without additional tweaking inside the dialog-constructor
@jens: I got confused :(, adding "CentreOnParent()" in the constructor works fine here. (WinXP, Single Monitor)
What should I do right now?
Do I need to test the PlaceWindow() function? How to test it?

Quote
If the Multimonitor placement checkbox  is unchecked and PlaceWindow() is not called with the third parameter set to true (defaults to false), it does nothing.
See sdk/globals.cpp:1017 .
The line 1017 is related to wxGTK, I have only Windows XP here.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Debugger branch: Placement of Windows
« Reply #17 on: January 19, 2012, 09:57:38 am »
ollydbg: you have to test without CenterToParent and with PlaceWindow(..., pdlCentre, ...) instead.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #18 on: January 19, 2012, 10:15:36 am »
ollydbg: you have to test without CenterToParent and with PlaceWindow(..., pdlCentre, ...) instead.
I just add one line in the end of constructor.
Code
PlaceWindow(this,pdlCentre,true);
It works fine.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Debugger branch: Placement of Windows
« Reply #19 on: January 19, 2012, 10:19:23 am »
Can you change the PlaceWindow line, where ShowModal is called (it is in main.cpp) and remove your PlaceWindow from the constructor?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #20 on: January 19, 2012, 10:26:26 am »
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:
Code
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.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #21 on: January 27, 2012, 07:01:37 am »
Any news about this issue?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: Debugger branch: Placement of Windows
« Reply #22 on: January 27, 2012, 09:12:42 am »
Any news about this issue?
The code from your previous post in this thread looks good to me. I guess it will be applied with the next commit of oBFusCATed.
Edit: BTW: Shouldn't this be the same with the compiler settings dialog then, too?
« Last Edit: January 27, 2012, 09:15:42 am by MortenMacFly »
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Debugger branch: Placement of Windows
« Reply #23 on: January 27, 2012, 01:13:27 pm »
I have no intention to fix this, because I don't see the problem on linux, so I can't test it.

And yes, both dialogs should behave the same.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #24 on: February 11, 2012, 10:30:16 am »
RFA:
Is this patch OK?
Code
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:
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.

« Last Edit: February 11, 2012, 10:32:48 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Debugger branch: Placement of Windows
« Reply #25 on: February 11, 2012, 10:59:58 am »
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!')
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #26 on: February 11, 2012, 11:53:16 am »
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:
Code
CompilerSettingsDlg::CompilerSettingsDlg(wxWindow* parent)
{
......

    // make sure everything is laid out properly
    GetSizer()->SetSizeHints(this);
    CentreOnParent();
}
See, it is already CentreOnparent() on the constructor of the dialog.

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().
Code
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?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Debugger branch: Placement of Windows
« Reply #27 on: February 11, 2012, 12:31:50 pm »
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 can not test at the moment.

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).

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Debugger branch: Placement of Windows
« Reply #28 on: February 11, 2012, 12:39:52 pm »
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:
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:
Code
    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.

Quote
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?
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: Debugger branch: Placement of Windows
« Reply #29 on: February 11, 2012, 12:46:50 pm »
The settings dialog should be overworked, to make clear we can place windows in multimonitor and standard mode.
The user can decide where the window should be opened and not be forced to always get it centred on parent.

We can have centre on screen (that's what we have already, if I remember correctly), centre on parent (that is forced by the congtructor in trunk at the moment (for debuggersettings), and possible other settings, like left-top, at mouse-üposition, on left monitor, on right monitor or whatever might be useful.

It should not be so hard to implement.

I can see if I find the time and provide a patch for testing.