From 771ebfa9a95fef46c316c9cbfb643f623525f7e1 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Mon, 1 Feb 2021 22:56:36 +0100 Subject: [PATCH 01/11] Add wxDataViewCtrl debug option to display render bounds For debugging convenience define DEBUG_RENDER_EXTENTS to draw a red rectangle around a custom rendered cell's full rectangle, and a green rect for the extent of the item appearing inside it. Custom renderers ordinarily should not draw outside of the green rect. A notable exception is drawn text, particularly multi-line ones. --- src/common/datavcmn.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 2c6fbe11d1..715bfc08cc 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -32,6 +32,10 @@ #include "wx/access.h" #endif // wxUSE_ACCESSIBILITY +// Uncomment this line to, for custom renderers, visually show the extent +// of both a cell and its item. +//#define DEBUG_RENDER_EXTENTS + const char wxDataViewCtrlNameStr[] = "dataviewCtrl"; namespace @@ -1035,6 +1039,20 @@ wxDataViewCustomRendererBase::WXCallRender(wxRect rectCell, wxDC *dc, int state) if ( m_attr.HasFont() ) changeFont.Set(m_attr.GetEffectiveFont(dc->GetFont())); +#ifdef DEBUG_RENDER_EXTENTS + { + + wxDCBrushChanger changeBrush(*dc, *wxTRANSPARENT_BRUSH); + wxDCPenChanger changePen(*dc, *wxRED); + + dc->DrawRectangle(rectCell); + + dc->SetPen(*wxGREEN); + dc->DrawRectangle(rectItem); + + } +#endif + Render(rectItem, dc, state); } From d04dfd6f2205d51bbf8224a29448b354843970ab Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Tue, 2 Feb 2021 00:01:25 +0100 Subject: [PATCH 02/11] Add render-related options to dataview sample To more easily expose problems add options to the dataview sample related to rendering of items (applying mostly to the MyListModel page only): * Use left/centre/right alignment (Ctrl+1/2/3) * Use top/centre/bottom alignment (Ctrl+4/5/6) * Toggle tall row usage (Ctrl+7) * Toggle keep on using small wx logo, regardless of row size (Ctrl+8) * Toggle multi-line text usage (Ctrl+9) --- samples/dataview/dataview.cpp | 132 ++++++++++++++++++++++++++++++++-- samples/dataview/mymodels.cpp | 26 +++++-- samples/dataview/mymodels.h | 9 ++- 3 files changed, 155 insertions(+), 12 deletions(-) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index 742002aab2..eb847cb6e0 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -71,7 +71,8 @@ public: void BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, - unsigned long style = 0); + unsigned long style = 0, + int modelFlags = wxALIGN_CENTRE); private: // event handlers @@ -167,6 +168,13 @@ private: enum Lang { Lang_English, Lang_French }; void FillIndexList(Lang lang); + // Helper for checking ModelFlags of current panel (either currently + // building or selected one). + bool HasModelFlag(int flag) const + { + return (m_modelFlags[m_currentPanel] & flag) != 0; + } + // HasValue page. void OnHasValueValueChanged(wxDataViewEvent& event); @@ -186,7 +194,9 @@ private: Page_Max }; + unsigned int m_currentPanel; wxDataViewCtrl* m_ctrl[Page_Max]; + int m_modelFlags[Page_Max]; // Some of the models associated with the controls: @@ -406,6 +416,18 @@ enum ID_HORIZ_RULES, ID_VERT_RULES, + ID_ALIGN_LEFT, + ID_ALIGN_CENTRE_H, + ID_ALIGN_RIGHT, + + ID_ALIGN_TOP, + ID_ALIGN_CENTRE_V, + ID_ALIGN_BOTTOM, + + ID_TOGGLE_USE_TALL_ROWS, + ID_TOGGLE_KEEP_LOGO_SMALL, + ID_TOGGLE_USE_MULTI_LINE_TEXT, + ID_EXIT = wxID_EXIT, // about menu @@ -449,7 +471,8 @@ enum }; wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) - EVT_MENU_RANGE( ID_MULTIPLE, ID_VERT_RULES, MyFrame::OnStyleChange ) + EVT_MENU_RANGE( ID_MULTIPLE, ID_TOGGLE_USE_MULTI_LINE_TEXT, + MyFrame::OnStyleChange ) EVT_MENU( ID_EXIT, MyFrame::OnQuit ) EVT_MENU( ID_ABOUT, MyFrame::OnAbout ) EVT_MENU( ID_CLEARLOG, MyFrame::OnClearLog ) @@ -552,6 +575,23 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int style_menu->AppendCheckItem(ID_HORIZ_RULES, "Display horizontal rules"); style_menu->AppendCheckItem(ID_VERT_RULES, "Display vertical rules"); + wxMenu* align_menu = new wxMenu; + align_menu->AppendRadioItem(ID_ALIGN_LEFT, "Left\tCtrl-1"); + align_menu->AppendRadioItem(ID_ALIGN_CENTRE_H, "Centre Horizontal\tCtrl-2"); + align_menu->AppendRadioItem(ID_ALIGN_RIGHT, "Right\tCtrl-3"); + align_menu->AppendSeparator(); + align_menu->AppendRadioItem(ID_ALIGN_TOP, "Top\tCtrl-4"); + align_menu->AppendRadioItem(ID_ALIGN_CENTRE_V, "Centre Vertical\tCtrl-5"); + align_menu->AppendRadioItem(ID_ALIGN_BOTTOM, "Bottom\tCtrl-6"); + + wxMenu* size_menu = new wxMenu; + size_menu->AppendCheckItem(ID_TOGGLE_USE_TALL_ROWS, + "Use Tall Rows\tCtrl-7"); + size_menu->AppendCheckItem(ID_TOGGLE_KEEP_LOGO_SMALL, + "Keep Logo Size Small\tCtrl-8"); + size_menu->AppendCheckItem(ID_TOGGLE_USE_MULTI_LINE_TEXT, + "Use Multi-line Text\tCtrl-9"); + wxMenu *file_menu = new wxMenu; file_menu->Append(ID_CLEARLOG, "&Clear log\tCtrl-L"); file_menu->Append(ID_GET_PAGE_INFO, "Show current &page info"); @@ -563,6 +603,8 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int file_menu->AppendCheckItem(ID_CUSTOM_HEADER_HEIGHT, "Custom header &height"); #endif // wxHAS_GENERIC_DATAVIEWCTRL file_menu->Append(ID_STYLE_MENU, "&Style", style_menu); + file_menu->Append(wxID_ANY, "&Alignment", align_menu); + file_menu->Append(wxID_ANY, "Si&ze", size_menu); file_menu->Append(ID_INC_INDENT, "&Increase indent\tCtrl-I"); file_menu->Append(ID_DEC_INDENT, "&Decrease indent\tShift-Ctrl-I"); file_menu->AppendSeparator(); @@ -749,10 +791,14 @@ MyFrame::~MyFrame() delete wxLog::SetActiveTarget(m_logOld); } -void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned long style) +void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, + unsigned long style, int modelFlags) { wxASSERT(!m_ctrl[nPanel]); // should only be initialized once + m_currentPanel = nPanel; + m_modelFlags[nPanel] = modelFlags; + switch (nPanel) { case Page_Music: @@ -844,7 +890,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l m_ctrl[Page_List] = new wxDataViewCtrl( parent, ID_ATTR_CTRL, wxDefaultPosition, wxDefaultSize, style ); - m_list_model = new MyListModel; + m_list_model = new MyListModel(modelFlags); m_ctrl[Page_List]->AssociateModel( m_list_model.get() ); wxDataViewColumn* const colCheckIconText = new wxDataViewColumn @@ -855,13 +901,17 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l wxCOL_WIDTH_AUTOSIZE ); m_ctrl[Page_List]->AppendColumn(colCheckIconText); + const int alignment = modelFlags & wxALIGN_MASK; + colCheckIconText->GetRenderer()->SetAlignment(alignment); - m_ctrl[Page_List]->AppendTextColumn("editable string", + wxDataViewColumn* const colEditable = + m_ctrl[Page_List]->AppendTextColumn("editable string", MyListModel::Col_EditableText, wxDATAVIEW_CELL_EDITABLE, wxCOL_WIDTH_AUTOSIZE, wxALIGN_NOT, wxDATAVIEW_COL_SORTABLE); + colEditable->GetRenderer()->SetAlignment(alignment); m_ctrl[Page_List]->AppendDateColumn("date", MyListModel::Col_Date); @@ -936,7 +986,11 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l wxDefaultSize, style | wxDV_NO_HEADER ); m_ctrl[Page_TreeStore] = tc; - wxImageList *ilist = new wxImageList( 16, 16 ); + const bool useDefaultSize = !HasModelFlag(MODEL_USE_TALL_ROWS) + || HasModelFlag(MODEL_KEEP_LOGO_SMALL); + const int imageSize = useDefaultSize ? 16 : 32; + wxImageList *ilist = new wxImageList( imageSize, imageSize ); + ilist->Add( wxIcon(wx_small_xpm) ); tc->AssignImageList( ilist ); @@ -1045,6 +1099,8 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l break; } + if ( HasModelFlag(MODEL_USE_TALL_ROWS) ) + m_ctrl[nPanel]->SetRowHeight(32); } @@ -1158,6 +1214,7 @@ void MyFrame::OnDecIndent(wxCommandEvent& WXUNUSED(event)) void MyFrame::OnPageChanged( wxBookCtrlEvent& WXUNUSED(event) ) { unsigned int nPanel = m_notebook->GetSelection(); + m_currentPanel = nPanel; GetMenuBar()->FindItem(ID_STYLE_MENU)->SetItemLabel( wxString::Format("Style of panel #%d", nPanel+1)); @@ -1189,6 +1246,49 @@ void MyFrame::OnPageChanged( wxBookCtrlEvent& WXUNUSED(event) ) GetMenuBar()->FindItem(id)->Check( m_ctrl[nPanel]->HasFlag(style) ); } + const int modelFlags = m_modelFlags[nPanel]; + + for (unsigned int id = ID_ALIGN_LEFT; id <= ID_ALIGN_BOTTOM; ++id) + { + int align = wxALIGN_NOT; + bool check = false; + switch (id) + { + case ID_ALIGN_LEFT: + check = !(modelFlags & (wxALIGN_CENTRE_HORIZONTAL | wxALIGN_RIGHT)); + break; + case ID_ALIGN_CENTRE_H: + align = wxALIGN_CENTRE_HORIZONTAL; + break; + case ID_ALIGN_RIGHT: + align = wxALIGN_RIGHT; + break; + case ID_ALIGN_TOP: + check = !(modelFlags & (wxALIGN_CENTRE_VERTICAL | wxALIGN_BOTTOM)); + break; + case ID_ALIGN_CENTRE_V: + align = wxALIGN_CENTRE_VERTICAL; + break; + case ID_ALIGN_BOTTOM: + align = wxALIGN_BOTTOM; + break; + default: + wxFAIL; + } + + if ( align != wxALIGN_NOT ) + check = (modelFlags & align) != 0; + + GetMenuBar()->FindItem(id)->Check(check); + } + + GetMenuBar()->FindItem(ID_TOGGLE_USE_TALL_ROWS)->Check( + HasModelFlag(MODEL_USE_TALL_ROWS)); + GetMenuBar()->FindItem(ID_TOGGLE_KEEP_LOGO_SMALL)->Check( + HasModelFlag(MODEL_KEEP_LOGO_SMALL)); + GetMenuBar()->FindItem(ID_TOGGLE_USE_MULTI_LINE_TEXT)->Check( + HasModelFlag(MODEL_USE_MULTI_LINE_TEXT)); + GetMenuBar()->FindItem(ID_DISABLE)->Check(!m_ctrl[nPanel]->IsEnabled()); } @@ -1223,8 +1323,26 @@ void MyFrame::OnStyleChange( wxCommandEvent& WXUNUSED(event) ) else if (nPanel == 4) m_long_music_model.reset(NULL); + int flags = 0; + if ( GetMenuBar()->FindItem(ID_ALIGN_CENTRE_H)->IsChecked() ) + flags |= wxALIGN_CENTRE_HORIZONTAL; + if ( GetMenuBar()->FindItem(ID_ALIGN_RIGHT)->IsChecked() ) + flags |= wxALIGN_RIGHT; + if ( GetMenuBar()->FindItem(ID_ALIGN_CENTRE_V)->IsChecked() ) + flags |= wxALIGN_CENTRE_VERTICAL; + if ( GetMenuBar()->FindItem(ID_ALIGN_BOTTOM)->IsChecked() ) + flags |= wxALIGN_BOTTOM; + + if ( GetMenuBar()->FindItem(ID_TOGGLE_USE_TALL_ROWS)->IsChecked() ) + flags |= MODEL_USE_TALL_ROWS; + if ( GetMenuBar()->FindItem(ID_TOGGLE_KEEP_LOGO_SMALL)->IsChecked() ) + flags |= MODEL_KEEP_LOGO_SMALL; + if ( GetMenuBar()->FindItem(ID_TOGGLE_USE_MULTI_LINE_TEXT)->IsChecked() ) + flags |= MODEL_USE_MULTI_LINE_TEXT; + // rebuild the DVC for the selected panel: - BuildDataViewCtrl((wxPanel*)m_notebook->GetPage(nPanel), nPanel, style); + BuildDataViewCtrl((wxPanel*)m_notebook->GetPage(nPanel), + nPanel, style, flags); sz->Prepend(m_ctrl[nPanel], 1, wxGROW|wxALL, 5); sz->Layout(); diff --git a/samples/dataview/mymodels.cpp b/samples/dataview/mymodels.cpp index 2a312d8a12..418de7d886 100644 --- a/samples/dataview/mymodels.cpp +++ b/samples/dataview/mymodels.cpp @@ -339,9 +339,12 @@ static int my_sort( int *v1, int *v2 ) #define INITIAL_NUMBER_OF_ITEMS 10000 -MyListModel::MyListModel() : +MyListModel::MyListModel(int modelFlags) : wxDataViewVirtualListModel( INITIAL_NUMBER_OF_ITEMS ) { + const wxString multiLineText = L"top (\u1ED6)\ncentre\nbottom (g)"; + const bool useMultiLine = (modelFlags & MODEL_USE_MULTI_LINE_TEXT) != 0; + // the first 100 items are really stored in this model; // all the others are synthesized on request static const unsigned NUMBER_REAL_ITEMS = 100; @@ -349,17 +352,32 @@ MyListModel::MyListModel() : m_toggleColValues.reserve(NUMBER_REAL_ITEMS); m_textColValues.reserve(NUMBER_REAL_ITEMS); m_toggleColValues.push_back(false); - m_textColValues.push_back("first row with long label to test ellipsization"); + m_textColValues.push_back(useMultiLine + ? multiLineText + : wxString("first row with long label to test ellipsization")); for (unsigned int i = 1; i < NUMBER_REAL_ITEMS; i++) { m_toggleColValues.push_back(false); m_textColValues.push_back(wxString::Format("real row %d", i)); } - m_iconColValues.assign(NUMBER_REAL_ITEMS, "test"); + m_iconColValues.assign(NUMBER_REAL_ITEMS, + useMultiLine ? multiLineText : wxString("test")); m_icon[0] = wxIcon( null_xpm ); - m_icon[1] = wxIcon( wx_small_xpm ); + + const int newSize = m_icon[0].GetWidth() * 2; + const bool useTallRows = (modelFlags & MODEL_USE_TALL_ROWS) != 0; + + if ( useTallRows ) + m_icon[0].CopyFromBitmap( + wxImage(null_xpm).Rescale(newSize, newSize)); + + if ( !useTallRows || (modelFlags & MODEL_KEEP_LOGO_SMALL) ) + m_icon[1] = wxIcon( wx_small_xpm ); + else + m_icon[1].CopyFromBitmap( + wxImage(wx_small_xpm).Rescale(newSize, newSize)); } void MyListModel::Prepend( const wxString &text ) diff --git a/samples/dataview/mymodels.h b/samples/dataview/mymodels.h index d2a1963448..a81c82e28c 100644 --- a/samples/dataview/mymodels.h +++ b/samples/dataview/mymodels.h @@ -216,7 +216,7 @@ public: Col_Max }; - MyListModel(); + MyListModel(int modelFlags); // helper methods to change the model @@ -310,3 +310,10 @@ private: wxDECLARE_NO_COPY_CLASS(MyIndexListModel); }; + +enum ModelFlags +{ + MODEL_USE_TALL_ROWS = 1 << 0, + MODEL_KEEP_LOGO_SMALL = 1 << 1, + MODEL_USE_MULTI_LINE_TEXT = 1 << 2 +}; From 8263e4b366a432af1485e62bd91495b75d30c7a6 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 12 Feb 2021 21:44:32 +0100 Subject: [PATCH 03/11] Avoid assert failures in render sample with generic renderer Some generic render functions aren't implemented and result in obtrusive assert failures when switching to use the generic renderer in the render sample. Instead show text saying the drawing function is not implemented in the location where the sample would ordinarily draw the control. --- samples/render/render.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/samples/render/render.cpp b/samples/render/render.cpp index f6589271f6..2c6b8bedab 100644 --- a/samples/render/render.cpp +++ b/samples/render/render.cpp @@ -288,9 +288,15 @@ private: wxRect(wxPoint(x2, y), sizeMark), m_flags); y += lineHeight + sizeMark.y; + const wxString notImplementedText = "(generic version unimplemented)"; + dc.DrawText("DrawRadioBitmap()", x1, y); - renderer.DrawRadioBitmap(this, dc, - wxRect(wxPoint(x2, y), sizeCheck), m_flags); + if ( m_useGeneric ) + dc.DrawText(notImplementedText, x2, y); + else + renderer.DrawRadioBitmap(this, dc, + wxRect(wxPoint(x2, y), sizeCheck), m_flags); + y += lineHeight + sizeCheck.y; dc.DrawText("DrawCollapseButton()", x1, y); @@ -354,8 +360,11 @@ private: y += lineHeight; dc.DrawText("DrawChoice()", x1, y); - renderer.DrawChoice(this, dc, - wxRect(x2, y, width, 1.5*GetCharHeight()), m_flags); + if ( m_useGeneric ) + dc.DrawText(notImplementedText, x2, y); + else + renderer.DrawChoice(this, dc, + wxRect(x2, y, width, 1.5*GetCharHeight()), m_flags); } int m_flags; From d654f30d1458049637da950ddd700bf84ad07048 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Fri, 12 Feb 2021 22:27:48 +0100 Subject: [PATCH 04/11] Demonstrate wxRenderer::DrawItemText() alignment in render sample This is intended for wxMSW to show differences between themed text drawing (with wxRendererXP::DrawItemText() under Vista+) and wxRendererGeneric::DrawItemText(). Use existing option to switch between generic and native renderer (Ctrl-G) in the render sample to observe the differences. --- samples/render/render.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/samples/render/render.cpp b/samples/render/render.cpp index 2c6b8bedab..6bae34781b 100644 --- a/samples/render/render.cpp +++ b/samples/render/render.cpp @@ -246,6 +246,26 @@ private: dc.DrawText("Using flags: " + flagsString, x1, y); y += lineHeight*3; + const wxCoord heightListItem = FromDIP(48); + const wxCoord widthListItem = 30*GetCharWidth(); + + { + + dc.DrawText("DrawItemText() alignment", x1, y); + + wxRect textRect(x2, y, widthListItem, heightListItem); + wxDCBrushChanger setBrush(dc, *wxTRANSPARENT_BRUSH); + wxDCPenChanger setPen(dc, *wxGREEN_PEN); + dc.DrawRectangle(textRect); + + renderer.DrawItemText(this, dc, L"Top Left (\u1ED6)", textRect); + renderer.DrawItemText(this, dc, "Bottom right", textRect, + wxALIGN_BOTTOM | wxALIGN_RIGHT); + + y += lineHeight + heightListItem; + + } + const wxCoord heightHdr = renderer.GetHeaderButtonHeight(this); const wxCoord width = 15*GetCharWidth(); @@ -346,9 +366,6 @@ private: y += lineHeight + heightGauge; - const wxCoord heightListItem = FromDIP(48); - const wxCoord widthListItem = 30*GetCharWidth(); - dc.DrawText("DrawItemSelectionRect()", x1, y); renderer.DrawItemSelectionRect(this, dc, wxRect(x2, y, widthListItem, heightListItem), m_flags | wxCONTROL_SELECTED); From 1eb71503098867c7e2f2cf0c9c1394fc2c935bec Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Mon, 2 Aug 2021 23:48:18 +0200 Subject: [PATCH 05/11] Correct slightly misleading comment Comment in wxRendererXP::DrawItemText() could be interpreted as DrawThemeTextEx() being available for some XP editions but it is Vista+ only, so correct it. --- src/msw/renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msw/renderer.cpp b/src/msw/renderer.cpp index df0259409d..ab80ff8cc4 100644 --- a/src/msw/renderer.cpp +++ b/src/msw/renderer.cpp @@ -1057,7 +1057,7 @@ void wxRendererXP::DrawItemText(wxWindow* win, s_initDone = true; } - if ( s_DrawThemeTextEx && // Might be not available if we're under XP + if ( s_DrawThemeTextEx && // Not available under XP ::IsThemePartDefined(hTheme, LVP_LISTITEM, 0) ) { RECT rc = ConvertToRECT(dc, rect); From b8af267bf93364b5083fbd3037ad07467ccb82a0 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 13 Feb 2021 00:51:33 +0100 Subject: [PATCH 06/11] Fix centered vertical and bottom aligned themed text drawing When using non-TOP vertical alignment with wxRendererXP::DrawItemText() either DT_VCENTER or DT_BOTTOM gets passed to DrawThemeTextEx() but without the DT_SINGLELINE flag which is required for those alignment flags to take effect, resulting in text always being top aligned. Fix by passing DT_SINGLELINE when using either alignment, but only for single lines as multi-lines are rendered as a single line with invisible newline character. Draw multi-line text using top alignment and deal with vertical alignment ourselves, using GetThemeTextExtent() to get an accurate extent of the text. --- src/msw/renderer.cpp | 110 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 103 insertions(+), 7 deletions(-) diff --git a/src/msw/renderer.cpp b/src/msw/renderer.cpp index ab80ff8cc4..74a3cb26e4 100644 --- a/src/msw/renderer.cpp +++ b/src/msw/renderer.cpp @@ -1043,7 +1043,10 @@ void wxRendererXP::DrawItemText(wxWindow* win, const int itemState = GetListItemState(flags); typedef HRESULT(__stdcall *DrawThemeTextEx_t)(HTHEME, HDC, int, int, const wchar_t *, int, DWORD, RECT *, const WXDTTOPTS *); + typedef HRESULT(__stdcall *GetThemeTextExtent_t)(HTHEME, HDC, int, int, const wchar_t *, int, DWORD, RECT *, RECT *); + static DrawThemeTextEx_t s_DrawThemeTextEx = NULL; + static GetThemeTextExtent_t s_GetThemeTextExtent = NULL; static bool s_initDone = false; if ( !s_initDone ) @@ -1052,6 +1055,7 @@ void wxRendererXP::DrawItemText(wxWindow* win, { wxLoadedDLL dllUxTheme(wxS("uxtheme.dll")); wxDL_INIT_FUNC(s_, DrawThemeTextEx, dllUxTheme); + wxDL_INIT_FUNC(s_, GetThemeTextExtent, dllUxTheme); } s_initDone = true; @@ -1082,7 +1086,10 @@ void wxRendererXP::DrawItemText(wxWindow* win, textOpts.crText = textColour.GetPixel(); } - DWORD textFlags = DT_NOPREFIX; + const DWORD defTextFlags = DT_NOPREFIX; + DWORD textFlags = defTextFlags; + + // Always use DT_* flags for horizontal alignment. if ( align & wxALIGN_CENTER_HORIZONTAL ) textFlags |= DT_CENTER; else if ( align & wxALIGN_RIGHT ) @@ -1093,12 +1100,101 @@ void wxRendererXP::DrawItemText(wxWindow* win, else textFlags |= DT_LEFT; - if ( align & wxALIGN_BOTTOM ) - textFlags |= DT_BOTTOM; - else if ( align & wxALIGN_CENTER_VERTICAL ) - textFlags |= DT_VCENTER; - else - textFlags |= DT_TOP; + /* + Bottom (DT_BOTTOM) and centered vertical (DT_VCENTER) alignment + are documented to be only used with the DT_SINGLELINE flag which + doesn't handle multi-lines. In case of drawing multi-lines with + such alignment use DT_TOP (0), which does work for multi-lines, + and deal with the actual desired vertical alignment ourselves with + the help of GetThemeTextExtent(). + */ + bool useTopDrawing = + s_GetThemeTextExtent + && ( align & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL) ) != 0 + && text.Contains(wxS('\n')); + + if ( useTopDrawing ) + { + /* + Get the actual text extent using GetThemeTextExtent() and adjust + drawing rect if needed. + + Note that DrawThemeTextEx() in combination with DT_CALCRECT + and DTT_CALCRECT can also be used to get the text extent. + This seems to always result in the exact same extent (checked + with an assert) as using GetThemeTextExtent(), despite having + an additional WXDTTOPTS argument for various effects. + Some effects have been tried (DTT_BORDERSIZE, DTT_SHADOWTYPE + and DTT_SHADOWOFFSET) and while rendered correctly with effects + the returned extent remains the same as without effects. + + Official docs don't seem to prefer one method over the other + though a possibly outdated note for DrawThemeText() recommends + using GetThemeTextExtent(). Because Wine as of writing doesn't + support DT_CALCRECT with DrawThemeTextEx() while it does support + GetThemeTextExtent(), opt to use the latter. + */ + + /* + It's important for the dwTextFlags parameter passed to + GetThemeTextExtent() not to have some DT_* flags because they + influence the extent size in unwanted ways: Using + DT_SINGLELINE combined with either DT_VCENTER or DT_BOTTOM + results in a height that can't be used (either halved or 0), + and having DT_END_ELLIPSIS ends up always ellipsizing. + Passing a non-NULL rect solves these problems but is not + really a good option as it doesn't make the rectangle extent + a tight fit and calculations would have to be done with large + numbers needlessly (provided the passed rect is set to + something like {0, 0, LONG_MAX, LONG_MAX} ). + */ + RECT rcExtent; + HRESULT hr = s_GetThemeTextExtent(hTheme, dc.GetHDC(), + LVP_LISTITEM, itemState, text.wchar_str(), -1, + defTextFlags, NULL, &rcExtent); + if ( SUCCEEDED(hr) ) + { + /* + For height compare with the height of the passed rect and use + the difference for handling vertical alignment. This has + consequences for particularly multi-line text: it will now + always try to fit vertically while a rect received from wxDVC + may have its extent based on calculations for a single line + only and therefore couldn't show more than one line. + As a result of the expanded height clipping may be needed + which at least already happens with wxDVC which (out of + necessity) confines rendering to a cell's bounds. + */ + const int heightDiff = rect.GetHeight() - rcExtent.bottom; + if ( heightDiff ) + { + if ( align & wxALIGN_CENTRE_VERTICAL ) + { + const int heightOffset = heightDiff / 2; + rc.top += heightOffset; + rc.bottom -= heightOffset; + } + else if ( align & wxALIGN_BOTTOM ) + rc.top += heightDiff; + else // top aligned + rc.bottom -= heightDiff; + } + } + else + { + useTopDrawing = false; + } + } + + if ( !useTopDrawing ) + { + if ( align & wxALIGN_BOTTOM ) + textFlags |= DT_BOTTOM | DT_SINGLELINE; + else if ( align & wxALIGN_CENTER_VERTICAL ) + textFlags |= DT_VCENTER | DT_SINGLELINE; + else + textFlags |= DT_TOP; + } const wxString* drawText = &text; wxString ellipsizedText; From d83d1269597d2b9af04d2383e9b821a28649819a Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 13 Feb 2021 13:09:18 +0100 Subject: [PATCH 07/11] Fix off-by-ones with bottom and right aligned positioning wxDC::DrawLabel() positions the text one to the left with wxALIGN_RIGHT alignment and one up with wxALIGN_BOTTOM alignment. The same occurs in wxControlRenderer::DrawBitmap() for bitmaps. Both off-by-ones exist since inception in respectively 4d3c4c2f94 and bc60c3d699. This fix vertically aligns wxALIGN_BOTTOM drawing of wxDC::DrawLabel() with themed text drawing through wxRendererXP::DrawItemText(). --- src/common/dcbase.cpp | 4 ++-- src/univ/ctrlrend.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/dcbase.cpp b/src/common/dcbase.cpp index c669a028b9..fe93ee18bb 100644 --- a/src/common/dcbase.cpp +++ b/src/common/dcbase.cpp @@ -1196,7 +1196,7 @@ void wxDC::DrawLabel(const wxString& text, wxCoord x, y; if ( alignment & wxALIGN_RIGHT ) { - x = rect.GetRight() - width; + x = rect.GetRight() - width + 1; } else if ( alignment & wxALIGN_CENTRE_HORIZONTAL ) { @@ -1209,7 +1209,7 @@ void wxDC::DrawLabel(const wxString& text, if ( alignment & wxALIGN_BOTTOM ) { - y = rect.GetBottom() - height; + y = rect.GetBottom() - height + 1; } else if ( alignment & wxALIGN_CENTRE_VERTICAL ) { diff --git a/src/univ/ctrlrend.cpp b/src/univ/ctrlrend.cpp index 6039922384..70b27679fa 100644 --- a/src/univ/ctrlrend.cpp +++ b/src/univ/ctrlrend.cpp @@ -185,7 +185,7 @@ void wxControlRenderer::DrawBitmap(wxDC &dc, { if ( alignment & wxALIGN_RIGHT ) { - x = rect.GetRight() - width; + x = rect.GetRight() - width + 1; } else if ( alignment & wxALIGN_CENTRE ) { @@ -198,7 +198,7 @@ void wxControlRenderer::DrawBitmap(wxDC &dc, if ( alignment & wxALIGN_BOTTOM ) { - y = rect.GetBottom() - height; + y = rect.GetBottom() - height + 1; } else if ( alignment & wxALIGN_CENTRE_VERTICAL ) { From 7a8b210dffca8df1014c7d72be725d2e6ca41718 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 13 Feb 2021 13:28:37 +0100 Subject: [PATCH 08/11] Undo mimicking faulty wxDC::DrawLabel() wxALIGN_RIGHT positioning Revert b642747fd2 (Fix right aligned text position in wxDVC on MSW with system theme, 2015-09-30) which in wxRendererXP::DrawItemText() mimicks the off-by-one with right alignment of wxDC::DrawLabel() that got fixed by the previous commit. There's not a similar change copying wxDC::DrawLabel()'s former off-by-one behaviour with wxALIGN_BOTTOM that would need reverting. Both alignments now result in the same text positioning with both drawing functions. --- src/msw/renderer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/msw/renderer.cpp b/src/msw/renderer.cpp index 74a3cb26e4..1e6e4c6c62 100644 --- a/src/msw/renderer.cpp +++ b/src/msw/renderer.cpp @@ -1093,10 +1093,7 @@ void wxRendererXP::DrawItemText(wxWindow* win, if ( align & wxALIGN_CENTER_HORIZONTAL ) textFlags |= DT_CENTER; else if ( align & wxALIGN_RIGHT ) - { textFlags |= DT_RIGHT; - rc.right--; // Alignment is inconsistent with DrawLabel otherwise - } else textFlags |= DT_LEFT; From 9dd88e0159901c549e037bc0e8b75b79fb90cbe4 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 13 Feb 2021 13:56:54 +0100 Subject: [PATCH 09/11] Undo removal of text alignment for wxDataViewCtrl's RenderText() Revert e2e7d3d391 (Fix wxELLIPSIZE_END with wxALIGN_RIGHT in wxMSW wxDataViewCtrl, 2016-03-18). This fix is no longer needed after the previous commit reverted b642747fd2. Reverting also allows for text to be drawn vertically aligned again instead of always using top-left alignment. While the difference in appearance by not having alignment can be minor with default row heights, it becomes more noticeable with taller rows: a wxDataViewCheckIconText column with a tall icon will have its text stuck to the top of a row while other columns have their text vertically centered. This already occurs by default when not explicitly specifying an alignment (wxDVR_DEFAULT_ALIGNMENT) which results in wxALIGN_CENTRE_VERTICAL being used for row alignment when rendering. --- src/common/datavcmn.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 715bfc08cc..7d25c8fe2b 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -1090,17 +1090,12 @@ wxDataViewCustomRendererBase::RenderText(const wxString& text, if ( !(GetOwner()->GetOwner()->IsEnabled() && GetEnabled()) ) flags |= wxCONTROL_DISABLED; - // Notice that we intentionally don't use any alignment here: it is not - // necessary because the cell rectangle had been already adjusted to - // account for the alignment in WXCallRender() and using the alignment here - // results in problems with ellipsization when using native MSW renderer, - // see https://trac.wxwidgets.org/ticket/17363, so just don't do it. wxRendererNative::Get().DrawItemText( GetOwner()->GetOwner(), *dc, text, rectText, - wxALIGN_NOT, + GetEffectiveAlignment(), flags, GetEllipsizeMode()); } From ebf1141db25b47307d87ab4616bf2f0941bbcfd5 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Sat, 13 Feb 2021 15:05:16 +0100 Subject: [PATCH 10/11] Improve text extent rounding in wxGCDCImpl::DoGetTextExtent() Change wxGCDCImpl::DoGetTextExtent() from rounding to the nearest integer to rounding up: if e.g. height is 15.3 then 16 pixels should be used for height, and not 15. Rounding was previously improved from casting (which appears to be the initial code) in 0417955ddb (adding correct filling area to arc, correct rounding and clipping, 2007-10-21). Issues with nearest rounding became more visible after off-by-one fixes for wxDC::DrawLabel() with wxALIGN_RIGHT and wxALIGN_BOTTOM positioning. When using e.g. wxALIGN_RIGHT with a width of 72.4 the text could now be drawn beyond the extent on the right because the text is not offset by one to the left any longer. --- src/common/dcgraph.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/dcgraph.cpp b/src/common/dcgraph.cpp index 4427c260a1..3936df0fff 100644 --- a/src/common/dcgraph.cpp +++ b/src/common/dcgraph.cpp @@ -1279,13 +1279,13 @@ void wxGCDCImpl::DoGetTextExtent( const wxString &str, wxCoord *width, wxCoord * ); if ( height ) - *height = (wxCoord)wxRound(h); + *height = (wxCoord)ceil(h); if ( descent ) - *descent = (wxCoord)wxRound(d); + *descent = (wxCoord)ceil(d); if ( externalLeading ) - *externalLeading = (wxCoord)wxRound(e); + *externalLeading = (wxCoord)ceil(e); if ( width ) - *width = (wxCoord)wxRound(w); + *width = (wxCoord)ceil(w); if ( theFont ) { From 90ba137f2059355f9dd85de932653dd6a4e317e6 Mon Sep 17 00:00:00 2001 From: Dimitri Schoolwerth Date: Mon, 26 Jul 2021 21:39:07 +0200 Subject: [PATCH 11/11] Work around text extent differences resulting in clipped text Under at least some versions of Windows 10 with wxDVC themed text can be clipped horizontally because of discrepancies between the text extent as drawn by DrawThemeTextEx() and calculated with GetTextExtent() earlier. Work around the issue by always trying to use GetThemeTextExtent() in DrawItemText(), not just for alignment of multi-line strings, and adjust for any width differences similar to the existing adjustment for height. See #18487. --- src/msw/renderer.cpp | 49 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/msw/renderer.cpp b/src/msw/renderer.cpp index 1e6e4c6c62..b85629e31f 100644 --- a/src/msw/renderer.cpp +++ b/src/msw/renderer.cpp @@ -1104,13 +1104,16 @@ void wxRendererXP::DrawItemText(wxWindow* win, such alignment use DT_TOP (0), which does work for multi-lines, and deal with the actual desired vertical alignment ourselves with the help of GetThemeTextExtent(). - */ - bool useTopDrawing = - s_GetThemeTextExtent - && ( align & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL) ) != 0 - && text.Contains(wxS('\n')); - if ( useTopDrawing ) + [TODO] Ideally text measurement should only be needed for the above + mentioned situations but because there can be a difference between + the extent from GetThemeTextExtent() and the rect received by this + function could have involved other text measurements (e.g. with wxDVC, + see #18487), use it in all cases for now. + */ + bool useTopDrawing = false; + + if ( s_GetThemeTextExtent != NULL ) { /* Get the actual text extent using GetThemeTextExtent() and adjust @@ -1151,6 +1154,30 @@ void wxRendererXP::DrawItemText(wxWindow* win, defTextFlags, NULL, &rcExtent); if ( SUCCEEDED(hr) ) { + /* + Compensate for rare cases where the horizontal extents differ + slightly. Don't use the width of the passed rect here to deal + with horizontal alignment as it results in the text always + fitting and ellipsization then can't occur. Instead check for + width differences by comparing with the extent as calculated + by wxDC. + */ + const int textWidthDc = dc.GetMultiLineTextExtent(text).x; + const int widthDiff = textWidthDc - rcExtent.right; + if ( widthDiff ) + { + if ( align & wxALIGN_CENTRE_HORIZONTAL ) + { + const int widthOffset = widthDiff / 2; + rc.left += widthOffset; + rc.right -= widthOffset; + } + else if ( align & wxALIGN_RIGHT ) + rc.left += widthDiff; + else // left aligned + rc.right -= widthDiff; + } + /* For height compare with the height of the passed rect and use the difference for handling vertical alignment. This has @@ -1163,8 +1190,12 @@ void wxRendererXP::DrawItemText(wxWindow* win, necessity) confines rendering to a cell's bounds. */ const int heightDiff = rect.GetHeight() - rcExtent.bottom; - if ( heightDiff ) + if ( heightDiff + && (align & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) + && text.Contains(wxS('\n')) ) { + useTopDrawing = true; + if ( align & wxALIGN_CENTRE_VERTICAL ) { const int heightOffset = heightDiff / 2; @@ -1177,10 +1208,6 @@ void wxRendererXP::DrawItemText(wxWindow* win, rc.bottom -= heightDiff; } } - else - { - useTopDrawing = false; - } } if ( !useTopDrawing )