Improve and test 3.0 compatibility in wxPG (PR #23375)
VZ
This replaces #23371.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/23375
Commit Summary
- cb03ddf Restore compatibility with 3.0-like DrawCaptionSelectionRect()
- eb72571 Simplify the use of wxPG_DEPRECATED_MACRO_VALUE
- 650aeb7 Update options used in CI builds for testing compatibility
File Changes
(6 files)
- M .github/workflows/ci.yml (4)
- M .github/workflows/ci_msw_cross.yml (2)
- M docs/changes.txt (4)
- M include/wx/defs.h (4)
- M include/wx/propgrid/property.h (21)
- M src/propgrid/property.cpp (50)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/23375.patch
- https://github.com/wxWidgets/wxWidgets/pull/23375.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AW
@a-wi commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
To me the intent of this this logic is not clear but if you want to do something like this, the better place would be probably wxPGDrawFocusRect, where you know what DrawCaptionSelectionRect calls it (win ==/ != nullptr) and where you already have conditional blocks for v3.0 compatibility code.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
The intent here is to call the overridden function with the old signature as this is what's really needed for compatibility sand this wasn't done any longer after the latest changes (before them the old function was called but the new one was not, which wasn't ideal neither).
This couldn't be done solely in wxPGDrawFocusRect().
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AW
@a-wi commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
OK, but I still don't get why do you insist on using DrawCaptionSelectionRect() with old signature in wx code in v3.0 mode.
In this mode DrawCaptionSelectionRect() is defined in two versions: with old and new signature, so why not to use only new one internally? I think version with old signature should be only used (potentially) by user app (in v3.0 mode).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
It's a virtual function, so I assume that it can be overridden in the user code, so we need to call it. If not, why was it virtual in the first place?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AW
@a-wi commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
OK, good point. But maybe simpler implementation would be just to call in v3.0 mode DrawCaptionSelectionRect() with old signature from DrawCaptionSelectionRect() with new signature? I mean something like this:
DrawCaptionSelectionRect(wxWindow*, wxDC& dc,...)
{
#if v3.0
DrawCaptionSelectionRect(dc, ...)
#else
wxPGDrawFocusRect(...)
#endif
}
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
This means enabling 3.0 compatibility (which might be needed for something completely unrelated) would make wxPG draw uglier looking focus rect by default, which doesn't seem good. I.e. the goal is to always call wxPGDrawFocusRect() with a valid window by default, but still call overridden virtual function with 3.0-compatible signature if 3.0 compatibility is enabled.
And I still don't see any simpler/better way to do it than the hack in this PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
@a-wi Do you still object to merging this in the current state and/or see a better way to handle this?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + if ( wxPGCellRendererDrawCaptionSelectionRectFlag )
+ {
+ // This means that the user-defined overridden version of the
+ // function was called -- because the base class one was not.
+ // So just reset the flag and don't do anything else.
+ wxPGCellRendererDrawCaptionSelectionRectFlag = false;
+ }
+ else
+ // This means that our own version was called, so we now need
+ // to call the other overload, to use the user-defined version
+ // if any or to just draw the rectangle in the base class
+ // version otherwise.
+#endif // WXWIN_COMPATIBILITY_3_0
+ DrawCaptionSelectionRect( const_cast<wxPropertyGrid*>(propertyGrid), dc,
+ rectCaption.x, rectCaption.y,
+ rectCaption.width, rectCaption.height );
I'm going to push this, we can always change it later if you find a better way 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.
VZ
Merged #23375 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.