Enable double buffering by default in wxMSW (PR #22851)

84 views
Skip to first unread message

VZ

unread,
Oct 5, 2022, 9:26:51 PM10/5/22
to wx-...@googlegroups.com, Subscribed

I've simply turned on WS_EX_COMPOSITED for all our windows and so far I haven't seen anything bad happening in the few samples I've tried (drawing, grid, notebook, toolbar, widgets). This doesn't mean there are no problems at all, of course, so it would be great if people could please test their applications with this PR before it gets merged.

Please report any problems you see with it!

See #22846.


You can view, comment on, or merge this pull request online at:

  https://github.com/wxWidgets/wxWidgets/pull/22851

Commit Summary

  • 1796292 Allow passing NULL to wxWindow::CreateUsingMSWClass()
  • cd15602 Enable double buffering for all generic windows in wxMSW
  • fcc7430 Set wxALWAYS_NATIVE_DOUBLE_BUFFER to 1 to wxMSW too
  • 2a16c0e Don't bother calling SetDoubleBuffered(true) in wxComboCtrl

File Changes

(4 files)

Patch Links:


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851@github.com>

AliKet

unread,
Oct 6, 2022, 2:42:53 PM10/6/22
to wx-...@googlegroups.com, Subscribed

@AliKet commented on this pull request.


In src/msw/window.cpp:

> @@ -539,6 +539,19 @@ bool wxWindowMSW::CreateUsingMSWClass(const wxChar* classname,
     msflags &= ~WS_BORDER;
 #endif // wxUniversal
 
+    // Enable double buffering by default for all our own, i.e. not the ones
+    // using native controls, classes.
+    //
+    // Note that this function is not used for top-level windows, so we don't
+    // set this style for them, and also that setting it for children of
+    // windows that already have WS_EX_COMPOSITED set doesn't seem to have any
+    // bad effect as the style is just ignored in this case, so we don't bother
+    // checking it it's already set for the parent, even though we could.
+    if ( !classname )
+    {
+        exstyle |= WS_EX_COMPOSITED;
+    }
+

Tried this PR with the wxGrid control and it doesn't seem to work
because this style WS_EX_COMPOSITED is not applied for it (or its children) !
i.e. classname is != NULL and has this value "wxWindowNR", what does that mean ?


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/review/1133481833@github.com>

AliKet

unread,
Oct 6, 2022, 4:29:16 PM10/6/22
to wx-...@googlegroups.com, Subscribed

@AliKet commented on this pull request.


In src/msw/window.cpp:

> @@ -539,6 +539,19 @@ bool wxWindowMSW::CreateUsingMSWClass(const wxChar* classname,
     msflags &= ~WS_BORDER;
 #endif // wxUniversal
 
+    // Enable double buffering by default for all our own, i.e. not the ones
+    // using native controls, classes.
+    //
+    // Note that this function is not used for top-level windows, so we don't
+    // set this style for them, and also that setting it for children of
+    // windows that already have WS_EX_COMPOSITED set doesn't seem to have any
+    // bad effect as the style is just ignored in this case, so we don't bother
+    // checking it it's already set for the parent, even though we could.
+    if ( !classname )
+    {
+        exstyle |= WS_EX_COMPOSITED;
+    }
+

No, it's a false alarm sorry.
I just recompiled everything from the beginning and I confirm that it works perfectly well
sorry again for the noise!


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/review/1133600195@github.com>

VZ

unread,
Oct 15, 2022, 12:14:13 PM10/15/22
to wx-...@googlegroups.com, Subscribed

I'm not sure if anybody has really tested this, but I think that the only way to really get it tested is to merge it into master and see if anybody complains, so I'm going to do it.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/c1279775530@github.com>

VZ

unread,
Oct 15, 2022, 12:16:08 PM10/15/22
to wx-...@googlegroups.com, Subscribed

Merged #22851 into master.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/issue_event/7595191043@github.com>

PB

unread,
Oct 15, 2022, 4:03:06 PM10/15/22
to wx-...@googlegroups.com, Subscribed

I think I found first victims already, just by running the bundled samples.
wxAuiToolbar::OnPaint asserts when creating wxAutoBufferedPaintDC:

D:\Dev\Desktop!Lib\wxWidgets-PB\include\wx/dcbuffer.h(229): assert "win->GetBackgroundStyle() == wxBG_STYLE_PAINT" failed in wxAutoBufferedPaintDC::wxAutoBufferedPaintDC(): You need to call SetBackgroundStyle(wxBG_STYLE_PAINT) in ctor, and also, if needed, paint the background in wxEVT_PAINT handler.

I also see several drawing issues when compared to the recentish master but I am not sure if it is caused by this commit:

  1. When dragging an image in dragimg sample, neither image nor cursor are shown.
  2. When dragging a card in the forty demo, the card is not shown.
  3. When starting listctrl sample, a black rectangle is shown in place of the list for about a half a second. The background can be also observed when resizing the list. IIRC, there was an issue like this with the control, fixed loong time ago.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/c1279823432@github.com>

VZ

unread,
Oct 15, 2022, 6:54:16 PM10/15/22
to wx-...@googlegroups.com, Subscribed

Thanks for testing!

I'll check in the fix for wxAuiToolBar problem in a moment, but I'll have to look at the other ones a bit later.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/c1279846713@github.com>

Dietmar Schwertberger

unread,
Feb 12, 2024, 5:43:40 PM2/12/24
to wx-...@googlegroups.com, Subscribed

@vadz : I think I found a problem with double buffering.
When dragging a row or col, the drop marker is missing now.

E.g. in the grid sample with "Grid -> Row drag move".
In the screenshot it's between lines 6 and 7:

Before merging this branch:
grafik.png (view on web)

After:
grafik.png (view on web)

I can work around by commenting out exstyle |= WS_EX_COMPOSITED;.

The relevant code for drawing the marker is in wxGrid::ProcessRowColLabelMouseEvent.
I would guess that some additional action is required around the ClientDC, but could not see what is missing. Other code like "pressing" the row label looks similar, but does not have problems.

if ( markerPos != m_dragLastPos || markerColour != m_dragLastColour )
{
    wxClientDC dc( headerWin );
    oper.PrepareDCForLabels(this, dc);

    int markerLength = oper.Select(headerWin->GetClientSize());

    // Clean up the last indicator if position has changed
    if ( m_dragLastPos >= 0 && markerPos != m_dragLastPos )
    {
        wxPen pen(headerWin->GetBackgroundColour(), 2);
        dc.SetPen(pen);
        oper.DrawParallelLine(dc, 0, markerLength, m_dragLastPos + 1);
        dc.SetPen(wxNullPen);

        int lastLine = oper.PosToLine(this, m_dragLastPos, nullptr, false);

        if ( lastLine != -1 )
            oper.DrawLineLabel(this, dc, lastLine);
    }

    // Draw the marker
    wxPen pen(*markerColour, 2);
    dc.SetPen(pen);
    oper.DrawParallelLine(dc, 0, markerLength, markerPos + 1);
    dc.SetPen(wxNullPen);

    m_dragLastPos = markerPos;
    m_dragLastColour = markerColour;
}


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22851/c1939732120@github.com>

Reply all
Reply to author
Forward
0 new messages