Custom syntax highlighting in STC w/ background thread Topic is solved

[TobiasA]

Good morning everybody,

after some small difficulties, I finally managed to write a custom syntax highlighting for CNC G-Code (Syntax roughly similar to basic, including positions in coordinates and stuff).
I did ask a few questions here: viewtopic.php?f=1&t=43800 which did help a lot.
The parser is working fine.
The main problem I face is that I run into serious performance problems. Parsing for G-Godes is rather fast, but searching ~900 keywords via RegEx is a pain performance-wise. And since STC will wait for the event to being finished, I have to parse the whole document each time a character is added
I did try:

Code: Select all

uint64_t startpos=GUIFrame::m_EditorSTC->GetEndStyled();
uint64_t endpos=startpos+event.GetLength();
this->highlightSTCsyntax(startpos,startpos+endpos);

but still, the performance isn't sufficient.
Anyway, my text files aren't this large (usually below 20k) so I could just parse in a background thread and highlight later on.
But: If I do this, some data race seems to occur, leading to a segmentation fault somewhere deep in wxwidgets.
I also thought about parsing the G-Codes first (similar to a scintilla lexer, pretty fast, only going through the document once) and then parsing all those keywords in a background thread, adding the style later- which fails for the same reason.

Is there any thread-safe way to update the styling of a STC with a background thread (including extracting the actual text out of the STC with getText())?
I have some deep fear that this might actually end in writing a custom scintilla lexer... Any chance I can do so without recompiling wxwidgets?

Thank you very much!

[TobiasA]

I found out the following code was the cause for the segmentation fault:

Code: Select all

GUIFrame::m_EditorSTC->StyleSetForeground(21,m_Ncolor);

If this is called in a thread, it will crash the application.

At least a small step forward. But I couldn't get the style's event position, it is always 0. Looking into the code in stc.cpp, it says:

