Refactor code so selection highlighting does not depend on platform supporting inverted blit mode.

[dmalec]

Summary

This is one possible approach for handling text selection highlighting. Instead of using wxINVERT as a post-process step, it handles selection highlighting by setting the foreground and background when doing the initial draw of the characters. This brings it in line with the new approach for STANDOUT. This looks like it should work on OSX, Windows, and Linux.

Color Selection

One point that might particularly be worth discussing further - selection of colors for highlighting. It looks like it's possible to get the system preferred highlight colors using:

    wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHTTEXT);
    wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT);

This does carry the risk that the system background doesn't contrast with the current terminal background. This particularly caught me off guard on Ubuntu when I put the system in high contrast mode and found that the system's selection colors were identical to the terminal's default ones... so there's was no indication of what was selected. @brianharvey had touched on the point about contrast between the various colors in the discussion about STANDOUT.

This approach proposes trusting that the system will have adequate contrast between the two colors for highlighting; but, that we might have to dynamically decide which is foreground and background based on contrast with the current terminal background. It also treats selection highlighting as distinct from STANDOUT. This is just one approach I thought I'd put out there - I think there are other valid ways we could decide on the colors for this.

Examples from OSX Build

Default:

With the background at a light blue (demonstrating the swap in system colors when highlighting):

System setting for highlighting set to orange:

Test Environments

• OSX Catalina (10.15.7) w/ wxWidgets 3.0.5
• Ubuntu Bionic (18.04.5) w/ wxWidgets 3.0.5

• Windows 10 Home (1909) w/ wxWidgets 3.0.5

[jrincayc]

This sounds and looks reasonable (I'll review it later this week.)

Thanks.

[jrincayc]

I think UpdateNormalizedTextSelection could be simplified, otherwise it looks fine.

[jrincayc]

Hm, shouldn't this be something like:

if (m_selx1 > m_selx2) {
    m_normalized_sel_x1 = m_selx2;
    m_normalized_sel_x2 = m_selx1;
} else {
    m_normalized_sel_x1 = m_selx1;
    m_normalized_sel_x2 = m_selx2;
}
if (m_sely1 > m_sely2) {
    m_normalized_sel_y1 = m_sely2;
    m_normalized_sel_y2 = m_sely1;
 } else {
    m_normalized_sel_y1 = m_sely1;
    m_normalized_sel_y2 = m_sely2;
}

[dmalec]

That specific approach won't quite work as it's not actually normalizing the rectangle (this was part of the original code that took me a bit to grok the mental model). If the selection is a single line, it does need to normalize the x values and things work reasonably intuitively. However, it gets interesting in multi-line.

For multi-line, the code will highlight:

• from m_selx1 to the end of the first line
• the entirety of all middle lines
• from the beginning of the last line to m_selx2

So, for the multi-line case, m_selx_1 and m_selx_2 are independent of each other and dependent on the y values.

All that said, I could add additional commenting (or see if I can rework the variable names / code a bit more) in order to make this clearer. It's something it definitely took me a while to puzzle out when reading the original code as I was thinking along the same lines about normalizing the rectangle :)

[jrincayc]

Ah, in that case can you update the comment? (I made the mistake of assuming the comment "selection coordinates to be from top-left to bottom-right" was just for a box.) Thanks.

[dmalec]

Ah, good call out there - I suspect the method comment particularly dates from my early understanding and I forgot to update it as my understanding changed, mea culpa. Will do :)

[dmalec]

I've updated the comments to try and better capture what's going on in the method - that said, game to revise further if needed.

[jrincayc]

(Also, I apologize for not getting to it in the week that I said I would.)

[dmalec]

No worries on timing :)

[jrincayc]

Reviewed and tested, and I approve.

[jrincayc]

Thank you for making selection work :)

[dmalec]

You're welcome, no worries :)