Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected Discrepancy Between Expected and Actual UI Positions When Using wxGridBagSizer. #24379

Open
kahuz opened this issue Mar 6, 2024 · 5 comments

Comments

@kahuz
Copy link

kahuz commented Mar 6, 2024

Platform and version information

  • wxWidgets version you use : 3.2.2.6
  • wxWidgets port : wxMSW
  • OS and its version: Windows 10

Description

I have encountered a bug while developing a UI program using wxWidgets. I am planning to report this issue on wxWidgets' GitHub.

I am using wxGridBagSizer to implement the same UI according to the design layout. When placing myButton in wxGridBagSizer, I expected that specifying the desired starting position of the Grid (wxGBPosition) and the cell area where myButton is intended to be placed (wxGBSpan) would result in the implementation as I desire.

For example:

  • wxGridBagSizer->Add(myButton, wxGBPosition(cell_row_pos, cell_col_pos), wxGBSpan(cell_row_stretch_length, cell_col_stretch_length), wxALL, 0 /*No Border*/);

However, as seen in the picture and code below, the actual output differs from my expectations.

  • The red squares represent the cell areas.

image

SizerPreviewPanel::SizerPreviewPanel(wxFrame* parent, const wxPoint& pos, const wxSize& size, long style)
    : wxPanel(parent, wxID_ANY, pos, size, style)
{
    Initialize_(pos, size, style);
}

void SizerPreviewPanel::Initialize_(const wxPoint& pos, const wxSize& size, long style)
{
    auto defaultFont = wxSystemSettings::GetFont(wxSystemFont::wxSYS_DEFAULT_GUI_FONT);
    defaultFont.SetPointSize(10);
    SetBackgroundColour(wxT("#AAAAAA"));
    InitializeUiWithSizer_();
    BuildUiLayout_();
}
void SizerPreviewPanel::InitializeUiWithSizer_()
{
    wxSize tmpSize{ 50,14 };

    btnOne_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
    btnOne_->SetBackgroundColour(wxT("#CCCCCC"));

    staticTextOne_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
    staticTextOne_->SetBackgroundColour(wxT("#CCCCCC"));

    bitmapBtnOne_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
    bitmapBtnOne_->SetBackgroundColour(wxT("#CCCCCC"));

    btnTwo_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
    btnTwo_->SetBackgroundColour(wxT("#CCCCCC"));

    staticTextTwo_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
    staticTextTwo_->SetBackgroundColour(wxT("#CCCCCC"));

    bitmapBtnTwo_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
    bitmapBtnTwo_->SetBackgroundColour(wxT("#CCCCCC"));
}