Code: Select all

    switch (scn.nmhdr.code) {
    case SCN_STYLENEEDED:
        evt.SetEventType(wxEVT_STC_STYLENEEDED);
        break;
        

I guess I will never be able to access the event's position, so scintilla keeps requesting a new style over and over. The length is always 0.
Looking at SCN_MODIFIED, the length is given:

Code: Select all

    case SCN_MODIFIED:
        evt.SetEventType(wxEVT_STC_MODIFIED);
        evt.SetModificationType(scn.modificationType);
        SetEventText(evt, scn.text, scn.length);
        evt.SetLength(scn.length);
        evt.SetLinesAdded(scn.linesAdded);
        evt.SetLine(scn.line);
        evt.SetFoldLevelNow(scn.foldLevelNow);
        evt.SetFoldLevelPrev(scn.foldLevelPrev);
        evt.SetToken(scn.token);
        evt.SetAnnotationLinesAdded(scn.annotationLinesAdded);
        break;

Is this a bug or am I missing something, since it should be something like

Code: Select all

event.GetLength();

which is never possible with SCN_STYLENEEDED?

Thanks!

[evstevemd]

Good morning everybody,
..............
I have to parse the whole document each time a character is added

Why?
You should check last time it was colorized and only colorize that part. Unless I don't understand your question something like:

Code: Select all

int lastStyled = GetEndStyled();
    if(lastStyled < pos) //pos can be your current position
    {
        Colourise(lastStyled, pos);
    }

[ONEEYEMAN]

Hi,
Why do you use regular expressions?
Anyway, there is an example of using custom lexer in in Git HEAD of wxWidgets. You should check it and do something similar.

Thank you.

[evstevemd]

Hi,
Why do you use regular expressions?
Anyway, there is an example of using custom lexer in in Git HEAD of wxWidgets. You should check it and do something similar.

Thank you.

Putting a link to it will be quiet helpful

[ONEEYEMAN]

Hi,
Just clone a repo and check the stc sample.

Thank you.

[evstevemd]

Hi,
Just clone a repo and check the stc sample.

Thank you.

I was talking of custom lexer. I have it here and never heard of it before. Can you point out the path?

[ONEEYEMAN]

Hi,

I have it here and never heard of it before.

Which one is it?

Thank you.

[evstevemd]

Which one is it?

I mean this one. It is interesting and I would like to have a look at it

Anyway, there is an example of using custom lexer in in Git HEAD of wxWidgets.

[ONEEYEMAN]

Hi,
Just grab the latest wxWidgets sources and look at the stc sample.

Thank you.

[New Pagodi]

Which one is it?

I mean this one. It is interesting and I would like to have a look at it

Anyway, there is an example of using custom lexer in in Git HEAD of wxWidgets.

That particular patch hasn't been accepted yet. The patch is available here. It will only work with the latest wxWidgets from git since no official release supports external lexers yet.

[evstevemd]

That particular patch hasn't been accepted yet. The patch is available here. It will only work with the latest wxWidgets from git since no official release supports external lexers yet.

Thank you!

[New Pagodi]

Good morning everybody,

after some small difficulties, I finally managed to write a custom syntax highlighting for CNC G-Code (Syntax roughly similar to basic, including positions in coordinates and stuff).
I did ask a few questions here: viewtopic.php?f=1&t=43800 which did help a lot.
The parser is working fine.
The main problem I face is that I run into serious performance problems. Parsing for G-Godes is rather fast, but searching ~900 keywords via RegEx is a pain performance-wise. And since STC will wait for the event to being finished, I have to parse the whole document each time a character is added
I did try:

Code: Select all

uint64_t startpos=GUIFrame::m_EditorSTC->GetEndStyled();
uint64_t endpos=startpos+event.GetLength();
this->highlightSTCsyntax(startpos,startpos+endpos);

but still, the performance isn't sufficient.
Anyway, my text files aren't this large (usually below 20k) so I could just parse in a background thread and highlight later on.
But: If I do this, some data race seems to occur, leading to a segmentation fault somewhere deep in wxwidgets.
I also thought about parsing the G-Codes first (similar to a scintilla lexer, pretty fast, only going through the document once) and then parsing all those keywords in a background thread, adding the style later- which fails for the same reason.

Is there any thread-safe way to update the styling of a STC with a background thread (including extracting the actual text out of the STC with getText())?
I have some deep fear that this might actually end in writing a custom scintilla lexer... Any chance I can do so without recompiling wxwidgets?

Thank you very much!

Handling styling messages is a little complicated, but it shouldn't be necessary to do it in a secondary thread. I'm not sure you're doing it right though. In particular, you shouldn't parse the entire document every time. Scintilla is pretty good at guessing what needs to be styled when a change to a document is made, and you should only attempt to style the portions it thinks should be styled.

Here's a simple example of how to do that. It just assigns a style number for a piece of entered text to be line number of where it was entered. That's a silly thing to do but I can't think of a better example of choosing style numbers numbers right now:

Code: Select all

// For compilers that support precompilation, includes "wx/wx.h".
#include "wx/wxprec.h"

#ifdef __BORLANDC__
    #pragma hdrstop
#endif

// for all others, include the necessary headers (this file is usually all you
// need because it includes almost all "standard" wxWidgets headers)
#ifndef WX_PRECOMP
    #include "wx/wx.h"
#endif

#include <wx/spinctrl.h>
#include <wx/stc/stc.h>

class myFrame : public wxFrame
{
    public:
        myFrame( wxWindow* parent, int id = wxID_ANY, wxString title = "Demo"
                , wxPoint pos = wxDefaultPosition, wxSize size = wxSize(481,466)
                , int style = wxDEFAULT_FRAME_STYLE|wxTAB_TRAVERSAL );
    private:
        wxStatusBar* statusBar;
        wxStyledTextCtrl* m_stc;
        wxTextCtrl* m_textCtrl1;
        wxSpinCtrl* m_spinCtrl1;

        void OnStyleNeeded(wxStyledTextEvent& event);
        void OnUpdateUI(wxStyledTextEvent& event);
        void OnGetStyleAt( wxCommandEvent& event );
};


myFrame::myFrame( wxWindow* parent, int id, wxString title, wxPoint pos
                 , wxSize size, int style )
        :wxFrame( parent, id, title, pos, size, style )
{
    statusBar = CreateStatusBar( 2, wxST_SIZEGRIP, wxID_ANY );

    wxPanel* panel = new wxPanel( this );
    wxBoxSizer* bSizer1 = new wxBoxSizer( wxVERTICAL );

    m_stc = new wxStyledTextCtrl(panel) ;
    bSizer1->Add( m_stc, 1, wxALL|wxEXPAND, 5 );

    m_textCtrl1 = new wxTextCtrl( panel, wxID_ANY, wxEmptyString,
                                  wxDefaultPosition, wxDefaultSize,
                                  wxTE_READONLY);
    bSizer1->Add( m_textCtrl1, 0, wxALL|wxEXPAND, 5 );

    wxBoxSizer* bSizer2 = new wxBoxSizer( wxHORIZONTAL );
    wxButton* button = new wxButton( panel, wxID_ANY, "GetStyleAt:");
    m_spinCtrl1 = new wxSpinCtrl( panel, wxID_ANY, wxEmptyString,
                                  wxDefaultPosition, wxDefaultSize,
                                  wxSP_ARROW_KEYS, 0, 10000, 0 );
    bSizer2->Add( button, 0, wxALL, 5 );
    bSizer2->Add( m_spinCtrl1, 0, wxALL, 5 );
    bSizer1->Add( bSizer2, 0, wxEXPAND, 5 );


    panel->SetSizer( bSizer1 );

    Layout();

    button->Bind( wxEVT_BUTTON, &myFrame::OnGetStyleAt, this );
    m_stc->Bind(wxEVT_STC_STYLENEEDED,&myFrame::OnStyleNeeded,this);
    m_stc->Bind(wxEVT_STC_UPDATEUI,&myFrame::OnUpdateUI,this);
}

void myFrame::OnStyleNeeded(wxStyledTextEvent& event)
{
    //Collect the data about the range that needs to be styled:
    int last_postion_to_style = event.GetPosition();
    int start_psn = m_stc->GetEndStyled();
    int line_number = m_stc->LineFromPosition(start_psn);
    start_psn = m_stc->PositionFromLine(line_number);

    //Do the styling.  A real styling routine would iterate over the text in the
    //range that needs to be styled and compute a style numbers for portions of
    //of the text based on the surrounding text.
    //
    //However for this example, I'll just use the linenumber for the style to be
    //assigned and assign it to all the text in the range to be styled.
    int style_to_set = line_number+1;
    int range_length = last_postion_to_style - start_psn ;
    m_stc->StartStyling(start_psn,0);
    //For reasons I'd rather not go into, if using wxWidgets 3.03 or earlier,
    //you should instead use the following line:
    //m_stc->StartStyling(startPos,255);
    m_stc->SetStyling(range_length,style_to_set);

    //For this example, also output a message about the work done.
    m_textCtrl1->Clear();
    (*m_textCtrl1) << "Style requested at " << last_postion_to_style << ".";
    (*m_textCtrl1) << "  " << range_length;
    (*m_textCtrl1) << " characters starting at " << start_psn;
    (*m_textCtrl1) << " set to style " << style_to_set << ".";
}


void myFrame::OnUpdateUI(wxStyledTextEvent& event)
{
    int updated = event.GetUpdated();

    if( updated&wxSTC_UPDATE_SELECTION || updated&wxSTC_UPDATE_CONTENT )
    {
        int cur_psn = m_stc->GetCurrentPos();
        wxString msg =
           wxString::Format("current postion: %d  current style: %d ",
                            cur_psn, m_stc->GetStyleAt(cur_psn));
        statusBar->SetStatusText( msg , 1 );
    }
}

void myFrame::OnGetStyleAt( wxCommandEvent& event )
{
    int postion_requested = m_spinCtrl1->GetValue();

    m_textCtrl1->Clear();
    (*m_textCtrl1) << "Style at position " << postion_requested << " is ";
    (*m_textCtrl1) << m_stc->GetStyleAt(postion_requested);
}


class myApp : public wxApp
{
    public:
        virtual bool OnInit()
        {
            myFrame* frame = new myFrame(NULL);
            frame->Show();
            return true;
        }
};

wxIMPLEMENT_APP(myApp);

If using wxWidgets 3.03 or earlier, change line 87 to "m_stc->StartStyling(startPos,255)" instead.

[TobiasA]

Why?
You should check last time it was colorized and only colorize that part. Unless I don't understand your question something like:

Code: Select all

int lastStyled = GetEndStyled();
    if(lastStyled < pos) //pos can be your current position
    {
        Colourise(lastStyled, pos);

Now that's one big step towards the solution... I was looking for that end position over and over. After reading your code snipped and trying around a little, I found

Code: Select all

    uint64_t startpos=GUIFrame::m_EditorSTC->GetEndStyled();
    uint64_t endpos=GUIFrame::m_EditorSTC->GetLineEndPosition(GUIFrame::m_EditorSTC->LineFromPosition(GUIFrame::m_EditorSTC->GetCurrentPos()));
    this->highlightSTCsyntax(startpos,endpos);

to work out perfectly. To make absolutely sure that I get everything alright, I parse the few words in front of that sequence as well since a variable might have gotten "cut off".
Still, there are some flaws performance- wise but this is not related to wxWidgets at all. I found a workaround for that, more of that later.
My deep trouble was finding the end position of "style end" which I have been searching inside the event all day long. I thought that the event specifies the end of the position you need to restyle
Thank you very much!

Hi,
Why do you use regular expressions?
Anyway, there is an example of using custom lexer in in Git HEAD of wxWidgets. You should check it and do something similar.

Thank you.

I have to explain the background, I think... There are lines like this in a siemens NC program:

Code: Select all

SUPA G0 G90 G40 G60 Z=($MA_POS_LIMIT_PLUS[AX3]-0.1)

SUPA is an NC command, followed by a couple of G-Codes. $MA_POS_LIMIT_PLUS is the software end, but this includes "POS" as an NC-command as well. In total, the siemens knows about ~1100 different nc commands each with its own command. After removing G- and M-Codes and stuff from the list, about 900 remain.
Every NC command has to have a space or line feed in front and might be trailed by "=" or "[]" or "()" or digits (like G54).
I came up with having

Code: Select all

std::string key=("\\b("+m_CommandHighlightVector[i]+")\\W");

which gives good results (no digits, but those left don't have digits), but is one hell of a performance eater.
In total, the CNC code is a programming language on its own, similar to BASIC, just with a few hundred keywords and thousands of different variables
I plan to parse things like

Code: Select all

DEF INT VARIABLE

inside the NC program and display those variables (the above declares a "member variable" in your NC program), along with some other fancy stuff in the future. The ultimate goal is to include some kind of autocompletion for NC variables and parsing control structures... Maybe.
Think of some kind of "NC-Code IDE".

To sum things up: I need to parse a lot of stuff in the background which includes pulling the whole text from the STC at once, parsing for about up to a minute or so (I should do that multithreaded) and then (hopefully) recoloring keywords if the content did not change in the meantime.
The solution I got now is that I parse the basic G-Codes like "G0 G90", M-Codes and stuff with a fast parser and not highlighting the ~900 nc commands.
Doing some quick testing, I think this should work the same way using a std mutex on STC_modified- either style or modify.

I hope that someone having a similar problem can find this topic via google now- I've seen quite a few questions, sadly none of the suggested solutions did work.
I will have a closer look on the possibility of an external lexer.
The deeper I dive into wxWidgets, the more I love it...
Thank you all for your help!

[TobiasA]

Handling styling messages is a little complicated, but it shouldn't be necessary to do it in a secondary thread. I'm not sure you're doing it right though. In particular, you shouldn't parse the entire document every time. Scintilla is pretty good at guessing what needs to be styled when a change to a document is made, and you should only attempt to style the portions it thinks should be styled.

Here's a simple example of how to do that. It just assigns a style number for a piece of entered text to be line number of where it was entered. That's a silly thing to do but I can't think of a better example of choosing style numbers numbers right now:

Code: Select all

// For compilers that support precompilation, includes "wx/wx.h".
#include "wx/wxprec.h"

#ifdef __BORLANDC__
    #pragma hdrstop
#endif

// for all others, include the necessary headers (this file is usually all you
// need because it includes almost all "standard" wxWidgets headers)
#ifndef WX_PRECOMP
    #include "wx/wx.h"
#endif

#include <wx/spinctrl.h>
#include <wx/stc/stc.h>

class myFrame : public wxFrame
{
    public:
        myFrame( wxWindow* parent, int id = wxID_ANY, wxString title = "Demo"
                , wxPoint pos = wxDefaultPosition, wxSize size = wxSize(481,466)
                , int style = wxDEFAULT_FRAME_STYLE|wxTAB_TRAVERSAL );
    private:
        wxStatusBar* statusBar;
        wxStyledTextCtrl* m_stc;
        wxTextCtrl* m_textCtrl1;
        wxSpinCtrl* m_spinCtrl1;

        void OnStyleNeeded(wxStyledTextEvent& event);
        void OnUpdateUI(wxStyledTextEvent& event);
        void OnGetStyleAt( wxCommandEvent& event );
};


myFrame::myFrame( wxWindow* parent, int id, wxString title, wxPoint pos
                 , wxSize size, int style )
        :wxFrame( parent, id, title, pos, size, style )
{
    statusBar = CreateStatusBar( 2, wxST_SIZEGRIP, wxID_ANY );

    wxPanel* panel = new wxPanel( this );
    wxBoxSizer* bSizer1 = new wxBoxSizer( wxVERTICAL );

    m_stc = new wxStyledTextCtrl(panel) ;
    bSizer1->Add( m_stc, 1, wxALL|wxEXPAND, 5 );

    m_textCtrl1 = new wxTextCtrl( panel, wxID_ANY, wxEmptyString,
                                  wxDefaultPosition, wxDefaultSize,
                                  wxTE_READONLY);
    bSizer1->Add( m_textCtrl1, 0, wxALL|wxEXPAND, 5 );

    wxBoxSizer* bSizer2 = new wxBoxSizer( wxHORIZONTAL );
    wxButton* button = new wxButton( panel, wxID_ANY, "GetStyleAt:");
    m_spinCtrl1 = new wxSpinCtrl( panel, wxID_ANY, wxEmptyString,
                                  wxDefaultPosition, wxDefaultSize,
                                  wxSP_ARROW_KEYS, 0, 10000, 0 );
    bSizer2->Add( button, 0, wxALL, 5 );
    bSizer2->Add( m_spinCtrl1, 0, wxALL, 5 );
    bSizer1->Add( bSizer2, 0, wxEXPAND, 5 );


    panel->SetSizer( bSizer1 );

    Layout();

    button->Bind( wxEVT_BUTTON, &myFrame::OnGetStyleAt, this );
    m_stc->Bind(wxEVT_STC_STYLENEEDED,&myFrame::OnStyleNeeded,this);
    m_stc->Bind(wxEVT_STC_UPDATEUI,&myFrame::OnUpdateUI,this);
}

void myFrame::OnStyleNeeded(wxStyledTextEvent& event)
{
    //Collect the data about the range that needs to be styled:
    int last_postion_to_style = event.GetPosition();
    int start_psn = m_stc->GetEndStyled();
    int line_number = m_stc->LineFromPosition(start_psn);
    start_psn = m_stc->PositionFromLine(line_number);

    //Do the styling.  A real styling routine would iterate over the text in the
    //range that needs to be styled and compute a style numbers for portions of
    //of the text based on the surrounding text.
    //
    //However for this example, I'll just use the linenumber for the style to be
    //assigned and assign it to all the text in the range to be styled.
    int style_to_set = line_number+1;
    int range_length = last_postion_to_style - start_psn ;
    m_stc->StartStyling(start_psn,0);
    //For reasons I'd rather not go into, if using wxWidgets 3.03 or earlier,
    //you should instead use the following line:
    //m_stc->StartStyling(startPos,255);
    m_stc->SetStyling(range_length,style_to_set);

    //For this example, also output a message about the work done.
    m_textCtrl1->Clear();
    (*m_textCtrl1) << "Style requested at " << last_postion_to_style << ".";
    (*m_textCtrl1) << "  " << range_length;
    (*m_textCtrl1) << " characters starting at " << start_psn;
    (*m_textCtrl1) << " set to style " << style_to_set << ".";
}


void myFrame::OnUpdateUI(wxStyledTextEvent& event)
{
    int updated = event.GetUpdated();

    if( updated&wxSTC_UPDATE_SELECTION || updated&wxSTC_UPDATE_CONTENT )
    {
        int cur_psn = m_stc->GetCurrentPos();
        wxString msg =
           wxString::Format("current postion: %d  current style: %d ",
                            cur_psn, m_stc->GetStyleAt(cur_psn));
        statusBar->SetStatusText( msg , 1 );
    }
}

void myFrame::OnGetStyleAt( wxCommandEvent& event )
{
    int postion_requested = m_spinCtrl1->GetValue();

    m_textCtrl1->Clear();
    (*m_textCtrl1) << "Style at position " << postion_requested << " is ";
    (*m_textCtrl1) << m_stc->GetStyleAt(postion_requested);
}


class myApp : public wxApp
{
    public:
        virtual bool OnInit()
        {
            myFrame* frame = new myFrame(NULL);
            frame->Show();
            return true;
        }
};

wxIMPLEMENT_APP(myApp);

If using wxWidgets 3.03 or earlier, change line 87 to "m_stc->StartStyling(startPos,255)" instead.

I do the styling with

Code: Select all

for (int i=0; i<m_ParseResults.G_Vector.size(); i++) {
        uint64_t startpos=m_ParseResults.G_Vector[i].first;
        uint64_t endpos=m_ParseResults.G_Vector[i].second;
        uint64_t length=(endpos-startpos);
        GUIFrame::m_EditorSTC->StartStyling(startpos+style_offset,20);
        GUIFrame::m_EditorSTC->SetStyling(length,20);
    }

ParseResults.G_Vector is a std::pair of int64, first value is the start and second value is the end of the G-Code that was found.
Style 20 is a customized style which is set upon the construction of my frame:

Code: Select all

GUIFrame::m_EditorSTC->StyleSetForeground(20,m_Gcolor);

Which works fine. However, I'm on wxWidgets 3.0.3 and I use this to "reset" the style prior to applying the new style and it works:

Code: Select all

    GUIFrame::m_EditorSTC->StartStyling(style_offset,0);
    GUIFrame::m_EditorSTC->SetStyling(toPos-style_offset,0);

Since style 0 is not set at all, this should have the same result I think.
Does anything bad happen if I leave it like that since it seems to work?

Basically, the above sample would mean that applying the style should be possible in a background thread.
It has to be a background thread because the workload is incredible compared to a usual syntax parser containing only a few dozen keywords.
Still, I think I should go back to a plain C or very lean C++ to optimize parsing the keywords, I think the C++11 regex is not really the best solution in terms of performance. It takes up to a minute to parse a common NC program in debug
This means that it might take up to 5 seconds or so until you can process another keypress if parsing the regexes after the G-codes like I explained in my earlier post (I wrote the reply just at the same time you replied to this thread). The basic idea is to parse the regexes later on in a background thread. I wrote a lean and pretty fast C++ function to parse through the code and find the most common G-Codes so this lightweight solution covers up the main syntax highlighting and the background thread finds all the complexer NC commands.
Given this information, this should work.

Thank you very much!