wxRichTextCtrl Selected Text Shifts (Issue #22771)
lsri2
Describe the bug
When you are selecting text in the control, you can see other text along the line "shift".
Expected vs observed behaviour
The text shouldn't shift.
Patch or snippet allowing to reproduce the problem
Using the wxRichTextCtrl sample, start selecting the line beginning with "What can you do with this thing?". After you select the W, you will see other text along that line shift.
Platform and version information
- wxWidgets version you use: 3.2.0
- wxWidgets port you use: wxMSW
- OS and its version: Windows 7 Pro
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
I cannot reproduce this on native Win10 or Win7 under VirtualBox.
However, when I run the sample and move it from the primary screen at 125% DPI to the secondary one with the standard DPI and attempt to select the sentence, selecting behaves erratically, selecting other parts than those dragged over by mouse. Sometimes it does work though.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Cătălin Răceanu
IIRC (from what was an old discussion on the same subject which nor I nor google can find) selected text is drawn separately from the rest of text. For partial word selection, it behaves as if there are 2 words next to each other, which renders differently from a single big word. A solution might be to change all that so that the background with the selected color was drawn before/under the text, rather than drawing the selected text separately from the unselected one.
IIRC this required significant changes to text rendering.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)Also experiencing this on wxWidgets 3.3.1 on macOS Ventura. It's not a big deal, but is a little unpolished.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#22771)Yes, this looks bad and should definitely be fixed, but unfortunately this library doesn't have an active maintainer and I don't know it enough and don't use it myself, so it's hard to find the motivation to spend a lot of time on doing it. Of course, if somebody could contribute a fix for this, it would be great.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
lsri2
lsri2 left a comment (wxWidgets/wxWidgets#22771)Hi, guys, yeah I wasn't sure of a fix. What I did that seems to help it a bit was comment out this code in msw\dc.cpp DoGetTextExtent(). Not a real fix, but I just wanted to share.
/* const wxFont font2 = GetFont();
if ( font2.IsOk() && font2.GetStyle() != wxFONTSTYLE_NORMAL && len > 0 )
{
ABC width;
const wxChar chFirst = *string.begin();
if ( ::GetCharABCWidths(GetHdc(), chFirst, chFirst, &width) )
{
if ( width.abcA < 0 )
sizeRect.cx -= width.abcA;
if ( len > 1 )
{
const wxChar chLast = *string.rbegin();
::GetCharABCWidths(GetHdc(), chLast, chLast, &width);
}
//else: we already have the width of the last character
if ( width.abcC < 0 )
sizeRect.cx -= width.abcC;
}
//else: GetCharABCWidths() failed, not a TrueType font?
}*/
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)I've spent a fair amount of time this week digging into this.
Inside wxRichTextPlainText::Draw(), a line of text in which there is a partial selection—such as the line in which you're currently selecting text—is drawn in three segments via wxRichTextPlainText::DrawTabbedString(), which in turn calls wxDC::DrawText().
On canvasses with aggressive kerning and subpixel antialiasing, successive calls to DrawText() do not look the same, because font shaping is part of each call; it isn't cumulative.
This is why this jitter happens on all macOS (CoreGraphics) backends; happens on Windows with GDI+ backends but not GDI backends; and doesn't happen at all on Linux/GTK. The CoreGraphics and GDI+ implementations of wxDC appear to engage in heavy font hinting, meaning that DrawText("ABC"); appears different from DrawText("A"); DrawText("BC"); appears different from DrawText("AB"); DrawText("C");
There are basically three ways to fix this:
-
Try to disable font hinting at the platform implementation level. This is what @lsri2 was getting close to. However, lots of things use wxDC besides wxRichTextCtrl, so I'm hesitant to do that.
-
Brute-force disable of font hinting by drawing each character in its own DrawText() call. This might be tedious to code, especially taking into account some special handling of tabs. It would also likely look ugly and disable all font hinting. And I have no idea what the performance impact would be (i.e., what the overhead of each DrawText() call is aside from the actual rendering).
-
Print the entire line at once, all the time, even in partial selections. Draw the selection box independently of the text, and don't use font selection-color or selection-background in the DrawText() call, only when drawing the selection box. This has the additional advantage of significantly simplifying the code. It does require a minor API change to DrawTabbedString(), but it's a private method and it can be made backward compatible by providing default arguments to the new parameters.
I did a little proof of concept of approach number 3, and it seemed to work okay.
https://github.com/user-attachments/assets/38c9b4f4-f667-43a5-96d0-fd97cfbbd802
The code is the result of me messing around and trying different approaches, and littered with fprintfs to stderr so I could figure out what I'm doing. It is a total mess. I'll need to clean up the code significantly, and test on Windows and Linux, before I consider a 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 left a comment (wxWidgets/wxWidgets#22771)Thanks for looking into this!
I think (1) could be a simple solution if we provided functions for disabling it in wxDC: wxRTC could then call it to explicitly disable the font hinting without affecting anything else. But I agree that (3) is better, it's just that this requires more changes to wxRTC — but if you can make them, please do, TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)@lsri2 Can you help me understand the exact circumstances you're seeing the same behavior on Windows? I've tried on Win7, Win10, and Win11 in a plain EXE. I'm going to try with various manifest options too (maybe it only shows up with GDI scaling or self-attested dpi awareness?). But if you can help me narrow it down, that would be great.
The fix does make some slight compromises, so I think I'm only going to enable it on affected platforms. But to do that, I need to know what platforms are affected (and under what circumstances).
Thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)I can't reproduce on Windows 7, unfortunately.
The workaround works best for Mac because it doesn't change the text color. Mac by default doesn't change the foreground color for the selected text, it only has a selection rectangle, so drawing all the text at once works well on that platform. A similar workaround on Windows would be more complicated.
Because the workaround works well on Mac but not on Windows, and because the problem is reproducible on Mac but not on Windows, I will implement the fix for Mac only.
PR should be inbound later tonight or tomorrow.
https://github.com/user-attachments/assets/c19ec5fb-32d6-4c4c-9600-c8ccc5a890fd
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)Not ready for a PR yet, but making good progress.
There are still some slight edge cases where it doesn't match the original behavior.
In particular, when a chunk of text is only partially selected, I don't change the foreground color for the selected portion (to avoid the multiple DrawText calls that lead to kerning issues). However, when it's completely selected, it falls back to the original ::DrawTabbedString(), which DOES change the foreground color, leading to some inconsistency.
https://github.com/user-attachments/assets/721a37f0-f373-481d-98e3-f9c2a4db0fa1
I might end up splitting up DrawText calls anyway, but using the result of GetPartialTextExtents on the whole string to find the starting point for the section of text after the selection. This should lead to minimal jitter, right around the selection cursor. Will experiment more this week.
Patch of include/wx/richtext/richtextbuffer.h so far:
--- a/include/wx/richtext/richtextbuffer.h
+++ b/include/wx/richtext/richtextbuffer.h
@@ -4766,6 +4766,9 @@ public:
private:
bool DrawTabbedString(wxDC& dc, const wxRichTextAttr& attr, const wxRect& rect, wxString& str, wxCoord& x, wxCoord& y, bool selected);
+#ifdef __WXMAC__
+ bool DrawTabbedStringWithPartialSelection(wxDC& dc, const wxRichTextAttr& attr, const wxRect& rect, wxString& str, wxCoord& x, wxCoord& y, int selStart, int selEnd);
+#endif
protected:
wxString m_text;
Patch of src/richtext/richtextbuffer.cpp so far:
--- a/src/richtext/richtextbuffer.cpp
+++ b/src/richtext/richtextbuffer.cpp
@@ -6949,6 +6949,11 @@ bool wxRichTextPlainText::Draw(wxDC& dc, wxRichTextDrawingContext& context, cons
}
else
{
+#ifdef __WXMAC__
+ int selStart = wxMax(selectionRange.GetStart(), range.GetStart()) - range.GetStart();
+ int selEnd = wxMin(selectionRange.GetEnd(), range.GetEnd()) - range.GetStart();
+ DrawTabbedStringWithPartialSelection(dc, textAttr, rect, stringChunk, x, y, selStart, selEnd);
+#else
// (c) Part selected, part not
// Let's draw unselected chunk, selected chunk, then unselected chunk.
@@ -7036,10 +7041,142 @@ bool wxRichTextPlainText::Draw(wxDC& dc, wxRichTextDrawingContext& context, cons
DrawTabbedString(dc, textAttr, rect, stringFragment, x, y, false);
}
+#endif
}
return true;
}
+#ifdef __WXMAC__
+bool wxRichTextPlainText::DrawTabbedStringWithPartialSelection(wxDC& dc, const wxRichTextAttr& attr, const wxRect& rect, wxString& str, wxCoord& x, wxCoord& y, int selStart, int selEnd)
+{
+ ///////////////////////////////////////////////////////////////////////////////////////
+ // This function assumes str has uniform formatting (attr applies to entire string). //
+ // The calling code should split text into style runs before calling this function. //
+ ///////////////////////////////////////////////////////////////////////////////////////
+ wxColour highlightColour(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT));
+ // wxColour highlightTextColour(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHTTEXT));
+ wxCoord originx = x;
+ wxArrayInt startpos; // Character starting positions
+ wxArrayInt endpos; // Character ending positions (cumulative widths)
+ wxArrayInt tabstops; // Array of tab stop positions
+ wxPen savedpen;
+
+
+ int strikethrough = (attr.HasTextEffects() && (attr.GetTextEffects() & wxTEXT_ATTR_EFFECT_STRIKETHROUGH)) ? 1 : 0;
+ int strikeheight = (int) (rect.y + rect.GetHeight()/2.0 + 0.5); // Cast truncates; +0.5 forces rounding
+ int tabcount;
+ int effectivetabstop;
+ int posadjustment;
+ int printstart;
+ int i = 0; // initialization iterator
+ int si = 0; // string iterator
+ int ti = 0; // tabstop iterator
+ int pi = 0; // position iterator
+
+
+ ///////////////////////////////////////////////////////////////////////////////////////
+ // Initial setup //
+ ///////////////////////////////////////////////////////////////////////////////////////
+
+ // Get character ending positions
+ dc.GetPartialTextExtents(str, endpos);
+ startpos.SetCount(endpos.GetCount());
+
+ startpos[0] = originx;
+ for (i = 0; i < endpos.GetCount() - 1; i++) {
+ endpos[i] = originx + endpos[i];
+ startpos[i+1] = endpos[i];
+ }
+ endpos[i] = originx + endpos[i];
+
+ // Get all of the tabstops
+ tabstops = (attr.GetTabs().IsEmpty()) ? wxRichTextParagraph::GetDefaultTabs() : attr.GetTabs();
+ tabcount = tabstops.GetCount();
+ for (i = 0; i < tabcount; i++)
+ tabstops[i] = originx + ConvertTenthsMMToPixels(dc, tabstops[i]);
+
+
+ ///////////////////////////////////////////////////////////////////////////////////////
+ // Tab stop position adjustments //
+ ///////////////////////////////////////////////////////////////////////////////////////
+
+ // Adjust horizontal positions to account for tab stops.
+ // Do this by looping through to find every tab character, calculate the appropriate tab stop for it, measure
+ // the horizontal space difference, and adjust the tab's ending position and the start/end positions of every
+ // character after it.
+ //
+ // TL;DR: Tab Char --> Tab Stop --> Pixel Adjustment --> New startpos/endpos
+ //
+ // NOTE: TODO: This almost certainly does not work for RTL languages unless wxDC has some real x-flipping magic.
+ for (si = 0; si < str.Len(); si++) {
+
+ if (str[si] == '\t') {
+ // We have a tab. Find the next tab stop AFTER this tab's start position.
+ while (ti < tabcount && tabstops[ti] <= startpos[si]) ti++;
+
+ if (ti < tabcount) {
+ // We have a valid tabstop in tabstops[ti] that is past startpos[si]
+ effectivetabstop = tabstops[ti];
+ } else {
+ // No more tabstops defined; use default tab width
+ effectivetabstop = startpos[si] + ConvertTenthsMMToPixels(dc, WIDTH_FOR_DEFAULT_TABS);
+ }
+
+ // Adjust position calculations accordingly
+ posadjustment = effectivetabstop - endpos[si];
+ endpos[si] += posadjustment;
+ for (pi = si + 1; pi < endpos.GetCount(); pi++) {
+ startpos[pi] += posadjustment;
+ endpos[pi] += posadjustment;
+ }
+ }
+ }
+
+ ///////////////////////////////////////////////////////////////////////////////////////
+ // Draw selection rectangle //
+ ///////////////////////////////////////////////////////////////////////////////////////
+ if (selStart >=0 && selStart < endpos.GetCount()) {
+ if (selEnd >= selStart && selEnd < endpos.GetCount()) {
+ int selectionWidth = endpos[selEnd] - startpos[selStart] + 1;
+ savedpen = dc.GetPen();
+ wxCheckSetBrush(dc, wxBrush(highlightColour));
+ wxCheckSetPen(dc, wxPen(highlightColour));
+ wxRect selRect(startpos[selStart], rect.y, selectionWidth, rect.GetHeight());
+ dc.DrawRectangle(selRect);
+ wxCheckSetPen(dc, savedpen);
+ }
+ }
+
+ ///////////////////////////////////////////////////////////////////////////////////////
+ // Draw text (and strikethrough) //
+ ///////////////////////////////////////////////////////////////////////////////////////
+ wxCheckSetPen(dc, wxPen(attr.GetTextColour(), 1));
+ dc.SetTextForeground(attr.GetTextColour());
+ dc.SetBackgroundMode(wxBRUSHSTYLE_TRANSPARENT);
+ printstart = 0;
+ for (si = 0; si < str.Len(); si++) {
+ if (str[si] == '\t') {
+ if (si - printstart > 0) {
+ dc.DrawText(str.Mid(printstart, si-printstart), startpos[printstart], y);
+ if (strikethrough) {
+ if (selStart <= si && si <= selEnd)
+ dc.DrawLine(startpos[printstart], strikeheight, endpos[si], strikeheight);
+ else
+ dc.DrawLine(startpos[printstart], strikeheight, startpos[si], strikeheight);
+ }
+ }
+ printstart = si + 1;
+ }
+ }
+ if (printstart < str.Len()) {
+ dc.DrawText(str.Mid(printstart), startpos[printstart], y);
+ if (strikethrough) dc.DrawLine(startpos[printstart], strikeheight, endpos[endpos.GetCount()-1], strikeheight);
+ }
+
+ return true;
+}
+#endif
bool wxRichTextPlainText::DrawTabbedString(wxDC& dc, const wxRichTextAttr& attr, const wxRect& rect,wxString& str, wxCoord& x, wxCoord& y, bool selected)
{
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Joshua Lee Ockert
torstenvl left a comment (wxWidgets/wxWidgets#22771)Due to the refactor that @vadz asked me to do to keep the #ifdefs to a minimum, all the selection drawing code now uses wxDC::GetPartialTextExtents() on the whole string to position the subparts when partially selected, rather than checking for differences in various wxDC::GetTextExtent() calls.
According to the development roadmap, it was the theory at the time that this would fix or ameliorate the issue on all platforms.
As the selection is expanded, the text jumps slightly due to kerning differences between drawing a single text string versus drawing several fragments separately. This could be improved by using wxDC::GetPartialTextExtents to calculate exactly where the separate fragments should be drawn.
Given that (1) We have been unable to reproduce on Windows; and (2) commit c969634 definitively fixes the issue on Mac; and (3) the same commit takes an approach that was theorized to fix the issue... I wonder if we should close this issue for now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.