void SizerPreviewPanel::BuildUiLayout_()
{
    wxBoxSizer* baseSizer = new wxBoxSizer(wxVERTICAL);

    wxGridBagSizer* gridBagSizer = new wxGridBagSizer(0, 0);
    gridBagSizer->SetEmptyCellSize(wxSize(24, 18));
    gridBagSizer->Add(btnOne_, wxGBPosition(1, 1), wxGBSpan(1, 3), wxALL, 0);
    gridBagSizer->Add(staticTextOne_, wxGBPosition(1, 5), wxGBSpan(1, 3), wxALL, 0);
    gridBagSizer->Add(bitmapBtnOne_, wxGBPosition(1, 9), wxGBSpan(1, 6), wxALL, 0);

    gridBagSizer->Add(btnTwo_, wxGBPosition(5, 1), wxGBSpan(1, 3), wxALL, 0);
    gridBagSizer->Add(staticTextTwo_, wxGBPosition(5, 5), wxGBSpan(1, 3), wxALL, 0);
    gridBagSizer->Add(bitmapBtnTwo_, wxGBPosition(5, 9), wxGBSpan(1, 6), wxALL, 0);
    
    baseSizer->Add(gridBagSizer, 1, wxALL, 0);
    
    SetSizer(baseSizer);
    Layout();

For ButtonOne, it was created at the expected position of row 1, col 1 as per the code. However, for other UI elements (e.g., Static Text One, ButtonTwo, Static Text Two, etc..), it was observed that they deviated slightly from the specified position in the code.

Could you provide assistance on how I should approach and modify the code to resolve this issue?

@vadz
Copy link
Contributor

vadz commented Mar 7, 2024

I've tried testing using the provided code by putting it into the minimal sample and I don't see the problem here. Could you please confirm that it happens for you with this patch and, if it doesn't, modify to actually show it?

Patch to minimal sample
diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
index 5f32257c6b..c99edda16d 100644
--- a/samples/minimal/minimal.cpp
+++ b/samples/minimal/minimal.cpp
@@ -26,6 +26,8 @@
     #include "wx/wx.h"
 #endif
 
+#include "wx/gbsizer.h"
+
 // ----------------------------------------------------------------------------
 // resources
 // ----------------------------------------------------------------------------
@@ -170,6 +172,52 @@ MyFrame::MyFrame(const wxString& title)
     SetSizer(sizer);
 #endif // wxUSE_MENUBAR/!wxUSE_MENUBAR
 
+    struct MyPanel : wxPanel {
+        explicit MyPanel(wxWindow* parent)
+            : wxPanel(parent, wxID_ANY)
+        {
+            SetBackgroundColour(wxT("#AAAAAA"));
+
+            wxSize tmpSize{ 50,14 };
+
+            auto btnOne_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            btnOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto staticTextOne_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            staticTextOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto bitmapBtnOne_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
+            bitmapBtnOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto btnTwo_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            btnTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto staticTextTwo_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            staticTextTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto bitmapBtnTwo_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
+            bitmapBtnTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            wxBoxSizer* baseSizer = new wxBoxSizer(wxVERTICAL);
+
+            wxGridBagSizer* gridBagSizer = new wxGridBagSizer(0, 0);
+            gridBagSizer->SetEmptyCellSize(wxSize(24, 18));
+            gridBagSizer->Add(btnOne_, wxGBPosition(1, 1), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(staticTextOne_, wxGBPosition(1, 5), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(bitmapBtnOne_, wxGBPosition(1, 9), wxGBSpan(1, 6), wxALL, 0);
+
+            gridBagSizer->Add(btnTwo_, wxGBPosition(5, 1), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(staticTextTwo_, wxGBPosition(5, 5), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(bitmapBtnTwo_, wxGBPosition(5, 9), wxGBSpan(1, 6), wxALL, 0);
+
+            baseSizer->Add(gridBagSizer, 1, wxALL, 0);
+
+            SetSizer(baseSizer);
+        }
+    };
+
+    new MyPanel(this);
+
 #if wxUSE_STATUSBAR
     // create a status bar just for fun (by default with 1 pane only)
     CreateStatusBar(2);

P.S. Just to be clear: your code doesn't show how do you draw the lines on the screenshot, so I wonder if the lines could be wrong, rather than positions of the controls.

@vadz vadz added the repro needed A way to reproduce the problem is required label Mar 7, 2024
@kahuz
Copy link
Author

kahuz commented Mar 7, 2024

@vadz

I have switched the version to v3.2.3 to clarify the issue, and retested it in the minimal sample.

The problem persists even in this scenario.

Below is the code with an added function for drawing a grid cell in the minimal sample:

sample code

Github Gist

sample code gist

Git Diff

warning: in the working copy of 'samples/minimal/minimal.cpp', LF will be replaced by CRLF the next time Git touches it
diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
index 3d71bd276c..3684f06115 100644
--- a/samples/minimal/minimal.cpp
+++ b/samples/minimal/minimal.cpp
@@ -25,7 +25,7 @@
 #ifndef WX_PRECOMP
     #include "wx/wx.h"
 #endif
-
+#include "wx/gbsizer.h"
 // ----------------------------------------------------------------------------
 // resources
 // ----------------------------------------------------------------------------
@@ -53,18 +53,52 @@ public:
     virtual bool OnInit() wxOVERRIDE;
 };

+const int kGridCellWidth = 24;
+const int kGridCellHeight = 18;
 // Define a new frame type: this is going to be our main frame
 class MyFrame : public wxFrame
 {
+private:
+    struct DrawingLayoutInfo_
+    {
+        wxPoint pos{ 0,0 };
+        wxSize size{ 0,0 };
+        wxColour color{ wxT("#000000") };
+    };
+
+    const int kDefaultPenThickness = 1;
+
 public:
     // ctor(s)
-    MyFrame(const wxString& title);
+    MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size);

+    void DrawSizerLayout(wxPanel* basePanel, const int& drawAreaWidth, const int& drawAreaHeight);
     // event handlers (these functions should _not_ be virtual)
     void OnQuit(wxCommandEvent& event);
     void OnAbout(wxCommandEvent& event);

 private:
+    void AddDrawLayouts_(wxPanel* baseWindow, const std::vector<DrawingLayoutInfo_>& layoutInfos, bool forceDraw)
+    {
+        baseWindow->Bind(wxEVT_PAINT, [this, baseWindow, layoutInfos](wxPaintEvent& inEvent)
+            {
+                wxPaintDC dc(baseWindow);
+                dc.SetBrush(*wxTRANSPARENT_BRUSH);
+
+                for (auto layoutInfo : layoutInfos)
+                {
+                    dc.SetPen(wxPen(layoutInfo.color, kDefaultPenThickness));
+                    dc.DrawRectangle({ layoutInfo.pos.x,layoutInfo.pos.y }, layoutInfo.size);
+                }
+            }
+        );
+
+        if (forceDraw)
+        {
+            baseWindow->Refresh();
+            baseWindow->Update();
+        }
+    }
     // any class wishing to process wxWidgets events must use this macro
     wxDECLARE_EVENT_TABLE();
 };
@@ -121,7 +155,7 @@ bool MyApp::OnInit()
         return false;

     // create the main application window
-    MyFrame *frame = new MyFrame("Minimal wxWidgets App");
+    MyFrame *frame = new MyFrame("Minimal wxWidgets App", wxDefaultPosition, wxSize(600,500));

     // and show it (the frames, unlike simple controls, are not shown when
     // created initially)
@@ -138,8 +172,8 @@ bool MyApp::OnInit()
:
+                dc.SetBrush(*wxTRANSPARENT_BRUSH);
+
+                for (auto layoutInfo : layoutInfos)
+                {
+                    dc.SetPen(wxPen(layoutInfo.color, kDefaultPenThickness));
+                    dc.DrawRectangle({ layoutInfo.pos.x,layoutInfo.pos.y }, layoutInfo.size);
+                }
+            }
+        );
+
+        if (forceDraw)
+        {
+            baseWindow->Refresh();
+            baseWindow->Update();
+        }
+    }
     // any class wishing to process wxWidgets events must use this macro
     wxDECLARE_EVENT_TABLE();
 };
@@ -121,7 +155,7 @@ bool MyApp::OnInit()
         return false;

     // create the main application window
-    MyFrame *frame = new MyFrame("Minimal wxWidgets App");
+    MyFrame *frame = new MyFrame("Minimal wxWidgets App", wxDefaultPosition, wxSize(600,500));

     // and show it (the frames, unlike simple controls, are not shown when
     // created initially)
@@ -138,8 +172,8 @@ bool MyApp::OnInit()
 // ----------------------------------------------------------------------------

 // frame constructor
-MyFrame::MyFrame(const wxString& title)
-       : wxFrame(NULL, wxID_ANY, title)
+MyFrame::MyFrame(const wxString& title, const wxPoint& pos, const wxSize& size)
+       : wxFrame(NULL, wxID_ANY, title, pos, size)
 {
     // set the frame icon
     SetIcon(wxICON(sample));
@@ -169,6 +203,55 @@ MyFrame::MyFrame(const wxString& title)
     sizer->Add(aboutBtn, wxSizerFlags().Center());
     SetSizer(sizer);
 #endif // wxUSE_MENUBAR/!wxUSE_MENUBAR
+    struct MyPanel : wxPanel {
+        explicit MyPanel(wxWindow* parent, const wxPoint& pos, const wxSize& size)
+            : wxPanel(parent, wxID_ANY, pos, size)
+        {
+            SetBackgroundColour(wxT("#AAAAAA"));
+
+            wxSize tmpSize{ 50,14 };
+
+            auto btnOne_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            btnOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto staticTextOne_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            staticTextOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto bitmapBtnOne_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
+            bitmapBtnOne_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto btnTwo_ = new wxButton(this, wxID_ANY, "ButtonOne", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            btnTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto staticTextTwo_ = new wxStaticText(this, wxID_ANY, "Static Text Two", wxDefaultPosition, wxDefaultSize, wxBORDER_NONE);
+            staticTextTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            auto bitmapBtnTwo_ = new wxBitmapButton(this, wxID_ANY, wxNullBitmap, wxDefaultPosition, tmpSize, wxBORDER_NONE);
+            bitmapBtnTwo_->SetBackgroundColour(wxT("#CCCCCC"));
+
+            wxBoxSizer* baseSizer = new wxBoxSizer(wxVERTICAL);
+
+            wxGridBagSizer* gridBagSizer = new wxGridBagSizer(0, 0);
+            gridBagSizer->SetEmptyCellSize(wxSize(kGridCellWidth, kGridCellHeight));
+            gridBagSizer->Add(btnOne_, wxGBPosition(1, 1), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(staticTextOne_, wxGBPosition(1, 5), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(bitmapBtnOne_, wxGBPosition(1, 9), wxGBSpan(1, 6), wxALL, 0);
+
+            gridBagSizer->Add(btnTwo_, wxGBPosition(5, 1), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(staticTextTwo_, wxGBPosition(5, 5), wxGBSpan(1, 3), wxALL, 0);
+            gridBagSizer->Add(bitmapBtnTwo_, wxGBPosition(5, 9), wxGBSpan(1, 6), wxALL, 0);
+
+            baseSizer->Add(gridBagSizer, 1, wxALL, 0);
+
+            SetSizer(baseSizer);
+        }
+    };
+
+    const int panelWidth = 400;
+    const int panelHeight = 300;
+    auto myPanel = new MyPanel(this, wxDefaultPosition, wxSize(panelWidth, panelHeight));
+
+    DrawSizerLayout(myPanel, panelWidth, panelHeight);

 #if wxUSE_STATUSBAR
     // create a status bar just for fun (by default with 1 pane only)
@@ -177,6 +260,28 @@ MyFrame::MyFrame(const wxString& title)
 #endif // wxUSE_STATUSBAR
 }

+void MyFrame::DrawSizerLayout(wxPanel* basePanel, const int& drawAreaWidth, const int& drawAreaHeight)
+{
+    std::vector<DrawingLayoutInfo_> dawingLayoutInfos;
+
+    int columnWidth = kGridCellWidth;
+    int columnHeight = kGridCellHeight;
+    int drawLimitColumnIdx = drawAreaWidth / columnWidth;
+    int drawLimitRowIdx = drawAreaHeight / columnHeight;
+
+    wxColour drawColour = { wxT("#AA0000") };
+
+    for (int rowIdx = 0; rowIdx < drawLimitRowIdx; ++rowIdx)
+    {
+        for (int drawColumn = 1; drawColumn <= drawLimitColumnIdx; ++drawColumn)
+        {
+            dawingLayoutInfos.emplace_back(DrawingLayoutInfo_{ { 0, rowIdx * (columnHeight - 1) },{ columnWidth * drawColumn, columnHeight }, drawColour });
+        }
+    }
+
+    AddDrawLayouts_(basePanel, dawingLayoutInfos, true);
+}
+

 // event handlers

@github-actions github-actions bot removed the repro needed A way to reproduce the problem is required label Mar 7, 2024
@vadz
Copy link
Contributor

vadz commented Mar 8, 2024

Sorry, I don't know what happened here, but the patch above doesn't apply (error: corrupt patch at line 78 due to a colon in the first position and removing it doesn't help). Could you please remake it?

@kahuz
Copy link
Author

kahuz commented Mar 11, 2024

@vadz
Please refer to the Gist attached in the comments or check if it can be reproduced with the "draw_gird_cell.patch" attached this time.

Note: I've worked on the code based on the v3.2.3 tag, and checked it in Visual Studio 2019 Release/Dbug 64bit on Windows 10 environment.

draw_grid_cell.patch

@kahuz
Copy link
Author

kahuz commented Mar 16, 2024

@vadz
Having trouble identifying the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants