Adding R2L support to Scintilla

[Raghda Morsy‏]

Hello Neil,

I'm Raghda from Uniface , we want to contribute on adding R2L support in Scintilla .

I've added a small portion of code to "ScintillaBase.cxx" to "AddCharUTF" function , to explain the direction issue , and included Arabic characters only :

if (s[0] == ' ')
    {
        std::string str = Editor::RangeText(Editor::CurrentPosition() - 2, Editor::CurrentPosition());
        const char * prevChar;
        prevChar = str.c_str();

        if ((prevChar[0] >= '\xd8' && prevChar[0] <= '\xdb') && (prevChar[1] >= '\x80' && prevChar[1] <= '\xbf'))
        {
            int newPos = Editor::pdoc->NextWordEnd(Editor::CurrentPosition(), -1);
            int currentColumn = Editor::pdoc->GetColumn(newPos);
            if (currentColumn == 0)
            {
                //Editor::pdoc->InsertString(newPos+1, " ", 1);
                Editor::AddCharUTF(s, len, treatAsDBCS);
                Editor::MovePositionTo(newPos);
                return;
            }
            Editor::MovePositionTo(newPos);
           
            int currentPosition = Editor::CurrentPosition();
        }
    }

This fixed the direction issue , so it types correctly from right to left , but still didn't fix other issues , like using "" or // which swaps the direction , and fails in getting the correct selection.
Also some lexers are still in need of modifications to fix the direction.

Please find the the files in this link for explanation :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIRHlseDV5X3pBb2c?usp=sharing


Waiting for your suggestions for the points we should care of :)

Thanks
Raghda

[Neil Hodgson‏]

Hi Raghda,

> I've added a small portion of code to "ScintillaBase.cxx" to "AddCharUTF" function , to explain the direction issue , and included Arabic characters only :
>
> if (s[0] == ' ')
> {
> std::string str = Editor::RangeText(Editor::CurrentPosition() - 2, Editor::CurrentPosition());
> const char * prevChar;
> prevChar = str.c_str();
>
> if ((prevChar[0] >= '\xd8' && prevChar[0] <= '\xdb') && (prevChar[1] >= '\x80' && prevChar[1] <= '\xbf’))

Here, the code is assuming that interesting characters take up 2 bytes but some Arabic characters take 3 or 4 bytes in UTF-8. Its also performing range checks on the bytes of a UTF-8 string and it is easier to check the code point of that character. The nicest method to find the character before a position is currently Document::CharacterBefore which returns both the code point value and the width in bytes, so:

Document::CharacterExtracted ce = pdoc->CharacterBefore(CurrentPosition());

The byte range expression is testing that the character is in the main Arabic block of Unicode U+600..U+6FF. This should be tied up into a function so it can easily be extended to other ranges:

static bool IsArabic(unsigned int character) {
return (character >= 0x600) && (character <= 0x6FF);
}

Then the extraction and check become:

Document::CharacterExtracted ce = pdoc->CharacterBefore(CurrentPosition());
if (IsArabic(ce.character)) {

Eventually, this code will have to handle rectangular and discontiguous selections by looping for each caret and performing the same check but, for now, we’ll concentrate on the simpler case of a single selection.

> {

> int newPos = Editor::pdoc->NextWordEnd(Editor::CurrentPosition(), -1);
> int currentColumn = Editor::pdoc->GetColumn(newPos);
> if (currentColumn == 0)
> {
> //Editor::pdoc->InsertString(newPos+1, " ", 1);
> Editor::AddCharUTF(s, len, treatAsDBCS);
> Editor::MovePositionTo(newPos);
> return;
> }
> Editor::MovePositionTo(newPos);

So, when the caret is after an Arabic letter, one of two paths is taken. If the word containing the Arabic letter is the first on the line then the space is placed after the current position. Otherwise the current position is moved to the end of the word before the word containing the Arabic letter before inserting the space. Thus the space appears before the word.

These two paths do not appear equivalent: with the first, each newly typed word is initially connected to the previously typed word until a space is typed to break them apart. With the second path, each new word appears independent. Perhaps this is considered OK but it appears weird to me.

> int currentPosition = Editor::CurrentPosition();
> }
> }
>
> This fixed the direction issue , so it types correctly from right to left , but still didn't fix other issues , like using "" or // which swaps the direction , and fails in getting the correct selection.
> Also some lexers are still in need of modifications to fix the direction.

The reason that you are seeing different orders with different lexers is that lexers assign different styles to different types of element and Scintilla breaks the text up into sequences with the same style before drawing. Thus “a b” in text is a single run with 3 characters but it is (commonly) 3 runs in C++ with styles (identifier, default, identifier). If ‘a’ and ‘b’ are Arabic words then the words will each be correctly drawn but they will be ordered differently since they are drawn as ‘a’ then ‘ ‘ then ‘b’ with the x position moving after each substring is drawn.

This can not be fixed by changing the lexers: they just assign styles to ranges of bytes and do not perform the drawing.

> Please find the the files in this link for explanation :
> https://drive.google.com/drive/folders/0B1_ta5eAkhnIRHlseDV5X3pBb2c?usp=sharing

The default lexer in SciTE is C++ so the text is being displayed as a sequence of separate words. Save the file after performing the typing as shown in the video. Then open that file in Notepad and the text may be displaying the words in a different order. Or copy and paste between SciTE and other applications.

Having different display orders in C++ mode and text mode may be the correct thing to do as a C++ compiler will be reading the file in the logical order interpreting the lexemes as they are found - it doesn’t reorder the line to match the way it would be read as text.

Neil

[Raghda Morsy‏]

Thank you so much for the explanation , I'm going to apply your suggestions, and give you feedback. 

[Raghda Morsy‏]

Hi Neil,

To add right to left characters , logically the caret should move to the left after each character, so I added the following code to ScintillaBase::AddCharUTF  , instead of the code mentioned before:

    Editor::AddCharUTF(s, len, treatAsDBCS);

    Document::CharacterExtracted ce = pdoc->CharacterBefore(CurrentPosition());
    if (IsArabic(ce.character)) {

        Editor::MovePositionTo(CurrentPosition()-len);       
    }

But the resulting text is drawn incorrectly , if I type :
رغدة
the resulting text is :
ةدغر
The letters are reversed ! although the caret is at the correct position, each newly added character is written at the most right instead of being added at the most left.
But when I apply this to English characters , the logic works fine , when I type Hello the resulting text is olleH , even if this is wrong English , but this is the correct logic in Arabic.
Why does this work fine with English , and doesn't work with Arabic ? And where can I write modifications to apply changes in the letters order ?

And regarding searching for a text, in the original code , without adding any modifications :
 if I type an Arabic sentence inside "" then try to find a word , although the word is can be found correctly , but it can't be selected correctly !
So if I type :
"رغدة أحمد مرسي طايع"
Then when I try to find the word :
مرسي
Although the word is found , the returning position of the word is incorrect !
The correct position should be 10 , but the returning position is  19 !  Which means that even if the Arabic sentence inside "" looks written correctly , it is actually stored incorrectly , so the sentence is stored like this :
"طايع مرسي أحمد رغدة"
So I think if the characters are added according to the correct logic , that can fix the searching problem .

Am I thinking in the correct way ? if yes , where should I start ? if no , what is the right way ?

Thanks
Raghda

[Neil Hodgson‏]

Raghda:

> To add right to left characters , logically the caret should move to the

> left after each character, so I added the following code ...

> But the resulting text is drawn incorrectly , if I type :
> رغدة
> the resulting text is :
> ةدغر
> The letters are reversed ! although the caret is at the correct position,
> each newly added character is written at the most right instead of being
> added at the most left.

The underlying Win32 calls used to draw the text see that the text
is Arabic and draw it reversed.

I think that moving the caret is not a good approach as it seems to
be different to how bidirectional text is handled by other software.

The common way to approach this seems to be to keep the text
ordered logically, as it would be typed, but then to display it
reordered as needed for the languages involved. So, using upper-case
for Arabic and lower-case for English, typing an English word "word"
followed by an Arabic word "WORD" would produce a document containing
"wordWORD" but would be displayed like "wordDROW".

The introduction of this document may help:
http://www.unicode.org/reports/tr9/

To implement this path, there should be some code that takes a line
of text and breaks it up into directional runs then draws these runs
in order. There would also be changes to drawing other elements like
the caret so it appears in the correct location and to the correlation
of mouse clicks to text. There are other open-source projects such as
editors and web browsers that have implemented this and it may be
worthwhile reading the source code to one of these.

Neil

[Raghda Morsy‏]

   Hello Neil,

 

I     have added a function “FillR2LPositions” to calculate the real positions of R2L characters, and the virtual positions which we see on screen.

   It’s tested only with characters inside “”, supposing there is only 1 segment of quotes “”, and only one line.
     The positions are calculated fine, but when I try it in FindText , supposing the search is done for complete word :

  long Document::FindText(int minPos, int maxPos, const char *search,

                        int flags, int *length)

  , although the position is returned correctly the selection is not, the selection is always shifted right.

 

  So if I type:

  Hi “السلام عليكم و رحمة الله و بركاته”

  And then search for “عليكم”, I get the selection like this:

  Hi “السلام عليكم و رحمة الله و بركاته”

The real position of the word "عليكم" is 17 , while the virtual position is 41 , if I subtract 2 (which is the Arabic character size) from virtual position , the selection start is correct , but the selection end is wrong and shorter than the word size!

Here is what I added to class ScintillaBase

   static bool IsArabic(unsigned int character) {

        return (character >= 0x600) && (character <= 0x6FF);
    }

    void FillR2LPositions(int startPos , int endPos , unsigned int startSortChar , unsigned int endSortChar , char delimiters);

And at class Document :

struct R2LWordPosition
    {
        std::string word;
        int realPos;
        int virtualPos;
        bool operator < (const R2LWordPosition& pos) const
        {
            return (word < pos.word);
        }
    };
    R2LWordPosition R2LP;
    std::vector<R2LWordPosition> R2LPositions;

And here is the function definition :

void ScintillaBase::FillR2LPositions(int startPos, int endPos, unsigned int startSortChar, unsigned int endSortChar, char delimiters)
{
    bool sortStarted = false;
    int startSortPos = -1;
    int endSortPos = -1;
    bool foundR2L = false;
    int wordsLength = 0;
    int delimiterWidth = 0;   
   
    //Find the positions of startSortChar and endSortChar (like '\"')
    Document::CharacterExtracted currentChar = pdoc->CharacterAfter(startPos);
    for (int i = currentChar.widthBytes; i < endPos; i += currentChar.widthBytes)
    {
        if (currentChar.character == startSortChar && !sortStarted)
        {
            sortStarted = true;
            startSortPos = i - currentChar.widthBytes;

        }

        else if (IsArabic(currentChar.character) && sortStarted) {
            foundR2L = true;
        }

        else if (currentChar.character == endSortChar && sortStarted && startSortPos >= 0)
        {
            endSortPos = i - currentChar.widthBytes;
            sortStarted = false;
        }
        currentChar = pdoc->CharacterAfter(i);
    }

    //if the startSortChar & endSortChar don't include R2L chars between them , no need to go through this loop
    if (foundR2L)
    {
        int wordStart = -1;
        int wordEnd = -1;

        Document::CharacterExtracted ce = pdoc->CharacterBefore(endSortPos);
        //int w=0;
        for (int j = endSortPos - ce.widthBytes ; j > (startSortPos - ce.widthBytes) ; j -= ce.widthBytes)
        {

            if (IsArabic(ce.character) && wordEnd < 0)
            {
                wordEnd = j + ce.widthBytes;               
            }
            else if ((ce.character == delimiters || ce.character == startSortChar) && wordEnd >= 0)
            {
                wordStart = j + ce.widthBytes;
                delimiterWidth += ce.widthBytes;
                pdoc->R2LP.virtualPos = startSortPos + delimiterWidth + wordsLength;// -w;
                pdoc->R2LP.realPos = wordStart;
                pdoc->R2LP.word = RangeText(wordStart, wordEnd);
                pdoc->R2LPositions.push_back(pdoc->R2LP);
                wordEnd = -1;
                wordStart = -1;
                wordsLength += pdoc->R2LP.word.length();

            }
           
            ce = pdoc->CharacterBefore(j);
            //w = ce.widthBytes;
        }
    }
    std::sort(pdoc->R2LPositions.begin(), pdoc->R2LPositions.end());

}

I call it to fill the vector R2LPositions  , after pressing <ENTER> at ScintillaWin:

        if (wcs == '\n' || wcs == '\r')
                {
                    Document::CharacterExtracted ce('\"',1);
                    ce.character = '\"';
                    /*std::vector<char> delimiters;
                    delimiters.push_back(' ');*/
                    FillR2LPositions(0, CurrentPosition(), ce.character, ce.character, ' ');
                }

Then at FindText , I return the word virtual position :

    R2LP.word = search;
    auto it = std::find_if(R2LPositions.begin(), R2LPositions.end(), [&search](const R2LWordPosition& obj) {return obj.word == search; });
    return it->virtualPos;


What do you think about this approach of defining real and virtual positions , and let the user decide range of text , and the character like ( "" {} // ,...),  that isolates R2L characters virtually . (null values of startSortChar & endSortChar are not considered yet , but it's important to consider them latter as the full document can be R2L , not just segments"
And what do you suggest to solve the selection start & end positions ?
Also what if a user selects manually a piece of R2L text , what do you suggest to make the correct selection on screen , with correct words , so copy & past can work correctly ?


Thanks

[Neil Hodgson‏]

Raghda Morsy:

> I have added a function “FillR2LPositions” to calculate the real positions of R2L characters, and the virtual positions which we see on screen.
> It’s tested only with characters inside “”, supposing there is only 1 segment of quotes “”, and only one line.
> The positions are calculated fine, but when I try it in FindText , supposing the search is done for complete word :
> long Document::FindText(int minPos, int maxPos, const char *search,
> int flags, int *length)
>
> , although the position is returned correctly the selection is not, the selection is always shifted right.
>
>
> So if I type:
>
> Hi “السلام عليكم و رحمة الله و بركاته”
>
> And then search for “عليكم”, I get the selection like this:
>
> Hi “السلام عليكم و رحمة الله و بركاته”
>
>
> The real position of the word "عليكم" is 17 , while the virtual position is 41 , if I subtract 2 (which is the Arabic character size) from virtual position , the selection start is correct , but the selection end is wrong and shorter than the word size!

What I think is occurring here is that your approach may work (somewhat) if all the characters were exactly the same width in pixels (monospaced text) but each character has a different width (proportional). Arabic has a particularly wide range of character widths even with some (otherwise) monospaced fonts like Consolas.

For example, if we take the right end of the word السلام there is a wide character س and two short characters ل and ا. The (assuming Windows and DirectWrite) GetClusterMetrics method is passed [ا and ل and س] (document order) and returns widths of 3.1, 4.2, and 10.5 pixels. Now, that text is displayed in the order [س and ل and ا] so the widths don’t line up with the drawn characters.

For another view on this, make a document with just “الس” and zoom in to make it nice and large. Place the caret at the start and press cursor right. You will see a short move (wide enough for “ا", then a bit longer “ل", then a large move for “س” to the end of the document.

Therefore, any positioning of the selection or caret needs to take into account the different character widths.

Trying to reverse the text from “الس” to “سلا” then asking for widths is unlikely to work well due to the way Arabic reshapes characters depending on their context.

> What do you think about this approach of defining real and virtual positions , and let the user decide range of text , and the character like ( "" {} // ,...), that isolates R2L characters virtually . (null values of startSortChar & endSortChar are not considered yet , but it's important to consider them latter as the full document can be R2L , not just segments"
> And what do you suggest to solve the selection start & end positions ?

I don’t think working on this at the level of real and virtual positions in the document will work well due to differing character widths. Working on the lower level code that places and draws text, selections and carets at pixel locations may be better. Placing the caret for an Arabic sequence may require reversing the widths used.

> Also what if a user selects manually a piece of R2L text , what do you suggest to make the correct selection on screen , with correct words , so copy & past can work correctly ?

Drawing the selection is more complex so it may be worthwhile concentrating initially on drawing the caret positioned well and then that can be generalised to drawing the selection.

Neil

[Raghda Morsy‏]

Thank you for your explanation , where can I find the caret drawing ?

[Neil Hodgson‏]

Raghda Morsy:

> Thank you for your explanation , where can I find the caret drawing ?

EditView::DrawCarets

Neil

[Raghda Morsy‏]

Hi Neil,

Will it be a problem if we use : DWRITE_MATRIX , DWRITE_HIT_TEST_METRICS to calculate caret position ? any performance or compatibility issues ?
And what about using wstring and wchar_t * to hold the body text instead of char vector ?

Thanks
Raghda

[Neil Hodgson‏]

Hi Raghda,

> Will it be a problem if we use : DWRITE_MATRIX , DWRITE_HIT_TEST_METRICS to calculate caret position ? any performance or compatibility issues ?

Its more important to get something working first so if that is easier by communicating with DirectWrite structures then start that way.

Later on, this can be made portable.

> And what about using wstring and wchar_t * to hold the body text instead of char vector ?

wchar_t is generally a bad idea. Its 16-bits on Windows and 32-bits on Unix. Sometimes people are attracted to wchar_t on Windows because they think of it as a fixed-width encoding but then have trouble with characters outside the base multilingual plane.

Again, however, it could be used to create a proof of concept that is then adapted to work with UTF-8 or possibly UTF-32.

Neil

[Raghda Morsy‏]

Hi Neil ,

Would you please check this link :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIVlVaZVJuZTItb28?usp=sharing

It includes :

• Folder Demo : it's a video , showing R2L support on SciTE .
• Folder Raghda : it includes the supporting files (based on Microsoft sample codes for DirectWrite), isolated from the used functionalities from the rest of Scintilla code.
• Modified ScintillaWin.cxx  :  all the modifications are kept in blocks , between 2 comments (//Raghda-Start/// and //Raghda-End///)

Sorry I have disabled most of the events handles in WndProc , just to be able to handle the essential events .

The initialization is done in Initialize(HWND parentHwnd, const wchar_t* text) function , which is called after LoadD2D() , under SCI_SETTECHNOLOGY event.

Also I disabled WndPaint(uptr_t wParam) , and used OnDraw , instead of adding modifications inside Paint.

The main players here are :

•   std::wstring   text_;
•   IDWriteTextLayout*  textLayout_;
  
•   ID2D1HwndRenderTarget   *m_pRenderTarget; ( I couldn't find out where ID2D1RenderTarget *pRenderTarget; is initialized , so I used my own variable).

The code mainly dependable on text_ , and the layout is rendered with the whole text each time of Draw .

I think handling R2L text with wstring as one block is easier and more accurate than going though all the characters one by one , specially in R2L , the loops will increase if we check the whole paragraph each time a new character is added .

Unfortunately, I still don't understand the lexers and the styling process , so I couldn't add the styles , I just used vs.styles[STYLE_DEFAULT] in text format. Would you please explain the styling process , I may can try it , if you accept the new approach.

Using wstring will need changes to be done in GUI , so the messages sent to Scintilla will be in the same type. For example , for find and replace , the event lParam finding a string should be in wstring too.

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Would you please check this link :
> https://drive.google.com/drive/folders/0B1_ta5eAkhnIVlVaZVJuZTItb28?usp=sharing

OK. The video shows good behaviour and the source code can be built and run.

> The main players here are :
>
> • std::wstring text_;
> • IDWriteTextLayout* textLayout_;
> • ID2D1HwndRenderTarget *m_pRenderTarget; ( I couldn't find out where ID2D1RenderTarget *pRenderTarget; is initialized , so I used my own variable).

The ScintillaWin::EnsureRenderTarget method is where render targets are created.

> The code mainly dependable on text_ , and the layout is rendered with the whole text each time of Draw .
>
> I think handling R2L text with wstring as one block is easier and more accurate than going though all the characters one by one , specially in R2L , the loops will increase if we check the whole paragraph each time a new character is added .

While iterating over each character may be too detailed, using a single string and text layout object for the whole file is too coarse. I think the next step should be to treat the file as a sequence of lines or paragraphs.

> Unfortunately, I still don't understand the lexers and the styling process , so I couldn't add the styles , I just used vs.styles[STYLE_DEFAULT] in text format. Would you please explain the styling process , I may can try it , if you accept the new approach.

I think the approach used here is early and will need to unify to Scintilla’s current way of working. That is, it should aim to eventually use Scintilla’s existing text storage and styling code. It may be possible to work towards this over a sequence of steps.

> Using wstring will need changes to be done in GUI , so the messages sent to Scintilla will be in the same type. For example , for find and replace , the event lParam finding a string should be in wstring too.

The aim should be to use UTF-8 for document storage as Scintilla does currently.

Neil

[Raghda Morsy‏]

Hi Neil,

Here is another try , this link contains the video demo and the affected files :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIS1VfY2kyZ0ZZaGs?usp=sharing
All the changes added are between comments ////Raghda-Start//////// and /////////Raghda-End//////////////

This trial is using the current Scintilla storage and style , just adding the usage of DirectWrite hit test metrics and clusters metrics .

The functions added are :

• bool IsRightToLeft(const char *s, int len) :
  Converts the char* to TextWide buffer , checks the cluster metrics of the text , and returns true if the cluster is R2L .
  This function is called at TextSegment BreakFinder::Next(Surface *surface) which I have also changed from Next() to
  Next(Surface *surface) , because the main problem in R2L segments , that each segment is drawn alone , so R2L words are swapped L2R after each <space> . While when the R2L sentence words are all with the same style , like being inside "" , the whole sentence is treated as one segment , so DirectWrite draws it with the correct direction .
  I think this function should be used while style array is filled , but I don't know when and where the styles are filled.
  If we fill the R2L letters with the same style (I think until now we don't have any artificial language that uses R2L keywords ), it will be better in clusters metrics usage as I currently use a fixed number of clusters 1000.
  
  
• int PositionFromPoint(float x, float y, const char *s, int len, int width) :
  Calculates the caret position from a point x & y , using HitTestPoint .
  This function is called at SelectionPosition EditView::SPositionFromLocation(Surface *surface, const EditModel &model, PointDocument pt, bool canReturnInvalid, bool charPosition, bool virtualSpace, const ViewStyle &vs) , instead of the call of ll->FindPositionFromX(static_cast<XYPOSITION>(pt.x + subLineStart),rangeSubLine, charPosition);
  After getting the position inside wchar_t* text , it calculates the chars length , and returns the position inside char* text.
  
  
• XYFromPosition(const char *s, int len, int width , int caretPosition , int offset , float *x, float *y) :
  Calculates the point x&y from the given position , using HitTestTextPosition .
  This function is called at Point EditView::LocationFromPosition(Surface *surface, const EditModel &model, SelectionPosition pos, Sci::Line topLine, const ViewStyle &vs, PointEnd pe) , to calculate the point x & y .
  And also called twice at static void DrawTranslucentSelection(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,
      Sci::Line line, PRectangle rcLine, int subLine, Range lineRange, int xStart) , to calculate the left and the right of the selection rectangle.
  Each time this function called , the surface->MappedPosition(&ll->chars[0], portion.start.Position() - posLineStart) is called before it , so we can get the wchar_t position .
  
  
• void CaretRectangle(const char *s, int len, int width, int caretPosition, int offset, bool drag , PRectangle* rect) :
  Calculates the caret rectangle left & right , using HitTestTextPosition .
  This function is called at void EditView::DrawCarets(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,
      Sci::Line lineDoc, int xStart, PRectangle rcLine, int subLine) const , to get the caret rectangle . It's also called after calling surface->MappedPosition(&ll->chars[0], portion.start.Position() - posLineStart) .
  By the way, the input bool drag , was added to be used latter in selection range , but currently not used.
  
  
• int MappedPosition(const char *s, int len) :
  Calculates the text position , converts it from position inside char* text to position inside wchar_t*  text , because DirectWrite functionalities are dependent on wchar_t .
  It is called each time before calling surface->XYFromPosition(&ll->chars[0], ll->numCharsInLine, ll->widthLine, newPos, model.xOffset, &pt.x, &pt.y); and  surface->CaretRectangle(&ll->chars[0], ll->numCharsInLine, ll->widthLine, /*posCaret.Position()*/pos,(posCaret.Position() - offset), false, &rcCaret);
  

These functions are mainly dependent on lines layouts , so they take lineLayout chars as inputs , and create their own IDWriteTextLayout and their own IDWriteTextFormat , but to get more accurate results we should use pTextFormat ,or at least the same text format (like font name and size) used when the text is first drawn. Each time I try to use pTextFormat  I find it Null , when is it initialized ? when is it released ?

As you can see in the video , the R2L typing direction is working fine with the caret moving in the correct direction , also find and replace are working fine , but still the mouse click and the selection are not always accurate in styled text , although in normal text everything works fine. I'm still trying to find out the reason , but I guess it is related to the styled text format , because I used a fixed text format with font name "Verdana" , and font size 12.5 .

What do you think about this approach ? and what else do you thing should be fixed ?

Thank you for your patience
Raghda

[Neil Hodgson‏]

Hi Raghda,

The convention in Scintilla is to use the structured types Point and PRectangle where possible and return structures instead of using pointer parameters. So,

int PositionFromPoint(Point pt, const char *s, int len, int width);
Point XYFromPosition(const char *s, int len, int width, int caretPosition, int offset);
PRectangle CaretRectangle(const char *s, int len, int width, int caretPosition, int offset, bool drag);

> • bool IsRightToLeft(const char *s, int len) :

> Converts the char* to TextWide buffer , checks the cluster metrics of the text , and returns true if the cluster is R2L .

Should be mentioned that this reports the status of the first cluster of the text, not the status of the whole s[0..len].

I expect that further along, a better API will be to calculate the R2L status of each byte in a string (the whole line) and return this as an array.

> This function is called at TextSegment BreakFinder::Next(Surface *surface) which I have also changed from Next() to
> Next(Surface *surface) , because the main problem in R2L segments , that each segment is drawn alone , so R2L words are swapped L2R after each <space> . While when the R2L sentence words are all with the same style , like being inside "" , the whole sentence is treated as one segment , so DirectWrite draws it with the correct direction .
> I think this function should be used while style array is filled , but I don't know when and where the styles are filled.

Styles are filled in LayoutLine with the GetStyleRange call.

> If we fill the R2L letters with the same style (I think until now we don't have any artificial language that uses R2L keywords ), it will be better in clusters metrics usage as I currently use a fixed number of clusters 1000.

One of the points of the BreakFinder is to limit the size of text segments sent to platform APIs (<300bytes) as there have been bugs with trying to draw or calculate sizes of long strings.

> Each time this function called , the surface->MappedPosition(&ll->chars[0], portion.start.Position() - posLineStart) is called before it , so we can get the wchar_t position .

Basing this on wchar_t instead of character (Unicode code point) will likely fail for non-BMP text as it will be slicing whole characters.

> • int MappedPosition(const char *s, int len) :

> Calculates the text position , converts it from position inside char* text to position inside wchar_t* text , because DirectWrite functionalities are dependent on wchar_t .

As above, wchar_t will have problems. Some of the code (such as //Get caret rectangle) seems to be confused between wchar_t positions and byte positions.

> These functions are mainly dependent on lines layouts , so they take lineLayout chars as inputs , and create their own IDWriteTextLayout and their own IDWriteTextFormat , but to get more accurate results we should use pTextFormat ,or at least the same text format (like font name and size) used when the text is first drawn. Each time I try to use pTextFormat I find it Null , when is it initialized ? when is it released ?

Surface methods that act on fonts like DrawText take Font arguments that are found in the styling information in, for example, vsDraw.styles[styleOfText].font. The first action in these methods is to call SetFont to retrieve the pTextFormat.

> As you can see in the video , the R2L typing direction is working fine with the caret moving in the correct direction , also find and replace are working fine , but still the mouse click and the selection are not always accurate in styled text , although in normal text everything works fine. I'm still trying to find out the reason , but I guess it is related to the styled text format , because I used a fixed text format with font name "Verdana" , and font size 12.5 .

Yes, and this will also have to work when different fonts are used over a line.

The main things to do next are to use the correct font and use character instead of wchar_t. Some of the definitions in UniConversion.h like UTF16CharLength and SUPPLEMENTAL_PLANE_FIRST may help in converting between wchar_t positions and character positions.

Try using some characters from the Supplementary Multilingual Plane like 🀀, 😁, or some R2L Old South Arabian 𐩣𐩥𐩩.
https://en.wikipedia.org/wiki/Plane_(Unicode)#Supplementary_Multilingual_Plane
https://en.wikipedia.org/wiki/Ancient_South_Arabian_script

Neil

[Raghda Morsy‏]

Hi Neil ,

Here are the files after change :

https://drive.google.com/drive/folders/0B1_ta5eAkhnIQ2xTSzE2aEdmeWc?usp=sharing

int MappedPosition(const char *s, int len) is removed , as I used size_t UTF16Length(const char *s, size_t len) , also int PositionFromPoint(Point pt, const char * s, int len, int width, Font & font_) is now using size_t UTF8Length(const wchar_t *uptr, size_t tlen) to get the correct position in chars .

Also bool IsRightToLeft(const char *s, int len) has been removed from surface , and other 2 methods are added :

• bool CellBuffer::IsRightToLeft(unsigned int character) :
  It checks if the character is right to left according to : http://utf8-chartable.de/unicode-utf8-table.pl and https://en.wikipedia.org/wiki/Right-to-left#List_of_RTL_scripts
  
  
• void Document::GetRightToLeftRange(char * buffer, Sci_Position position, Sci_Position lengthRetrieve, bool *isRightToLeft_) :
  It uses IsRightToLeft to check the character right-to-left status , and fills the results in std::unique_ptr<bool[]> rightToLeft , so all the bytes used by this character will be filled with R2L status.
  This function is called at void EditView::LayoutLine(const EditModel &model, Sci::Line line, Surface *surface, const ViewStyle &vstyle, LineLayout *ll, int width) , then at TextSegment BreakFinder::Next() -which is returned back to Next() without inputs- we just check if character is right to left .
  

Now these functions are returning objects and using Font in inputs :

• int PositionFromPoint(Point pt, const char *s, int len, int width, Font & font_);
• Point XYFromPosition(const char *s, int len, int width , int caretPosition , int offset, Font & font_);
• PRectangle CaretRectangle(const char *s, int len, int width, int caretPosition, int offset , int caretWidth, Font & font_);
  

The fonts used are taken from the style of a specific position in the needed line , until now this seems working fine , even with styled text , but I don't know how will it behave if we have at the same line some very big characters with other small ones.

Also all of these functions are implemented under SurfaceD2D , while at SurfaceGDI I just added dummy methods.


Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Here are the files after change :
>
> https://drive.google.com/drive/folders/0B1_ta5eAkhnIQ2xTSzE2aEdmeWc?usp=sharing

That doesn’t include an EditView.cxx which appears required to call the new surface methods. I tried copying in the EditView.cxx from the July 11 update but that wouldn’t build as the Surface APIs have changed.

It would help avoid compile problems if all the changed files were included in each update.

> Also bool IsRightToLeft(const char *s, int len) has been removed from surface , and other 2 methods are added :
> • bool CellBuffer::IsRightToLeft(unsigned int character) :
> It checks if the character is right to left according to : http://utf8-chartable.de/unicode-utf8-table.pl and https://en.wikipedia.org/wiki/Right-to-left#List_of_RTL_scripts
>
> • void Document::GetRightToLeftRange(char * buffer, Sci_Position position, Sci_Position lengthRetrieve, bool *isRightToLeft_) :
> It uses IsRightToLeft to check the character right-to-left status , and fills the results in std::unique_ptr<bool[]> rightToLeft , so all the bytes used by this character will be filled with R2L status.

Moving this into platform-independent code is good as is locating the main method in Document. However, the position argument to GetRightToLeftRange is unused and its unclear what use it could be.

> Also all of these functions are implemented under SurfaceD2D , while at SurfaceGDI I just added dummy methods.

Yes, its likely that bidirectional text can’t be reasonably supported on GDI.

Neil

[Raghda Morsy‏]

Sorry I added by mistake the EditModel.cxx instead of EditView.cxx , now it's fixed , you will find EditView.cxx
https://drive.google.com/drive/folders/0B1_ta5eAkhnIQ2xTSzE2aEdmeWc?usp=sharing

Regarding GetRightToLeftRange , yes the position is not used , so I removed it , you will find the updated Document in the above link too.

[Neil Hodgson‏]

Raghda Morsy:

This code is working better on simple cases - its cool seeing the forward arrow moving to the left in right-to-left text.

> Now these functions are returning objects and using Font in inputs :

> • int PositionFromPoint(Point pt, const char *s, int len, int width, Font & font_);

> • Point XYFromPosition(const char *s, int len, int width , int caretPosition , int offset, Font & font_);
> • PRectangle CaretRectangle(const char *s, int len, int width, int caretPosition, int offset , int caretWidth, Font & font_);

These are using a caret position in wchar_t and so UTF16Length must be called first. Since not all platforms are UTF-16, the ‘caretPosition’ argument should be in bytes for each method with UTF16Length called inside the SurfaceD2D platform-layer code.

The CaretRectangle method doesn’t seem to be doing anything more than XYFromPosition: it sets the height but we already know the line height to use for the caret height. Unless you have a plan to report more information from CaretRectangle then it isn’t needed. Avoiding code is a goodness.

> The fonts used are taken from the style of a specific position in the needed line , until now this seems working fine , even with styled text , but I don't know how will it behave if we have at the same line some very big characters with other small ones.

Having different fonts, sizes and weights are important features of Scintilla. The BreakFinder::Next changes try to avoid breaking right-to-left text and then depending on the Surface methods to make it work together but I think that is not going to work for different text styles as only one Font is passed to each Surface call. It may be better to treat each direction switch as a break.

The current code works with simple text with few changes in direction or styles but it behaves less well as the text becomes more complex. I’ve been using this “Arabic.cxx" file to try with normal SciTE C++ styles where operators are bold and a different font is used for identifiers/numbers and strings. I don’t know which type of text Uniface are interested in - maybe its for documentation or only for a single text style but if its for source code with multiple styles (like bold for operators), its examples like the following that should be made to work well.

//PrintText العَرَبِيَّة
PrintText "العَرَبِيَّة"
PrintText("العَرَبِيَّة");
PrintText("العَرَبِيَّة", Point(12.0, 36.0), len("العَرَبِيَّة"));

Neil

[Raghda Morsy‏]

 Neil Hodgson:

   This code is working better on simple cases - its cool seeing the forward arrow moving to the left in right-to-left text.

Logically left arrow should move left , and right arrow should move right. But here as we are using a mixed text  , the arrows follows the text alignment direction.
 

> Now these functions are returning objects and using Font in inputs :

>         • int PositionFromPoint(Point pt, const char *s, int len, int width, Font & font_);

>         • Point XYFromPosition(const char *s, int len, int width , int caretPosition , int offset, Font & font_);
>         • PRectangle CaretRectangle(const char *s, int len, int width, int caretPosition, int offset , int caretWidth, Font & font_);

   These are using a caret position in wchar_t and so UTF16Length must be called first. Since not all platforms are UTF-16, the ‘caretPosition’ argument should be in bytes for each method with UTF16Length called inside the SurfaceD2D platform-layer code.

OK I will call UTF16Length inside SurfaceD2D methods instead of calling it before them.

   The CaretRectangle method doesn’t seem to be doing anything more than XYFromPosition: it sets the height but we already know the line height to use for the caret height. Unless you have a plan to report more information from CaretRectangle then it isn’t needed. Avoiding code is a goodness.

I agree with you  so will remove CaretRectangle method.

> The fonts used are taken from the style of a specific position in the needed line , until now this seems working fine , even with styled text , but I don't know how will it behave if we have at the same line some very big characters with other small ones.

   Having different fonts, sizes and weights are important features of Scintilla. The BreakFinder::Next changes try to avoid breaking right-to-left text and then depending on the Surface methods to make it work together but I think that is not going to work for different text styles as only one Font is passed to each Surface call. It may be better to treat each direction switch as a break.

 

I don't understand what you mean by treating each direction as a break , BreakFinder::Next breaks if the character is not right to left , so it breaks when the direction changes.

I'm thinking of another approach , to use IDWriteTextLayout SetFontSize ,SetFontWeight , SetFontFamilyName . So after we create the textLayout , if we iterate through the line chars styles , and set the font of textLayout  for each char , then we will have a textLayout with all needed fonts.
What do you think ?

   The current code works with simple text with few changes in direction or styles but it behaves less well as the text becomes more complex. I’ve been using this “Arabic.cxx" file to try with normal SciTE C++ styles where operators are bold and a different font is used for identifiers/numbers and strings. I don’t know which type of text Uniface are interested in - maybe its for documentation or only for a single text style but if its for source code with multiple styles (like bold for operators), its examples like the following that should be made to work well.

Yes, we use it for source code with multiple styles .
 

[Neil Hodgson‏]

Raghda Morsy:

> I don't understand what you mean by treating each direction as a break , BreakFinder::Next breaks if the character is not right to left , so it breaks when the direction changes.

Maybe I am misreading the code but the changes have added a condition
&& (!ll->rightToLeft[nextBreak])
Since this is anded to the previous clauses, this is a filter that reduces the set of matching outcomes so avoids triggering if ll->rightToLeft[nextBreak] is true. Therefore style changes are only treated as breaks if the character with the new style is left to right. So style changes within right to left text or where there is a transition from l2r to r2l are not treated as breaks.

> I'm thinking of another approach , to use IDWriteTextLayout SetFontSize ,SetFontWeight , SetFontFamilyName . So after we create the textLayout , if we iterate through the line chars styles , and set the font of textLayout for each char , then we will have a textLayout with all needed fonts.
> What do you think ?

This is something that I have thought a lot about for other reasons. Platforms now have styled text drawing and measuring calls which weren’t available when Scintilla started.

Styled text capabilities differ between platforms and don’t cover all of the drawing that Scintilla does. As an example, control characters are drawn by Scintilla as string blobs containing mnemonics like [ACK] with some extra spacing for the rounded rectangle surrounding the string. Tab width and alignment is handled by explicit code in Scintilla which may not match styled text APIs. Some styled text APIs include extension points where custom drawing and sizing can be added. Custom extensions would increase the amount of work needed. It would also be possible to drop or simplify some features when in a right-to-left supporting mode as there may be more net benefit (for some) from right-to-left.

Exploring DirectWrite’s capabilities will likely lead to a better informed choice here. Some of Charles Petzold’s articles in this area may be worth reading like this one and others on his blog and in MSDN magazine.
http://www.charlespetzold.com/blog/2014/01/Character-Formatting-Extensions-with-DirectWrite.html

Neil

[Raghda Morsy‏]

Hi Neil,

Here are the modified files after iterating characters styles of each line :

https://drive.google.com/drive/folders/0B1_ta5eAkhnIaFdfMldYZGxPekU?usp=sharing

There are 3 new arrays added to class LineLayout :

1. std::unique_ptr<CHARARRAY[]> styleFontsNames; ( CHARARRAY is  : typedef const char* CHARARRAY; )
   
2. std::unique_ptr<int[]> styleSizes;
3. std::unique_ptr<int[]> styleWeight;

They are filled from vstyle.styles[ll->styles[stylesInLine]].  at :

void EditView::FillLineTextFormats(const ViewStyle &vstyle, LineLayout *ll,  const char ** styleFontsNames, int * styleSizes, int * styleWeight)
Which is called after void EditView::LayoutLine(const EditModel &model, Sci::Line line, Surface *surface, const ViewStyle &vstyle, LineLayout *ll, int width)

Then the new arrays are passed to :

• Point SurfaceD2D::XYFromPosition(const char * s, int len, int width, int caretPosition, Font & font_ , const char ** styleFontsNames, int *styleSizes, int *styleWeight)
• int SurfaceD2D::PositionFromPoint(Point pt, const char * s, int len, int width, Font & font_ , const char ** styleFontsNames, int *styleSizes, int *styleWeight)

XYFromPosition and  PositionFromPoint  are calling a newly added function to fill the textLayout styles for each char :

void SurfaceD2D::FillTextLayoutFormats(const char ** styleFontsNames, int * styleSizes, int * styleWeight, IDWriteTextLayout * textLayout, wchar_t *buffer , int tLen)

But it's still not accurate, because the text size is not correct.

At FillTextLayoutFormats , textLayout->SetFontSize(styleSizes[stylesInLine] / 71, textRange);
styleSizes[stylesInLine] which is taking the value of vstyle.styles[ll->styles[stylesInLine]].size is too big , and when dividing it by 75-70 the behavior improves , but the value to divide by is not the same for all the styles .

So where can I find the correct size ? or the correct ratio to divide by?

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> There are 3 new arrays added to class LineLayout :
> • std::unique_ptr<CHARARRAY[]> styleFontsNames; ( CHARARRAY is : typedef const char* CHARARRAY; )
> • std::unique_ptr<int[]> styleSizes;
> • std::unique_ptr<int[]> styleWeight;

Array of struct is simpler and more expandable (such as adding italics if needed) than parallel arrays.

Another possibility is using an array of Font* as the Font objects were created by SurfaceD2D and the SurfaceD2D code can look inside them as it does in SurfaceD2D::SetFont then FormatAndMetrics::HFont.

> void SurfaceD2D::FillTextLayoutFormats(const char ** styleFontsNames, int * styleSizes, int * styleWeight, IDWriteTextLayout * textLayout, wchar_t *buffer , int tLen)
>
> But it's still not accurate, because the text size is not correct.
>
> At FillTextLayoutFormats , textLayout->SetFontSize(styleSizes[stylesInLine] / 71, textRange);
> styleSizes[stylesInLine] which is taking the value of vstyle.styles[ll->styles[stylesInLine]].size is too big , and when dividing it by 75-70 the behavior improves , but the value to divide by is not the same for all the styles .

See device size calculation in FontRealised::Realise.

There is a switch statement in FillTextLayoutFormats that is converting Scintilla weights to DirectWrite weights but Scintilla already uses the same weights as DirectWrite. So simply cast the styleWeights[x] into a DWRITE_FONT_WEIGHT like FontCached::FontCached does.

Font names are not in the document encoding but always UTF-8 as implemented in FontCached::FontCached. So drop the unicodeMode choice at the top of FillTextLayoutFormats.

DirectWrite may be more accurate for text aspects like kerning if the number of style ranges is minimized by determining runs of each style and applying that style over the run. For example,

PrintText “العَرَبِيَّة"

may be 3 (identifier,space,string) runs: 0..9(Verdana,8.7);10..10(Verdana,9,bold);11..20(Consolas,9.4)

Neil

[Raghda Morsy‏]

I want to use an array of Fonts or FontAlias , so I can get the textFormat for each char , but I get a compilation error for this line :

stylesFonts_[stylesInLine] = vstyle.styles[ll->styles[stylesInLine]].font;

When I use Font , I get this error :

 "Font &Font::operator=(const Font &)" (declared at line 279 of "e:\Software\scintilla373\scite375\scintilla\include\Platform.h") is inaccessible
This is line 279 :

Font &operator=(const Font &);
It's protected.

And when I use FontAlias , I get another error:

"FontAlias::operator=(const FontAlias &)" (declared at line 39 of "e:\Software\scintilla373\scite375\scintilla\src\Style.h") cannot be referenced -- it is a deleted function

This is line 39 :

FontAlias &operator=(const FontAlias &) = delete;

[Raghda Morsy‏]

Here is another try using FontID :
https://drive.google.com/drive/folders/0B1_ta5eAkhnINWFSaW9VWnZheGc?usp=sharing

So the 3 previous arrays have been replaced by std::unique_ptr<FontID[]> stylesFontsIDs;

And FillLineTextFormats method , has been replaced by :
void EditView::FillLineFontsIDs(const ViewStyle &vstyle, LineLayout *ll, FontID * stylesFontsIDs)

So now void SurfaceD2D::FillTextLayoutFormats(IDWriteTextLayout * textLayout, wchar_t *buffer , int tLen , FontID * stylesFontsIDs) uses stylesFontsIDs to get pfm->pTextFormat for each character in the line.

This shows a better behavior , but I see incremental spacing in the caret position , appears in complex statements like :
PrintText("العَرَبِيَّة", Point(12.0, 36.0), len("العَرَبِيَّة"));

What do you thing is missing in caret positioning ?

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Here is another try using FontID :
> https://drive.google.com/drive/folders/0B1_ta5eAkhnINWFSaW9VWnZheGc?usp=sharing
>
> So the 3 previous arrays have been replaced by std::unique_ptr<FontID[]> stylesFontsIDs;

The Font/FontID/FontAlias code is about memory management, lifetimes, and duplicate avoidance as fonts were expensive in earlier days. The use of FontID here will likely be replaced with Font* before integration but its good for now.

> This shows a better behavior , but I see incremental spacing in the caret position , appears in complex statements like :
> PrintText("العَرَبِيَّة", Point(12.0, 36.0), len("العَرَبِيَّة"));
>
> What do you thing is missing in caret positioning ?

I can’t see just what is happening here yet. It could be rounding from float to int or some attribute of the font isn’t copied across. The caret position works OK for me until the numbers where it starts deviating until its a whole character out at the end of the line.

There is also an accuracy problem that gets worse further down the file. With identical text (either including Arabic or not including Arabic) on every line, the caret position chosen by a click may be (approximately) one character to the right for every 10 lines down the screen.

Neil

[Raghda Morsy‏]

Now the two issues of incremental spacing and the incremental caret position by lines are fixed :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIRVpxT2JpcDNmVGc?usp=sharing

But I see 2 new issues :

1.  There is a tiny decrease in caret position when [TAB] is included in the line , so the caret is drawn inside the character, so it seems that the TAB is not copied in the line style fonts , where can I find the TAB style ?
2.  The top and down arrows , are able to move the caret only in the first 2 lines !!! where can I check this issue ?

Thanks

Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Now the two issues of incremental spacing and the incremental caret position by lines are fixed :
> https://drive.google.com/drive/folders/0B1_ta5eAkhnIRVpxT2JpcDNmVGc?usp=sharing

That is much better!

> But I see 2 new issues :
> • There is a tiny decrease in caret position when [TAB] is included in the line , so the caret is drawn inside the character, so it seems that the TAB is not copied in the line style fonts , where can I find the TAB style ?

Tab stops are supposed to line up even when multiple fonts are used so tab positions are calculated from the width of a space in the default style multiplied by the number of spaces as
tabWidth = styles[STYLE_DEFAULT].spaceWidth * tabInChars;
This information will need to be communicated to the platform layer so that it can be used in the calculations. Also remember that these are tab stops, not tab widths, and the width of a particular tab depends on its starting position.

> • The top and down arrows , are able to move the caret only in the first 2 lines !!! where can I check this issue ?

The up and down arrows work (in CursorUpOrDown; PositionUpOrDown) by calculating an x,y location on the previous/next line and then correlating this with the text layout by calling SPositionFromLocation. trace through PositionUpOrDown to work out what is occurring.

I noticed another problem. Clicking at the end of the line puts the caret 1 character in from the end.

Neil

[Raghda Morsy‏]

Hi Neil,

Here are the new files , after fixing the bugs mentioned above :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIYmdiR3dJQU1zR3c?usp=sharing

For calculating the tab width in Scintilla I used the position of the tab in line , positions_[positionsInLine + 1] - positions_[positionsInLine];

Also a new method is added , to calculate tab width in textLayout :
XYPOSITION SurfaceD2D::WCharWidth(FontID fontID_, wchar_t wch)
It's almost the same as XYPOSITION SurfaceD2D::WidthChar(Font &font_, char ch) , but using FonID instead of Font and returning textMetrics.width instead of textMetrics.widthIncludingTrailingWhitespace

What else do you think is missing or needs to be added ?

Thanks
Raghda

[Neil Hodgson‏]

   Hi Raghda,

Here are the new files , after fixing the bugs mentioned above :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIYmdiR3dJQU1zR3c?usp=sharing

   OK, that has fixed the up + down arrows and the line end selection.

For calculating the tab width in Scintilla I used the position of the tab in line , positions_[positionsInLine + 1] - positions_[positionsInLine];

   The positions are being passed in for calculations but not altered so they should be a ‘const’ pointer as should the font IDs. Like this in all platform methods:

virtual int PositionFromPoint(Point pt, const char *s, int len, int width , const FontID * stylesFontsIDs, const XYPOSITION * positions_) = 0;
virtual Point XYFromPosition(const char *s, int len, int width, int caretPosition, const FontID * stylesFontsIDs , const XYPOSITION * positions_) = 0;

   There is an unnecessary second ‘pfm’ variable inside PositionFromPoint which is not used. It upsets bug checkers so should be removed.

Also a new method is added , to calculate tab width in textLayout :
XYPOSITION SurfaceD2D::WCharWidth(FontID fontID_, wchar_t wch)
It's almost the same as XYPOSITION SurfaceD2D::WidthChar(Font &font_, char ch) , but using FonID instead of Font and returning textMetrics.width instead of textMetrics.widthIncludingTrailingWhitespace 

   This picture, on the last and second last lines shows some problems with caret positioning where the caret appears part way through the tab.

   The first 2 lines show some unusual selection highlighting when they are selected: the Arabic text is shown unselected.

What else do you think is missing or needs to be added ?

   Control characters are not handled well. Open a binary file like a PNG image in SciTE and there will be many control characters in little blocks. With the R2L patches it can sometimes show a control character but the caret moves inside its area. Most of the time, opening a binary file with the R2L patch crashes.

   Neil

[Raghda Morsy‏]

Hi Neil,

Would you please check this :

https://drive.google.com/drive/folders/0B1_ta5eAkhnISHBTTkMzZTNyeVU?usp=sharing

For the tab positioning , DirectWrite has a method SetIncrementalTabStop , I used it with vsDraw.tabWidth to give the textLayout tab the same size :

textLayout->SetIncrementalTabStop(tabWidth);

And for the control chars , some new methods are added to give the text layout the same representation of each control char :

• void ReplaceRepresentation(std::wstring &buffer);
  
• int GetPositionInLayout(const char *s, int len, int position);
  
• const char * IsRepresentation(unsigned int character); 
  

But there is something I don't know why it happens , when I try control chars with the original SciTE, the representative chars are displayed with dark gray background , while in my version they are displayed with light gray !

And regarding the right to left chars selection highlight , because the right and the left positions are swapped , and the selection range intersection sees only from left to right ,

I added these lines to : static void DrawTranslucentSelection(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,

Sci::Line line, PRectangle rcLine, int subLine, Range lineRange, int xStart)

                                  //If the end of the line contains right to left chars , the last character point will not be at the rightmost

XYPOSITION rightMost = xStart + ll->positions[portion.end.Position() - posLineStart] -

static_cast<XYPOSITION>(subLineStart) + portion.end.VirtualSpace() * spaceWidth;

if ((portion.end.Position() - posLineStart) == ll->numCharsInLine

&& rightMost > rcSegment.right && rightMost > rcSegment.left)

{

rcSegment.right = rightMost;

}

//If the end of the line contains right to left chars , the first character point will not be at the leftmost

XYPOSITION leftMost = xStart + ll->positions[portion.start.Position() - posLineStart] -

static_cast<XYPOSITION>(subLineStart) + portion.start.VirtualSpace() * spaceWidth;

if ((portion.start.Position() - posLineStart) == 0

&& leftMost < rcSegment.right && leftMost < rcSegment.left)

{

rcSegment.left = leftMost;

}

Also commented the If statement :

//if (rcSegment.right > rcLine.left)

SimpleAlphaRectangle(surface, rcSegment, SelectionBackground(vsDraw, r == model.sel.Main(), model.primarySelection), alpha);

Thanks

Raghda

[Neil Hodgson‏]

   Hi Raghda,

https://drive.google.com/drive/folders/0B1_ta5eAkhnISHBTTkMzZTNyeVU?usp=sharing

For the tab positioning , DirectWrite has a method SetIncrementalTabStop , I used it with vsDraw.tabWidth to give the textLayout tab the same size :
textLayout->SetIncrementalTabStop(tabWidth);

   That appears to be working well. There could be issues with minimum tab size (tabWidthMinimumPixels=2 pixels in current Scintilla) but I haven’t seen any problem yet.

   There is an ‘explicit tabstop’ feature that allows applications to tightly define layout on a per-line basis. This is rarely used, and was introduced to allow applications to implement ‘elastic tabstops' so can probably remain unsupported by the R2L code.

http://nickgravgaard.com/elastic-tabstops/

http://www.scintilla.org/ScintillaDoc.html#SCI_CLEARTABSTOPS

And for the control chars , some new methods are added to give the text layout the same representation of each control char :

• void ReplaceRepresentation(std::wstring &buffer);

• int GetPositionInLayout(const char *s, int len, int position);

• const char * IsRepresentation(unsigned int character); 

But there is something I don't know why it happens , when I try control chars with the original SciTE, the representative chars are displayed with dark gray background , while in my version they are displayed with light gray !

   I’m not seeing this - control character blobs seem to be coloured with their style for me.

   There are some other uses of ‘blob’ representations in Scintilla - invalid bytes in UTF-8 and explicit representations.

http://www.scintilla.org/ScintillaDoc.html#CharacterRepresentations

   For now, these shouldn’t block other issues but we might need to look at these again later, perhaps passing in the representations of each byte (NULL for no-representation) or with a callback to the code that knows about which bytes need representations. For bad UTF-8, this can be seen in a file in a different encoding like this Chinese text then switching to UTF-8 interpretation.

http://www.scintilla.org/locale.zh_gb.properties

And regarding the right to left chars selection highlight , because the right and the left positions are swapped , and the selection range intersection sees only from left to right ,
I added these lines to : static void DrawTranslucentSelection(Surface *surface, const EditModel &model, const ViewStyle &vsDraw, const LineLayout *ll,

   OK.

   There is an issue where there is a doubly-selected portion near some line ends and bad caret positioning in the same locations. Select All in the test file for line 2 and 3 looking wrong: 

  Looking at the behaviour of copying and pasting portions of R2L and mixed direction text and it appears wrong. For example, mouse down before the first “t(“ on line 4 and drag rightwards 4 characters to select some L2R and one Arabic character. Copy and then paste at the end of the document and more of the Arabic characters are in the paste. Perhaps the copy code needs to change to match the selection code.

   For good code hygiene, its best to try to fix any warnings from the compiler on maximum warning level (4 for Visual C++). Some warnings are not interesting (like the SurfaceGDI stubs not using their arguments so turn off the warnings by not including identifiers in the definitions) but others may be problems - for example, the “unreferenced formal parameter” on line 683 of EditView.cxx shows that a feature of the code is turned off. In this case its the ability to match a position to characters rather than the positions between characters. This is a minor issue but is used, for example, to show calltips when pointing at an identifier or when dragging a selection. Make the text large, select a word and watch where the mouse cursor changes between I-beam (selection) and arrow (drag) with modified and original Scintilla.

   To remove warnings for the stub implementations:

int SurfaceGDI::PositionFromPoint(Point pt, const char * s, int len, int width, const FontID * stylesFontsIDs,
const int ctrlCharPadding, float tabWidth)

   change to

int SurfaceGDI::PositionFromPoint(Point, const char *, int, int, const FontID *,
const int, float)

   Neil

[Neil Hodgson‏]

Bidirectional text may only be supported on some platforms and in some modes although this will change over time as it is implemented in more platform layers. Applications will need to be able to test whether bidirectional support is available.

The current bidirectional support requires additional calls to platform APIs which take additional time as well as extra memory. Applications may not wish to pay these costs so there should be an API to control this. In the future there may be more options to, for example, cache more bidirectional information.

Attached is a proposal for a bidirectional control API. It currently has two states, SC_BIDIRECTIONAL_DISABLED and SC_BIDIRECTIONAL_ENABLED, although this could be expanded in the future. The documentation for this API is:

—
SCI_SETBIDIRECTIONAL(int bidirectional)
SCI_GETBIDIRECTIONAL → int
Some languages, like Arabic and Hebrew, are written from right to left instead of from left to right as English is. Documents that use multiple languages may contain both directions and this is termed "bidirectional". Scintilla only correctly displays bidirectional text on some platforms and there can be additional processing and storage costs to this. Currently, bidirectional text only works on Win32 using DirectWrite. As some applications may not want to pay the costs, bidirectional support must be explicitly enabled by calling SCI_SETBIDIRECTIONAL(SC_BIDIRECTIONAL_ENABLED). This should be done after setting the technology to SC_TECHNOLOGY_DIRECTWRITE, SC_TECHNOLOGY_DIRECTWRITERETAIN, or SC_TECHNOLOGY_DIRECTWRITEDC.

If the call succeeded SCI_GETBIDIRECTIONAL will return SC_BIDIRECTIONAL_ENABLED (1) otherwise SC_BIDIRECTIONAL_DISABLED (0) is returned.
—

Neil

[Florian Balmer‏]

That's great news, many thanks to everybody for their efforts, I can't wait examining these new capabilities!

BIDI support has been a frequent feature request for Notepad2, and I hope I will be able to release an update, soon.

--Florian

[Raghda Morsy‏]

Hi Neil ,

Please find the modified files here :

https://drive.google.com/drive/folders/0B1_ta5eAkhnINkZkQXNnUXNWMTA?usp=sharing

Regarding minimum tab size (tabWidthMinimumPixels=2 pixels in current Scintilla) , I couldn't catch this case so I kept it as it is.

And for the ctrl representations , high control and  invalid bytes in UTF-8 representations are added to const wchar_t * SurfaceD2D::IsRepresentation(unsigned int character) .

And for the  doubly-selected portion near some line ends and bad caret positioning in the same locations. The problem was in FillTextLayoutFormats method and fixed .

And regarding copying a mixed  direction text , actually the copy method is working well , the problem is in DrawTranslucentSelection function , because it considers the selection to always from left to right , while in the selection of mixed direction text , the left to right should be highlighted from left to right , and the right to left text should be highlighted from right to left .

So a new method is added :

virtual void FillSelectionRectangle(const char *s, int len, int width, int start , int end, const FontID * stylesFontsIDs,

const int ctrlCharPadding, float tabWidth, PRectangle rcLine, ColourDesired fill, int alpha, int xStart)

Which uses DWRITE_HIT_TEST_METRICS , and HitTestTextRange to find the selected rectangle .

At DrawTranslucentSelection the call of SimpleAlphaRectangle has been commented , because AlphaRectangle is being called inside FillSelectionRectangle .

Regarding the usage of charPosition , it is now added to 

int SurfaceD2D::PositionFromPoint(Point pt, const char * s, int len, int width, const FontID * stylesFontsIDs,

const int ctrlCharPadding,float tabWidth, bool charPosition)

and considered in the position calculations :

if (charPosition)

{

pos = isTrailingHit ? hitTestMetrics.textPosition : caretMetrics.textPosition;

}

else

{

pos = isTrailingHit ? hitTestMetrics.textPosition + hitTestMetrics.length : caretMetrics.textPosition;

}

But I couldn't test if it serves the same as its original task or not , I have tried the drag of selected text with large text in both modified original Scintilla , but I couldn't see the difference !

Please note that R2L until now is only supporting strong right to left characters with spaces , while weak characters like comma , colon , ... are still not supported .

Weak characters can be added too , but I need to know first how much can this affect lexers , for example in case of braces () , what should be the behavior of the text ?

Thanks

Raghda

[Neil Hodgson‏]

Hi Raghda,

> https://drive.google.com/drive/folders/0B1_ta5eAkhnINkZkQXNnUXNWMTA?usp=sharing

It looks like there have been some good improvements here. I’ll try to examine the changes more when I have time but here are some new issues to look at.

A new HTML example is attached which comes from the BBC’s Arabic news page source code.
http://www.bbc.com/arabic

Style changes are sometimes not seen: the <span> on the first line appears as text instead of as a tag.

Wrapped lines are a new area to explore: turn on Options | Wrap. Selection does not appear on the second visual line of the second document line.

Neil

[Neil Hodgson‏]

Hi Raghda,

> https://drive.google.com/drive/folders/0B1_ta5eAkhnINkZkQXNnUXNWMTA?usp=sharing

One problem I noticed is the use of multi-character character constants which are a non-standard extension.
else if (character == '\xe2\x80\xa8’)
Even worse, this won’t work as the compiler will decode it as bytes (so probably 0xe280a8), not as UTF-8. ‘character’ is a wchar, so UTF-16 with the value of LS = 0x2028 and PS = 0x2029.
https://en.wikipedia.org/wiki/Newline#Unicode

> And for the ctrl representations , high control and invalid bytes in UTF-8 representations are added to const wchar_t * SurfaceD2D::IsRepresentation(unsigned int character) .

Having multiple pieces of code do this could lead to problems with differing results and the code drifting apart. SurfaceD2D::IsRepresentation is quite similar to SpecialRepresentations::RepresentationFromCharacter in PositionCache, except that it takes a wchar_t instead of a UTF-8 string. A pointer to the SpecialRepresentations object could be passed in although will require some header file refactoring. I expect this can be left until later, though.

> So a new method is added :
> virtual void FillSelectionRectangle(const char *s, int len, int width, int start , int end, const FontID * stylesFontsIDs,
> const int ctrlCharPadding, float tabWidth, PRectangle rcLine, ColourDesired fill, int alpha, int xStart)
> Which uses DWRITE_HIT_TEST_METRICS , and HitTestTextRange to find the selected rectangle .

This hard-codes the particular visual representation of selection inside the platform layer when really it is a policy choice - selections could be drawn with an outline or with a gradient shade in the future instead of a simple rectangle. It should probably eventually return a vector of rectangles which are then drawn by the caller instead of calling AlphaRectangle itself. Another ‘later' issue.

> But I couldn't test if it serves the same as its original task or not , I have tried the drag of selected text with large text in both modified original Scintilla , but I couldn't see the difference !

Well, it appears better to me now. Although if you select the last character on a line, the arrow cursor persists as you move rightwards over the character into line end space.

> Please note that R2L until now is only supporting strong right to left characters with spaces , while weak characters like comma , colon , ... are still not supported .
> Weak characters can be added too , but I need to know first how much can this affect lexers , for example in case of braces () , what should be the behavior of the text ?

I don’t know - do other applications like Visual Studio act in a way that appears sensible?

Neil

[Raghda Morsy‏]

Hi Neil ,

Is there a way to find the position at which the line wrap happens ? the problem of wrapping that new functions are treating the line as if it is always straight , I think if we have the position of wrapping then the line can be divided and treated as more than 1 line .

For the issue in the html file , the problem seems in getting the correct style , specially for character ">"  with right to left chars , the problem appears only in hypertext , XML , and Ada formats , I'll try to find out why.

Regarding Visual studio and R2L , I'm using Visual studio 2015 and it's not supporting R2L  :D

Actually visual studio doesn't support normal text , so when we write normal R2L text , it is written from left to right !

Only when we type R2L in styled text , like in a comment or between quotes "" , the text gets a good manner from right to left.

I mentioned the weak chars issue to get Florian's attention about using R2L support in Notepad2 with normal text .

I have tried using the function below , but it affects the styled text very badly. If it's possible , we may can use it as an option when normal text is used .

bool CellBuffer::IsWeakChar(unsigned int character)

{

if ((character >= 0x0 && character <= 0x40) || (character >= 0x5B && character <= 0x60) ||

(character >= 0x7B && character <= 0xBF) || (character >= 0x2B9 && character <= 0x362) ||

(character >= 0x1DC0 && character <= 0x1DC9) || (character >= 0x2010 && character <= 0x20F0) ||

(character >= 0x2190 && character <= 0x23FA) || (character >= 0x2422 && character <= 0x2426) ||

(character >= 0x2440 && character <= 0x2473) || (character >= 0x24EA && character <= 0x2B59) ||

(character >= 0x2E00 && character <= 0x2E3B) || (character >= 0x2FF0 && character <= 0x301F) ||

(character >= 0x3251 && character <= 0x325F) || (character >= 0x32B1 && character <= 0x32BF) ||

(character >= 0xFE10 && character <= 0xFF6B) || (character >= 0xFF01 && character <= 0xFF20) ||

(character >= 0xFF3B && character <= 0xFF40) || (character >= 0xFF5B && character <= 0xFF64) ||

(character >= 0x1D7E2 && character <= 0x1D7FF) || (character >= 0x1F030 && character <= 0x1F0F5) ||

(character >= 0x1F300 && character <= 0x1F6FF))

return true;

return false;

}

I'll go through the rest of issues , and try to fix them .

Thanks
Raghda

[Neil Hodgson‏]

Raghda:

> Is there a way to find the position at which the line wrap happens ?

The LineLayout::lineStarts field contains the positions within the line where wraps occur.

> Actually visual studio doesn't support normal text , so when we write normal R2L text , it is written from left to right !
> Only when we type R2L in styled text , like in a comment or between quotes "" , the text gets a good manner from right to left.

OK - I’d opened files like Arabic.cxx in Visual C++ and thought it looked correct.

> I have tried using the function below , but it affects the styled text very badly. If it's possible , we may can use it as an option when normal text is used .
>
> bool CellBuffer::IsWeakChar(unsigned int character)

> {…

Shouldn’t the weak characters look at their context for their true direction using the unicode bidirectional Algorithm?

Neil

[Raghda Morsy‏]

Hi Neil ,

Would you please check the modified files here :
https://drive.google.com/drive/folders/0B1_ta5eAkhnIM210ZVFDS1o2dGs?usp=sharing

Wrapping issues have been fixed ( selection , caret position ,...) .

And the html styling with R2L issue is fixed too.

surface->FillSelectionRectangle method has been changed to take std::unique_ptr<PRectangle[]>selectedRects; as an input and fills it with the selected rectangles , then the selection filing is done at SimpleAlphaRectangle 

while (rects<stringLength)

{
SimpleAlphaRectangle(surface, selectedRects[rects],

SelectionBackground(vsDraw, r == model.sel.Main(), model.primarySelection), alpha);

rects++;

}

SurfaceD2D::IsRepresentation has been removed , and a new array is used for each line std::unique_ptr<CharArray[]> lineReprs; , where CharrArray is defined at "Platform.h" typedef const char * CharArray;

And a new function void EditView::FillLineRepresentations(const EditModel &model, LineLayout * ll, CharArray * lineReprs) is added to fill lineReprs array , using model.reprs.RepresentationFromCharacter(&ll->chars[charsInLine], charWidth);

I didn't want to play in headers , as I still don't know most of the relations among files, which can lead me to a mess, so I used lineReprs array .

Also the isRightToLeft array has been removed with all its related functions , that was created mainly for normal text with R2L , so when there is R2L text there should not be any breaks , and DirectWrite can handle it .
The problem in normal text appears while typing a R2L text then open a brace () or a quote "" , in this case the text gets crazy , and opened brace goes to the most right while it should be at the most left! , like in this :
فإذا أتت الريح (من كل مكان) , فماذا تفعل؟

When you close the brace , it goes crazy again until you type another R2L letter , then it returns back to the correct position .

That's why I wanted to use isRightToLeft and isWeakChar , then I realized that Windows Notepad has the same problem , while Microsoft Word , and google editors have fixed this issue . They fixed it by giving an option for the user to choose text direction , so when R2L direction is chosen we don't see this issue , while we can see the problem again when we choose L2R and type R2L , which is the same case of Scintilla. 

So I don't see a need for isRightToLeft or isWeakChar , as this issue can be fixed automatically by DirectWrite if there is an option for choosing text direction.

Now, what do you think ? is it close enough to be released ?

Thanks
Raghda

[Hassan DRAGA‏]

Hello, me too trying adding RTL support to Scintilla, because i am using it in wxWidgets (wxStyledTextCtrl).. and i am surprised that a coder from china (Zane U. Ji) has fixed this problem, and its on Development Release wxWidgets 3.1.0 (Github).

https://github.com/wxWidgets/wxWidgets/blob/v3.1.0/docs/changes.txt

- Update Scintilla to v3.5.5 (Christian Walther, Heyoupeng, ARATA Mizuki).

- Add wxStyledTextCtrl copy/paste text events (Christian Walther).

- Improve RTL support in wxStyledTextCtrl (Zane U. Ji).

https://github.com/ZaneUJi

[Hassan DRAGA‏]

[Neil Hodgson‏]

Raghda Morsy:

> SurfaceD2D::IsRepresentation has been removed , and a new array is used for each line std::unique_ptr<CharArray[]> lineReprs; , where CharrArray is defined at "Platform.h" typedef const char * CharArray;

Some of the code isn’t portable since it assumes that “0” is the same pointer each time in every piece of code. There are warnings like
platwin.cxx(1671): warning C4130: '!=': logical operation on address of string constant
Checking whether a const char * is pointing at a string containing “0” is more normally done with strcmp. It may be possible to use nullptr instead as it is a globally special value.

NUL characters (value 0) appear to cause some problems. Or maybe its just files with many control characters. Open a zip file or similar are try selecting characters.

There are also some assumptions that a character fits in a single wchar_t in ReplaceRepresentation, for example, but some characters take 2 wchar_t units. The function UTF16CharLength in UniConversion.h can be used to see if a wchar_t is the first half of a 2 unit character. For example, in the attached file, the 😂 is an emoji that takes 2 wchar_t units and caret movement is disrupted. It looks something like this:

Pr😂int [ESC]Text "العَرَبِيَّة"

> And a new function void EditView::FillLineRepresentations(const EditModel &model, LineLayout * ll, CharArray * lineReprs) is added to fill lineReprs array , using model.reprs.RepresentationFromCharacter(&ll->chars[charsInLine], charWidth);
> I didn't want to play in headers , as I still don't know most of the relations among files, which can lead me to a mess, so I used lineReprs array .

The three methods on Surface take very similar argument lists (const char *s, int len, …) so one aspect that can be cleaned up is to pass in a class object that encapsulate the commonality. Also hiding the implementations inside that object would be

> Also the isRightToLeft array has been removed with all its related functions , that was created mainly for normal text with R2L , so when there is R2L text there should not be any breaks , and DirectWrite can handle it .

OK, that’s a simplification ;-)

> That's why I wanted to use isRightToLeft and isWeakChar , then I realized that Windows Notepad has the same problem , while Microsoft Word , and google editors have fixed this issue . They fixed it by giving an option for the user to choose text direction , so when R2L direction is chosen we don't see this issue , while we can see the problem again when we choose L2R and type R2L , which is the same case of Scintilla.
>
> So I don't see a need for isRightToLeft or isWeakChar , as this issue can be fixed automatically by DirectWrite if there is an option for choosing text direction.

Windows has a notion of a windows preferred direction which is stored in the WS_EX_LAYOUTRTL extended style flag.
https://msdn.microsoft.com/en-us/library/windows/desktop/ff700543(v=vs.85).aspx
We could use this flag to determine a base direction which means that the application would have to set the bit.

> Now, what do you think ? is it close enough to be released ?

Since it only works on Windows in DirectWrite mode, there should be conditional code so the bidirectional code path is only used when possible and desired as mentioned in the post I wrote about a SCI_SETBIDIRECTIONAL API.

Before release, there should also be some simplification and maybe generalization of the Surface methods but that may be something for me to do. Other platforms may need some changes to the interface.

Neil

[Hassan DRAGA‏]

okay ^_^

[Raghda Morsy‏]

Hi Hassan,

R2L is not released yet , at the current version of SciTE4.0.2 you can select  Language ->Text , so you can use it for plain text , the Arabic words will be displayed from right to left , but if you try to search for a word , you will get a different one selected , or even if you select a word and past it , you will find a different one pasted.

Thanks
Raghda

بتاريخ الأحد، 29 أكتوبر، 2017 4:55:45 م UTC+2، كتب Hassan DRAGA:

okay ^_^

[Neil Hodgson‏]

Looking more deeply at the code and there are some issues.

*** Off-by-one and NULs ***

//Convert the const char * to wchar_t * to be used in textLayout
const TextWide tbuf(s, len+1 , unicodeMode, codePageText);

The purpose of the +1 here is unclear. It is including an extra char from the input. For a simple line this is often a NUL which may have little effect but for a wrapped line it could be an extra visible character if ASCII or possibly the first byte in a multi-byte UTF-8 sequence which could fail badly. TextWide does not append a NUL wchar_t itself.

The ASCII case causes the next problem with the following code:

// Replace the original control characters with their representative chars.
std::wstring buffer = L"";
buffer.append(tbuf.buffer);

Say the document line is “a b c d\n” which has been wrapped into 2 screen lines with the 2nd line starting with ‘c’. For the first line, “a b “, the TextWide will *not* be NUL terminated, instead containing uninitialized arbitrary garbage after the converted characters. Therefore, an arbitrary amount of unititialized data may be appended to buffer.

There are changes that should be made in each location with similar code.

1) The TextWide should be just for the text segment actually being processed here without the +1.
const TextWide tbuf(s, len, unicodeMode, codePageText);

2) The ‘buffer’ variable should be initialized with a length constraint so that it doesn’t read uninitialized memory.
std::wstring buffer(tbuf.buffer, tbuf.tlen);

3) Don’t subtract 1 from uses of tbuf.tlen where this is avoding the +1 from before:
//Fill the textLayout chars with their own formats
FillTextLayoutFormats(textLayout, tbuf.buffer, tbuf.tlen , stylesFontsIDs , len , repStyle , lineReprs);

Buffer size off-by-one errors are easy to create and difficult to debug. In general, don’t add NULs except for calling platform APIs that require them (and DirectWrite prefers explicit length arguments); provide lengths to string/wstring constructors as well as functions like wstring::insert.

*** Parallel arrays ***

LineLayout currently contains 3 parallel arrays for the characters, styles and positions. The R2L patch adds 2 more parallel arrays for font IDs and representations. Except that styleFontIDs has an extra initial element for the control characters font which makes it non-parallel and complicates all the uses of styleFontIDs.

Trying to decipher just what is occurring here is difficult. The control character style is now a separate argument to the 3 Surface methods, so including it in styleFontIDs is duplication. Having it as the first element in styleFontIDs requires [1] and +1 in the Surface methods then a compensating -1 in the index calculations before each call.

Neil

[Raghda Morsy‏]

Hi Neil,

Yes, using TextWide doesn't work well with all control chars, unfortunately I discovered this recently, and I'm currently changing all related code to control chars and using TextWide for segments.

And for the representation style, I have also changed all the functions inputs to take them as one object , so representation style will be passed as a property of the object , and no need for the extra element.

I will send you the changes soon

Thanks

Raghda

--
You received this message because you are subscribed to a topic in the Google Groups "scintilla-interest" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/scintilla-interest/6u5YFzkRzRE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to scintilla-inter...@googlegroups.com.
To post to this group, send email to scintilla...@googlegroups.com.
Visit this group at https://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.

[lilo ragabov‏]

Thanks Raghda for all of your hard work, it's really appreciated. Dozens of users are asking for this feature in many text editors.
If you need an extra hand in coding you can assign tasks for me and I will help.

[Raghda Morsy‏]

Thank you so much Lilo , I appreciate your support :)

> To unsubscribe from this group and all its topics, send an email to scintilla-interest+unsub...@googlegroups.com. 

[Raghda Morsy‏]

Hi Neil,

Here are the new files :

https://drive.google.com/drive/folders/1VIbL2_LueePuI8upxmSRaaAeVDx4mbYC?usp=sharing

Sorry for the delay , I caught the case you mentioned before , about tabWidthMinimumPixels when the tab has a smaller width than minimum , and in this case the tab char takes additional width = tabWidth , such space gets lost in when I use DirectWrite textLayout , so a new method is added to calculate the lost space of tabs and ctrl chars , and adjust chars positions in textLayout , float FindLostSpace(lineSharedProps sharedProbs, int pos, IDWriteTextLayout *textLayout);

And a new parameter int tabWidthMinimumPixels; is added to struct lineSharedProps . this parameter has a default value 0 , because I can't pass EditView::tabWidthMinimumPixels to surface->FillSelectionRectangle at  DrawTranslucentSelection function , so currently when we have a tab width less than minimum, the selection gets crazy , is there any way to pass the value to  surface->FillSelectionRectangle ?

Also the way of converting to wstring and replacing ctrl chars has been changed in 
std::wstring ReplaceRepresentation(const CharArray * lineReprs , int len , const char *s);

And now using UTF8DrawBytes(reinterpret_cast<const unsigned char *>(&s[l]), len - l); 

Please note , that I'm considering only UTF-8 , so if you open a PNG , rar ,... file , you need to use UTF-8 , now with the new changes I think the behavior of such files is better.

Also the extra element of stylesFontsIDs has been removed , and passes directly to lineSharedProps objects. And also no extra chars with textWide.

Thanks
Raghda

[Neil Hodgson‏]

Hi Raghda,

> Here are the new files :
> https://drive.google.com/drive/folders/1VIbL2_LueePuI8upxmSRaaAeVDx4mbYC?usp=sharing

Thanks for the update.

> Sorry for the delay , I caught the case you mentioned before , about tabWidthMinimumPixels when the tab has a smaller width than minimum , and in this case the tab char takes additional width = tabWidth , such space gets lost in when I use DirectWrite textLayout , so a new method is added to calculate the lost space of tabs and ctrl chars , and adjust chars positions in textLayout , float FindLostSpace(lineSharedProps sharedProbs, int pos, IDWriteTextLayout *textLayout);

OK.

> And a new parameter int tabWidthMinimumPixels; is added to struct lineSharedProps . this parameter has a default value 0 , because I can't pass EditView::tabWidthMinimumPixels to surface->FillSelectionRectangle at DrawTranslucentSelection function , so currently when we have a tab width less than minimum, the selection gets crazy , is there any way to pass the value to surface->FillSelectionRectangle ?

DrawTranslucentSelection is local to the EditView.cxx file and is only called in one place so just add an extra parameter “int tabWidthMinimumPixels" to its parameter list.

> Also the way of converting to wstring and replacing ctrl chars has been changed in
> std::wstring ReplaceRepresentation(const CharArray * lineReprs , int len , const char *s);
> And now using UTF8DrawBytes(reinterpret_cast<const unsigned char *>(&s[l]), len - l);
> Please note , that I'm considering only UTF-8 , so if you open a PNG , rar ,... file , you need to use UTF-8 , now with the new changes I think the behavior of such files is better.

Yes, the conditional that enables this code will have to take the encoding into account and refuse to go into R2L mode unless its UTF-8. There has been some interest in supporting Windows-1256, ISO-8859-6, or Windows-1255 but they are nowhere near as important as UTF-8.

> Also the extra element of stylesFontsIDs has been removed , and passes directly to lineSharedProps objects. And also no extra chars with textWide.

Good.

The extra space around representations appears to not always be taken into account. When the representation is in the selection its OK but when a selection is made past the representation its not. Say the text is "cat[ESC]owl” then selecting “w” draws a selection rectangle to the left of where it should be.

When there is a transition from L2R to R2L, mouse clicking can place the caret at the right of the R2L text unexpectedly. For example in this example where there is a single style and there is one run of L2R followed by a run or R2L:
//PrintText العَرَبِيَّة
Clicking the mouse in the space after “PrintText” will place the caret in various locations depending on just where it is clicked. In the left half of the space, it goes just after the “t”, but after halfway through the space it places the caret at the right of the line, then as the mouse is positioned just before the R2L text it places the caret at the left of the R2L text.

Neil

https://en.wikipedia.org/wiki/Windows-1256
https://en.wikipedia.org/wiki/ISO/IEC_8859-6
https://en.wikipedia.org/wiki/Windows-1255

[Neil Hodgson‏]

Raghda:

> https://drive.google.com/drive/folders/1VIbL2_LueePuI8upxmSRaaAeVDx4mbYC?usp=sharing

In SurfaceD2D::PositionFromPoint, there is an
unsigned int lastTabPos = buffer.find_last_of( …
then there is code that checks
if (lastTabPos >= 0 && …
Since lastTabPos is unsigned, it is always 0 or greater and this test is superfluous. The case where no tab is found sets lastTabPos to std::wstring::npos which is a huge value.

For 64-bit builds, find_last_of returns a 64-bit value and 'unsigned int’ is only 32-bits which can lead to failures. The return type of find_last_of is ‘size_t’ so this should be the declared type of lastTabPos.

Neil

[Raghda Morsy‏]

Hi Neil,

Here are the new files :

https://drive.google.com/drive/folders/1QwOXj3Ks8_a6BrS9Me0Ec9_cc2eO-Mvv?usp=sharing

The extra space in selection before or after ctrl chars and tabs has been adjusted, and the unsigned int lastTabPos is now size_t lastTabPos

 Regarding caret position when there is a transition from L2R to R2L , this is normal because the textLayout reading direction is left to right.

you can try the same line on MSWord with left to right selected, or even here in google editor

//PrintText العَرَبِيَّة 

You will see the same behavior

You can try using arrows , you you will get the same behavior , because the letter after t is <space> and after space is the Arabic letter"ا" not "ة" , so it's normal when we click the close to the end of the space , the correct position is the position of the next char which is "ا" , and because the textLayout reading direction is still Left to Right , the position of the caret is considered at the left(before) of the char , and here with the Arabic char the position before (left) of the chars is actually right of the char.

So when you close to the end of the <space> you get the caret at the right of letter "ا" , and when you click close to the start of the <space> you get the caret before the <space> with is the left of the letter"t" .

And if you click the on letter "ة" you get the end of line!

You can see it more crazy if you try to add an English letter after :D

I can add and conditional statements to adjust it , but I think if you are going to give the user an option to select R2L or L2R , then I can change textLayout to something like :

textLayout->SetFlowDirection(DWRITE_FLOW_DIRECTION_RIGHT_TO_LEFT);

textLayout->SetReadingDirection(DWRITE_READING_DIRECTION_RIGHT_TO_LEFT);

 And this option will require the same change in DrawTextCommon , and all other char insertion, and even arrows moves will need to be reconsidered from the R2L perspective.

Thanks
Raghda

[Neil Hodgson‏]

The most recent R2L implementation seems to mostly be working well.

We need now to have a plan for integrating R2L into Scintilla while preserving the current behaviour for other platforms. As the R2L implementation may cause increases in memory use and processing time, applications should only pay those costs if they opt in to R2L behaviour.

Minimize merging costs.

It is likely that the methods added to Surface for R2L will require changes for other platforms. Platform lay implementations will need to update when each change is made, even if just to provide a null implementation and this will be repeated for any additional changes in interface. As well as requiring effort to implement the methods, this makes it more difficult to move code back and forth if projects need to maintain older releases.

Propose using the preprocessor to hide (with #if) the R2L methods and calls to those methods except when actually implemented and needed. This may minimize work although it makes the interface more complex. Once R2L is implemented on multiple platforms, the methods can be finalized and the #if statements removed.

Minimize performance impact.

The R2L methods in SurfaceD2D perform significant extra processing, even when there is no R2L text on a line. While correct behaviour is a great first step, its likely that performance will become important. The R2L methods should be defined in a way that allows later optimization.

Minimize space impact.

To support the R2L methods, additional memory is used on the LineLayout object to store font IDs and character representations. Each LineLayout on x64 currently takes 104 bytes + 6 bytes per byte of text. With the R2L additions its 120 bytes + 22 bytes per byte of text (2 extra pointers). As the per-byte cost predominates, this will often use 3x as much memory for LineLayout objects than currently which will impact applications that choose higher levels of LineLayout caching to make operations like wrapping faster.

The design should allow minimizing this extra memory use in the future. One approach would be to drop the explicit access to arrays and instead provide accessor methods that could be implemented in other ways such as lazy evaluation. To this end, lineSharedProps should be an interface that hides all or most of its data.

Make FillSelectionRectangle more flexible.

The array of rectangles returned from FillSelectionRectangle should also be optimizable for both memory management and so that only the non-empty rectangles are drawn. Perhaps passing a callback object into FillSelectionRectangle like this:

class IDrawRectangle {
public:
virtual void Draw(PRectangle rc)=0;
};
//...
virtual void FillSelectionRectangle(lineSharedProps sharedProbs,int start, int end,
PRectangle rcLine, int xStart, IDrawRectangle *pDraw) = 0;
//...
pDraw->Draw(selectionRect);
//...
class DrawSelectionRectangle : public IDrawRectangle {
Surface *surface;
ColourDesired colour;
int alpha;
public:
DrawSelectionRectangle(Surface *surface_, ColourDesired colour_, int alpha_) :
surface(surface_), colour(colour_), alpha(alpha_){}
void Draw(PRectangle rc) override {
SimpleAlphaRectangle(surface, rc, colour, alpha);
}
};
//...
DrawSelectionRectangle drawSelection(surface,

SelectionBackground(vsDraw, r == model.sel.Main(), model.primarySelection), alpha);

surface->FillSelectionRectangle(props ,selectionStart ,selectionEnd , rcLine , xStart, &drawSelection);

This would allow future versions to make different trade-offs, such as accumulating rectangles into a buffer that is then optimized before drawing.

"lineSharedProps" isn't a great name. I think what the R2L methods are dealing with are "text runs" or "display lines".

Neil

[Raghda Morsy‏]

Thank you Neil,

That's great news to reach this point, and start planning for applying the integration.

How can I help in applying this plan?

Between, I agree with you lineSharedProps isn't a good name :)

Raghda

--
You received this message because you are subscribed to a topic in the Google Groups "scintilla-interest" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/scintilla-interest/6u5YFzkRzRE/unsubscribe.

To unsubscribe from this group and all its topics, send an email to scintilla-inter...@googlegroups.com.

[Neil Hodgson‏]

Raghda Morsy:

> I can add and conditional statements to adjust it , but I think if you are going to give the user an option to select R2L or L2R , then I can change …

A question here is whether this should be controlled through the Win32 API by the extended window style flags like WS_EX_LAYOUTRTL or if Scintilla should provide an API for this purpose.

In an earlier message I proposed an SCI_SETBIDIRECTIONAL API with 2 states: SC_BIDIRECTIONAL_ENABLED and SC_BIDIRECTIONAL_DISABLED. This could be changed to 3 states with SC_BIDIRECTIONAL_DISABLED, SC_BIDIRECTIONAL_L2R and SC_BIDIRECTIONAL_R2L or LTR/RTL to be similar to Win32.

If using the Win32 flags, there are several RTL flags without much help on which should be used: WS_EX_LAYOUTRTL, WS_EX_RIGHT, or WS_EX_RTLREADING.
https://msdn.microsoft.com/en-us/library/windows/desktop/ee264314(v=vs.85).aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ff700543(v=vs.85).aspx

If using the window flags then they need to be checked somewhere. Its likely that this should occur when setting up the drawing surface in ScintillaWin::WndPaint and ScintillaWin::FullPaintDC. This is a starting point to affect painting but there may be other places that need this set such as when wrapping text. Then its necessary to hold this somewhere or pass it around. For now, adding a method on Surface to set the direction is probably OK.

A Scintilla Surface object does not always have an associated window as it may be drawing on a bitmap so must not call windowing functions to determine layout direction.

Neil

[Neil Hodgson‏]

Raghda Morsy:

> How can I help in applying this plan?

There are several steps I see here.

1) Decide whether to use Win32 extended window styles or a Scintilla API to control base text direction.

2) Implement the SCI_SETBIDIRECTIONAL API to allow a runtime choice of bidirectional behaviour.

3) Implement a preprocessor symbol to enable R2L code. Could call this “ENABLE_BIDIRECTIONAL”.

4) Add initial implementation of R2L code, enabled only when both compiled with ENABLE_BIDIRECTIONAL and the runtime setting.

5) Implement base text direction control.

Neil

[Raghda Morsy‏]

2017-12-09 13:06 GMT+02:00 Neil Hodgson <nyama...@me.com>:

1) Decide whether to use Win32 extended window styles or a Scintilla API to control base text direction.

I don't think  Win32 extended window styles should be done in Scintilla , we usually use WS_EX_LAYOUTRTL in applications GUI according to end user local settings, and user preferences, so this can be used for example in SciTE when creating the window.

So the applications developers should handle this GUI part (windows layouts , text direction buttons R2L & L2R ,...)

But Scintilla should have some flags like SC_BIDIRECTIONAL_L2R and SC_BIDIRECTIONAL_R2L , and when end user selects the R2L button, Scintilla will should have the SC_BIDIRECTIONAL_R2L ON.

2) Implement the SCI_SETBIDIRECTIONAL API to allow a runtime choice of bidirectional behaviour.

We need to make changes only when SC_BIDIRECTIONAL_R2L is ON , DrawTextCommon will need to consider the new text direction with this line 

pTextLayout->SetReadingDirection(DWRITE_READING_DIRECTION_RIGHT_TO_LEFT);

And all other Surface methods using textLayout should check the SC_BIDIRECTIONAL_R2L flag

Also arrows actions right & left , and the chars array should consider the new reading direction.

3) Implement a preprocessor symbol to enable R2L code. Could call this “ENABLE_BIDIRECTIONAL”.

4) Add initial implementation of R2L code, enabled only when both compiled with ENABLE_BIDIRECTIONAL and the runtime setting.

5) Implement base text direction control.

 

If you give me the exact points of changes , I may can do the needed changes .

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> I don't think Win32 extended window styles should be done in Scintilla , we usually use WS_EX_LAYOUTRTL in applications GUI according to end user local settings, and user preferences, so this can be used for example in SciTE when creating the window.
> So the applications developers should handle this GUI part (windows layouts , text direction buttons R2L & L2R ,...)
>
> But Scintilla should have some flags like SC_BIDIRECTIONAL_L2R and SC_BIDIRECTIONAL_R2L , and when end user selects the R2L button, Scintilla will should have the SC_BIDIRECTIONAL_R2L ON.

There is also the SC_BIDIRECTIONAL_DISABLED state to avoid any R2L costs. This will also be the case for GDI since R2L is only implemented for D2D.

> If you give me the exact points of changes , I may can do the needed changes .

The work is mostly in finding and checking where to put the code. Trying to write a specification for this would be more effort.

Neil

[Neil Hodgson‏]

Attached is an updated patch that changes the SCI_SETBIDIRECTIONAL API to allow choosing a default direction.

Neil

[Neil Hodgson‏]

While working on the Platform.h and PlatWin.cxx files, there are some minor issues I’d like to progress that don’t really change the algorithms.

1) Rename “lineSharedProps” to “ScreenLine” as it represents the text and attributes of a line on screen. While the term “display line” is used more in Scintilla, it commonly means the line number, not its contents.

2) Pass the ScreenLine by const pointer to avoid copying the struct on each call and to allow passing a subclass. The ScreenLine struct will evolve into an interface/implementation pair of classes that hides much of the implementation to allow future optimization without affecting platform code.

virtual int PositionFromPoint(const ScreenLine *screenLine, Point pt, bool charPosition)=0;

3) The XYPOSITION typedef is used to allow the possibility of switching from a 32-bit float to a 64-bit float for positioning if needed. Fields on ScreenLine should use XYPOSITION instead of float.

4) The “hight” field is incorrectly spelled and should be “height”.

5) In 4 locations, tab stop numbers are calculated by dividing an x-position by the tab width and relying on implicit truncation to integer. This produces data-loss warnings so the truncation should be made explicit. Adding a TabStop method to ScreenLine makes the intent clearer:

int TabStop(XYPOSITION x) const {
return static_cast<int>(x / tabWidth);
}

6) GCC shows signed/unsigned comparison warnings in some places. These should cast some results.

for (PosInString = 0; PosInString < len && position > static_cast<int>(wBuffer.length());) {

With the bidirectional methods, calls will more often be for larger amounts of text, and an argument could be made for using size_t (which can be 64-bit) for length variables and which is more compatible with standard library classes like std::string. However, I don’t think working with lines over 2 billion bytes wide is needed and will be too much trouble to implement and test.

7) Overridden methods should be marked “override” so that better warnings can be shown if mistakes are made. So, in the declarations of SurfaceD2D and SurfaceGDI:

int PositionFromPoint(const ScreenLine *screenLine, Point pt, bool charPosition) override;

8) The Google Drive files are derived from Scintilla 3.x and there were many changes for 4.x. Changes should be based on the main development branch which is 4.x.

Attached is a work-in-progress patch that goes over the Bidi2.patch over the current repository state. It also includes using a preprocessor guard; running astyle over the changes for consistent indentation and standardizing comment formatting.

Neil

[Raghda Morsy‏]

Good morning Neil,

I will prepare visual studio 2017 to use Scintilla 4.x , and apply the changes.

Thanks
Raghda

[Raghda Morsy‏]

Hi Neil,

Sorry , it seems that I miss understood what you said.
I though you want me to apply that changes and send to you.

After adding the changes to Scintilla 4.0.2 code , I found out that you already applied them in the patch file you sent.

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Sorry , it seems that I miss understood what you said.
> I though you want me to apply that changes and send to you.
>
> After adding the changes to Scintilla 4.0.2 code , I found out that you already applied them in the patch file you sent.

Yes, the patch includes some changes and there were a few goals. To check that I haven’t changed anything incorrectly. To be the basis for any more changes.

One issue not addressed by the patch is using EditModel::bidirectional to determine the argument to SetReadingDirection which I’d like you to work on and check the resulting behaviour.

I’ve been making further small simplifications in an effort to clarify just what is needed, particularly in FindLostSpace and other code that iterates the whole string. This code has the potential to impact performance when used on long lines and where most lines will have no representations and only have tabs at the beginning. If possible, I’d like to change whole string iteration to checking just the positions of interest where there is a representation or tab, using an array of tab/representation positions instead of an element per text byte. This could also minimize the storage cost. The reason I want to address this now is that it will be part of the interface to the ScreenLine class so may require changes to multiple platform layers if done later.

Neil

[Raghda Morsy‏]

2017-12-22 23:29 GMT+02:00 Neil Hodgson <nyama...@me.com>:

   One issue not addressed by the patch is using EditModel::bidirectional to determine the argument to SetReadingDirection which I’d like you to work on and check the resulting behaviour.

 

OK , no problem , I just need SciTE GUI to support switching from L2R to R2L , no need to switch the whole window, I just need the Editor to be able to switch to right , so I can apply the rest of changes and check them.

 

   I’ve been making further small simplifications in an effort to clarify just what is needed, particularly in FindLostSpace and other code that iterates the whole string. This code has the potential to impact performance when used on long lines and where most lines will have no representations and only have tabs at the beginning. If possible, I’d like to change whole string iteration to checking just the positions of interest where there is a representation or tab, using an array of tab/representation positions instead of an element per text byte. This could also minimize the storage cost. The reason I want to address this now is that it will be part of the interface to the ScreenLine class so may require changes to multiple platform layers if done later.

Yes , I was thinking of we have 2 additional flags to LineLayout , one when we have a representation , and the other is when tab width is less than minimum .
And we can check them at LayoutLine  , so we can call FindLostSpace only when on of them is ON.

   Neil

--

You received this message because you are subscribed to a topic in the Google Groups "scintilla-interest" group.

To unsubscribe from this topic, visit https://groups.google.com/d/topic/scintilla-interest/6u5YFzkRzRE/unsubscribe.

To unsubscribe from this group and all its topics, send an email to scintilla-interest+unsub...@googlegroups.com.

To post to this group, send email to scintilla-interest@googlegroups.com.

[Neil Hodgson‏]

Raghda Morsy:

> OK , no problem , I just need SciTE GUI to support switching from L2R to R2L , no need to switch the whole window, I just need the Editor to be able to switch to right , so I can apply the rest of changes and check them.

Implement Lua scripts that call the SCI_SETBIDIRECTIONAL API and add these to the tools menu. This first requires running the scite/scripts/RegenerateSource.py script to add entries for these new APIs to SciTE.

> Yes , I was thinking of we have 2 additional flags to LineLayout , one when we have a representation , and the other is when tab width is less than minimum .
> And we can check them at LayoutLine , so we can call FindLostSpace only when on of them is ON.

Sounds good.

Neil

[Raghda Morsy‏]

Hi Neil,

Merry Christmas!

I'm a bit confused about using IDrawRectangle in FillSelectionRectangle method, will the Draw function replace the SimpleAlphaRectangle? And if the the drawing will be inside FillSelectionRectangle  why should we use IDrawRectangle  as an input?

Also I still don't know how to use Lua scripts to apply right direction in the Editor!

Thanks
Raghda

[Neil Hodgson‏]

Raghda:

> I'm a bit confused about using IDrawRectangle in FillSelectionRectangle method, will the Draw function replace the SimpleAlphaRectangle?

FillSelectionRectangle calls Draw. Draw may call SimpleAlphaRectangle. It could also draw a rounded rectangle or a shadow if that feature was added.

> And if the the drawing will be inside FillSelectionRectangle why should we use IDrawRectangle as an input?

The rectangles that match the selection have to be communicated back to the platform-independent code. One way to do this is to use a callback like IDrawRectangle. Other approaches that could be used include returning a std::vector<PRectangle>. There are small benefits in memory allocation efficiency and the amount of code rewriting to using a callback.

Your 25-11 patch allocates one rectangle for each byte of the text which is way more than needed then iterates through all the elements, even the empty ones. This is inefficient both in memory and time but, more importantly, it doesn’t logically match the operation needed which is to find the set of rectangles selected and do something with them.

Another change that could be made here is that the operation is really 1-dimensional: find the set of x-ranges (left and right values) of the selections. FillSelectionRectangle is not doing any 2-dimensional layout (as it would if it were acting on all the display lines of a document line or paragraph) so does not need the rcLine parameter. An example of how this could be useful would be if there was a desire to draw an underline under a range of text correctly taking bidirectionality into account (so possibly multiple lines drawn) and then perform tests whether the mouse is over this range.

There is actually quite some overlap between XYFromPosition and FillSelectionRectangle with the differences being that only the first hit test is returned from XYFromPosition and its for a single position rather than a range. For flexibility, its often worthwhile making APIs that take a range but may be given an 0-width range for a single position. However, returning all the hits or just one is more difficult as the callers of LocationFromPosition are only interested in a single location.

> Also I still don't know how to use Lua scripts to apply right direction in the Editor!

First run scite/scripts/RegenerateSource.py and rebuild SciTE so it knows about the new Scintilla APIs.
Then, in the Lua Startup Script add some functions like:

function BidiOn()
editor.Bidirectional = SC_BIDIRECTIONAL_L2R
end

function BidiOff()
editor.Bidirectional = SC_BIDIRECTIONAL_DISABLED
end

Then add commands to the Tools menu by adding to the User Options File:

command.30.*=BidiOn()
command.name.30.*=Bidi On
command.mode.30.*=subsystem:immediate,savebefore:no

command.31.*=BidiOff()
command.name.31.*=Bidi Off
command.mode.31.*=subsystem:immediate,savebefore:no

Neil

[Neil Hodgson‏]

Me:

> Another change that could be made here is that the operation is really 1-dimensional: find the set of x-ranges (left and right values) of the selections. FillSelectionRectangle is not doing any 2-dimensional layout (as it would if it were acting on all the display lines of a document line or paragraph) so does not need the rcLine parameter.

Experimented with this a little more. Having both XYFromPosition and FillSelectionRectangle operate just in x-positions relative to the start of the text with no knowledge of scroll position / margins or vertical positioning appears clearer to me. Also renamed IDrawRectangle / Draw / FillSelectionRectangle to IAddInterval / Add / FindSelectionIntervals as “Draw” is overly specific.

struct Interval {
XYPOSITION left;
XYPOSITION right;
};

class IAddInterval {
public:
virtual void Add(Interval interval)=0;
};

class Surface {
virtual void FindSelectionIntervals(const ScreenLine *screenLine, int start, int end, IAddInterval *pAdd)=0;
}

class DrawSelectionRectangle : public IAddInterval {
Surface *surface;
PRectangle rcLine;
int xStart;

ColourDesired colour;
int alpha;
public:

DrawSelectionRectangle(Surface *surface_, PRectangle rcLine_, int xStart_, ColourDesired colour_, int alpha_) :
surface(surface_), rcLine(rcLine_), xStart(xStart_), colour(colour_), alpha(alpha_){}
void Add(Interval interval) override {
PRectangle rc(interval.left + xStart, rcLine.top, interval.right + xStart, rcLine.bottom);
SimpleAlphaRectangle(surface, rc, colour, alpha);
}
};

Neil

[Raghda Morsy‏]

Hi Neil,

I was trying the new enumeration EditModel::Bidirectional , so when bidiR2L is selected the user will be able to type from right to left.

so at void SurfaceD2D::DrawTextCommon , I use pTextFormat->SetReadingDirection(DWRITE_READING_DIRECTION_RIGHT_TO_LEFT);

SetReadingDirection needs to see the whole line rectangle to move to the right or the left .

So after adding this to void EditView::DrawForeground , the text moves to the right.

if (model.bidirectional == EditModel::Bidirectional::bidiR2L)

{
rcSegment.right = rcLine.right - rcSegment.left;
rcSegment.left = rcLine.left;

}

But this is working with me only in wrap mode , because the scroll is still thinking from the left , even when I change the scroll to the right, and the whole interface of SciTE to the right, the scroll insists to move to the left!

So where is the horizontal scrolling is being processed!

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> But this is working with me only in wrap mode , because the scroll is still thinking from the left , even when I change the scroll to the right, and the whole interface of SciTE to the right, the scroll insists to move to the left!
>
> So where is the horizontal scrolling is being processed!

If you want the place where the user is scrolling the window horizontally, then Editor::HorizontalScrollTo is the core method and its main job is to change the value of the xOffset field. There are a few other places where xOffset is set if you need to cover all cases.

Neil

[Neil Hodgson‏]

The bidi code changes to SurfaceD2D assume a particular type of text representation is always used. When representations were extended, bitmap representations were mentioned as a possible addition.
http://www.scintilla.org/CharacterImage.png
I’d like to make this type of change easier by removing most representation code from SurfaceD2D in the bidi patch and replace this with information on the pixel width of each representation. Then, use the DirectWrite ‘inline object’ feature to add an inline object for each representation that tells DirectWrite how wide the representation is. Then DirectWrite will take this into account when doing hit tests.

This technique can not be directly used for tabs as inline objects don’t know where they will be drawn and the tab stop calculation needs a starting point. However, there could be a two stage implementation where the tab stops are calculated then added as inline objects.

An inline object has to implement the IDWriteInlineObject interface but with mostly empty or boring methods except for GetMetrics:

class BlobInline : public IDWriteInlineObject {
XYPOSITION width;
//…
}
HRESULT STDMETHODCALLTYPE BlobInline::GetMetrics(
__out DWRITE_INLINE_OBJECT_METRICS* metrics
) {
metrics->width = width;
metrics->height = 2;
metrics->baseline = 1;
metrics->supportsSideways = FALSE;
return S_OK;
}

Then, in FillTextLayoutFormats, an object of this type is allocated and set as the inline object for a piece of text in the text layout like so:

std::vector<BlobInline> blobs;
//…
if (screenLine->widthReprs[charStart] > 0.0f) {
blobs.push_back(BlobInline(screenLine->widthReprs[charStart]));
textLayout->SetInlineObject(&blobs.back(), textRange);
}

Almost all of the current representation code can be removed after this.

A patch that implements this technique is attached although it has strayed from other sources so I’ll have to clean it up a bit before merging. It currently allocates a floating point width for each byte in the ScreenLine but this should be changed to only have one width for each representation (possibly an array of offset;width) to use less memory.

Also attached a copy of PlatWin.cxx after the patch as that is easier to read.

Neil

[Neil Hodgson‏]

The 25/11 R2L patch doesn’t handle rectangular selection well. in SciTE, open a common L2R file like the Global or User options and start a rectangular selection with Shift+Alt+Right and then Shift+Alt+Down. The wrong text is selected compared to the released SciTE. If the margins are turned off this doesn’t occur so somewhere the margins aren’t being taken into account.

The ‘inline object’ patch I sent has a memory move bug if there are many representations on a line as the vector holding the blobs may need to be reallocated if it doesn’t start with enough elements. The solution is to reserve enough elements at the top of FillTextLayoutFormats:

void SurfaceD2D::FillTextLayoutFormats(const ScreenLine *screenLine, IDWriteTextLayout *textLayout, std::vector<BlobInline> &blobs) {
// Reserve enough entries up front so they are not moved and the pointers handed
// to textLayout remain valid.
const ptrdiff_t numRepresentations = std::count_if(screenLine->widthReprs, screenLine->widthReprs+screenLine->len,
[](XYPOSITION w) {return w > 0.0f; });
blobs.reserve(numRepresentations);

Neil

[Neil Hodgson‏]

Me:

> The 25/11 R2L patch doesn’t handle rectangular selection well. in SciTE, open a common L2R file like the Global or User options and start a rectangular selection with Shift+Alt+Right and then Shift+Alt+Down. The wrong text is selected compared to the released SciTE. If the margins are turned off this doesn’t occur so somewhere the margins aren’t being taken into account.

Found this one: its in EditView::LocationFromPosition and it needs to take account of margins and scrolling like:

pt = surface->XYFromPosition(&props, caretPosition);
pt.x += vs.textStart - model.xOffset;

To make the absence of any Y coordinate calculation more explicit, I’d like to change the new Surface methods to take simple values instead of Points and rename them a little:

virtual int PositionFromX(const ScreenLine *screenLine, XYPOSITION xDistance, bool charPosition)=0;
virtual XYPOSITION XFromPosition(const ScreenLine *screenLine, int caretPosition)=0;

Neil

[Raghda Morsy‏]

Hi Neil,

Do you want me to fix the rectangular selection issue with R2L? or you already fixed it?

If you want me to work on it, should I wait for the rest of changes you are currently doing?

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Do you want me to fix the rectangular selection issue with R2L? or you already fixed it?

A fix was in my previous message:

Inside EditView::LocationFromPosition, allow for margins and scrolling:

pt.x += vs.textStart - model.xOffset;

You should make this change in any files you are working on and ensure it is in any new patches you publish.

The major outstanding topic I’m hoping you will address is using SetReadingDirection and any consequential changes. This is an area I don’t understand much at all and it may affect the design quite strongly.

Attached are some patches. The first BidiBase6424.patch is a refinement of the patch that introduces SCI_SETBIDIRECTIONAL to mark it as ‘Provisional’ which means it may change and that clients can opt out of seeing it with SCI_DISABLE_PROVISIONAL. The other BidiPreProc.patch adds an ENABLE_BIDIRECTIONAL command line flag that controls enabling bidirectional APIs when building with make or nmake.

Unless there are objections, I plan on committing these patches tomorrow.

Neil

[Neil Hodgson‏]

Committed initial change sets as
https://sourceforge.net/p/scintilla/code/ci/73343682cbda0937c5427ee45ea6c9104d97ac1e/
https://sourceforge.net/p/scintilla/code/ci/0f7f5a382dbb91c7e752cfeb3ee375d679096e7e/

Neil

[Raghda Morsy‏]

Hi Neil,

void SurfaceD2D::FillTextLayoutFormats(const ScreenLine *screenLine, IDWriteTextLayout *textLayout, std::vector<BlobInline> &blobs) works fine if we use only 1 chare per line, but failing with access violation error when the has more than 1 ctrl char .

 if (screenLine->widthReprs[charStart] > 0.0f) { 

                blobs.push_back(BlobInline(screenLine->widthReprs[charStart])); 

                textLayout->SetInlineObject(&blobs.back(), textRange); 

        } 

Please find the attached "Arabic2.cxx" file as an example.

I didn't do any changes BlobInline class, or in FillTextLayoutFormats method. I just used the "PlatWin.cxx" you sent before.
Please find the attached "PlatWin.cxx" file I'm using, it has other changes but in different functions related to DrawTextCommon

Thanks
Raghda

[Neil Hodgson‏]

Hi Raghda,

> void SurfaceD2D::FillTextLayoutFormats(const ScreenLine *screenLine, IDWriteTextLayout *textLayout, std::vector<BlobInline> &blobs) works fine if we use only 1 chare per line, but failing with access violation error when the has more than 1 ctrl char .

There is a memory bug in the initial version I posted on January 5 so I posted another message on January 6 which reserves as much memory as needed for all the control characters. It looks like this change isn’t in your PlatWin.cxx. At the start of FillTextLayoutFormats count the representations and allocate space for them all:

// Reserve enough entries up front so they are not moved and the pointers handed
// to textLayout remain valid.
const ptrdiff_t numRepresentations = std::count_if(screenLine->widthReprs, screenLine->widthReprs+screenLine->len,
[](XYPOSITION w) {return w > 0.0f; });
blobs.reserve(numRepresentations);

I haven’t managed to get your version to crash with the example file but this may be due to using different releases of Visual C++ or just memory allocation randomness.

Neil

[Raghda Morsy‏]

Thank you Neil, the post of January the 6th fixed the issue.

[Neil Hodgson‏]

Tabs can also be represented with inline objects and this allows IDWriteTextLayout to be relied upon more and the complex logic in FindLostSpace avoided. There is a bit of a trick to this as IDWriteTextLayout uses the incremental tab stop value even when an inline object is used for a tab, so tabs are replaced by another character (‘X’) before the layout is created.

Thus, ReplaceRepresentation becomes:

std::wstring SurfaceD2D::ReplaceRepresentation(int len, const char *s) {
const TextWide wideText(s, len, unicodeMode, codePageText);
std::wstring ws(wideText.buffer, wideText.tlen);
std::replace(ws.begin(), ws.end(), L'\t', L'X');
return ws;
}

FillTextLayoutFormats changes to treat ‘\t’ specially by calculating the tab width and adding a BlobInline for that width:

void SurfaceD2D::FillTextLayoutFormats(const ScreenLine *screenLine, IDWriteTextLayout *textLayout, std::vector<BlobInline> &blobs) {

// Reserve enough entries up front so they are not moved and the pointers handed
// to textLayout remain valid.

const ptrdiff_t numRepresentations = std::count_if(screenLine->widthReprs, screenLine->widthReprs + screenLine->len,

[](XYPOSITION w) {return w > 0.0f; });

const ptrdiff_t numTabs = std::count(screenLine->s, screenLine->s + screenLine->len, '\t');
blobs.reserve(numRepresentations + numTabs);

std::wstring wBuffer;

for (int l = 0; l < screenLine->len;) {
const int charStart = l;
const int charBytes = UTF8DrawBytes(reinterpret_cast<const unsigned char *>(&screenLine->s[l]), screenLine->len - l);
l += charBytes;

const TextWide wideChar(&screenLine->s[charStart], charBytes, unicodeMode, codePageText);
std::wstring charBuf(wideChar.buffer, wideChar.tlen);

DWRITE_TEXT_RANGE textRange;
textRange.length = static_cast<UINT32>(charBuf.length());
textRange.startPosition = static_cast<UINT32>(wBuffer.length());

if (screenLine->widthReprs[charStart] > 0.0f) {
blobs.push_back(BlobInline(screenLine->widthReprs[charStart]));
textLayout->SetInlineObject(&blobs.back(), textRange);

} else if (screenLine->s[charStart] == '\t') {

Point realPt;
DWRITE_HIT_TEST_METRICS realCaretMetrics;
textLayout->HitTestTextPosition(
static_cast<UINT32>(wBuffer.length()),
false, // trailing if false, else leading edge
&realPt.x,
&realPt.y,
&realCaretMetrics
);

const XYPOSITION nextTab = (static_cast<int>((realPt.x + screenLine->tabWidthMinimumPixels) / screenLine->tabWidth) + 1) * screenLine->tabWidth;
const XYPOSITION widthTab = nextTab - realPt.x;

blobs.push_back(BlobInline(widthTab));
textLayout->SetInlineObject(&blobs.back(), textRange);
}

FormatAndMetrics *pfm =
static_cast<FormatAndMetrics *>(screenLine->stylesFontsIDs[charStart]);

const unsigned int fontFamilyNameSize = pfm->pTextFormat->GetFontFamilyNameLength();
std::vector<WCHAR> fontFamilyName(fontFamilyNameSize + 1);

pfm->pTextFormat->GetFontFamilyName(fontFamilyName.data(), fontFamilyNameSize + 1);
textLayout->SetFontFamilyName(fontFamilyName.data(), textRange);

textLayout->SetFontSize(pfm->pTextFormat->GetFontSize(), textRange);
textLayout->SetFontWeight(pfm->pTextFormat->GetFontWeight(), textRange);
textLayout->SetFontStyle(pfm->pTextFormat->GetFontStyle(), textRange);

const unsigned int localNameSize = pfm->pTextFormat->GetLocaleNameLength();
std::vector<WCHAR> localName(localNameSize + 1);

pfm->pTextFormat->GetLocaleName(localName.data(), localNameSize);
textLayout->SetLocaleName(localName.data(), textRange);

textLayout->SetFontStretch(pfm->pTextFormat->GetFontStretch(), textRange);

IDWriteFontCollection *fontCollection;
pfm->pTextFormat->GetFontCollection(&fontCollection);
textLayout->SetFontCollection(fontCollection, textRange);

wBuffer.append(charBuf);
}

}

FindLostSpace doesn’t need to do anything now although that should then result in collapsing the logic that currently calls it:

float SurfaceD2D::FindLostSpace(const ScreenLine *, int, IDWriteTextLayout *) {
return 0.0f;
}

A small change is also needed in EditView::FillLineRepresentations. Since tabs have a representation “HT” that is never actually seen, they were creating representation width entries which weren’t correct but were previously ignored. These are now filtered out. The change is the line that includes ‘\t’.

void EditView::FillLineRepresentations(const EditModel &model, LineLayout * ll, XYPOSITION *widthReprs) {

for (int charsInLine = 0; charsInLine < ll->numCharsInLine; charsInLine++) {
const int charWidth = UTF8DrawBytes(reinterpret_cast<unsigned char *>(&ll->chars[charsInLine]), ll->numCharsInLine - charsInLine);
const Representation *repr = model.reprs.RepresentationFromCharacter(&ll->chars[charsInLine], charWidth);

widthReprs[charsInLine] = 0.0f;
if (repr && ll->chars[charsInLine] != '\t') {
widthReprs[charsInLine] = ll->positions[charsInLine + charWidth] - ll->positions[charsInLine];
}
if (charWidth > 1) {
for (int c = 1; c < charWidth; c++) {
charsInLine++;
widthReprs[charsInLine] = 0.0f;
}
}
}
widthReprs[ll->numCharsInLine] = 0.0f;

}

Updated files are attached.

Neil

[Raghda Morsy‏]

Hi Neil,

Giving tab a representation will make it easy, because I was thinking how to switch tab arrow direction from left to right.

But what about 
void SpecialRepresentations::SetRepresentation(const char *charBytes, const char *value)

and

void Editor::SetRepresentations()

If you check this screenshot https://prnt.sc/i9bmes you can see that void EditView::DrawForeground is still drawing tab in the usual way, so cursor position will not be calculated correctly when ReplaceRepresentation replaces "\t" by "X" , while the actual text on the screen is "\t".

Thanks
Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Giving tab a representation will make it easy, because I was thinking how to switch tab arrow direction from left to right.
> But what about
> void SpecialRepresentations::SetRepresentation(const char *charBytes, const char *value)
> and
> void Editor::SetRepresentations()

It would be possible to enhance SpecialRepresentations to add different sort of visuals so that tab could be represented by an arrow. Its probably more work than special-casing tabs as is done currently.

> If you check this screenshot https://prnt.sc/i9bmes you can see that void EditView::DrawForeground is still drawing tab in the usual way,

Are you concerned about drawing the arrow head on the wrong side? Or does the calculation need to run in reverse so that its measuring from the RHS? I thought that an R2L window would invert its coordinate space so 0 was on the right - then the tab calculation works the same as an L2R window.

Neil

[Raghda Morsy‏]

I want the arrow to be in the correct direction and the calculations need to be in reverse, because not all of the applications use right to left layout and they allow right to left reading direction. 

If we will consider applications layout, then we will have more cases , when layout is right, and reading direction is left, and the opposite case with left layout and right reading direction.

So l'm leaving GUI layout as the responsibility of applications developers.

We will follow constant situation with coordinates starting from top left as zero point, with reading directions from left to right and from right to left.

When applications have right to left layout , the developers will consider  that Scintilla calculates coordinates from left to right .

That's the way I'm working on, if you don't agree I will consider the other cases.

Actually DirectWrite doesn't have any problem in switching from right to left or left to right with any layout, it just needs the full line rectangle, and it does the needed work, but the problem is in text segmentation, I already fixed the cases with pure characters, and wth ctrl chars, still facing a problem with end of line chars position and with tab arrow.

For tab arrow I wanted to suggest using a line without arrow in bidiR2L

--
You received this message because you are subscribed to a topic in the Google Groups "scintilla-interest" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/scintilla-interest/6u5YFzkRzRE/unsubscribe.

To unsubscribe from this group and all its topics, send an email to scintilla-inter...@googlegroups.com.
To post to this group, send email to scintilla...@googlegroups.com.

[Neil Hodgson‏]

Hi Raghda,

> For tab arrow I wanted to suggest using a line without arrow in bidiR2L

That is SCI_SETTABDRAWMODE(SCTD_STRIKEOUT).

file:///Users/neil/merc/scintilla/doc/ScintillaDoc.html#SCI_SETTABDRAWMODE

> I want the arrow to be in the correct direction

Here’s a version of the function that does this that takes a boolean headOnRight argument. If all the tabs in a window draw in the same direction then this should be a state on ViewStyle.

static void DrawTabArrow(Surface *surface, PRectangle rcTab, int ymid, const ViewStyle &vsDraw, bool headOnRight) {
if ((rcTab.left + 2) < (rcTab.right - 1))
surface->MoveTo(static_cast<int>(rcTab.left) + 2, ymid);
else
surface->MoveTo(static_cast<int>(rcTab.right) - 1, ymid);
surface->LineTo(static_cast<int>(rcTab.right) - 1, ymid);

// Draw the arrow head if needed
if (vsDraw.tabDrawMode == tdLongArrow) {
int ydiff = static_cast<int>(rcTab.bottom - rcTab.top) / 2;
const int xpoint = static_cast<int>(headOnRight ? (rcTab.right - 1) : (rcTab.left + 1));
int xhead = xpoint + (headOnRight ? -ydiff : ydiff);
if (headOnRight) {
if (xhead <= rcTab.left) {
xhead = static_cast<int>(rcTab.left) - 1;
ydiff = xpoint - xhead;
}
} else {
if (xhead >= rcTab.right) {
xhead = static_cast<int>(rcTab.right) + 1;
ydiff = xhead - xpoint;
}
}
surface->MoveTo(xpoint, ymid);
surface->LineTo(xhead, ymid - ydiff);
surface->MoveTo(xpoint, ymid);
surface->LineTo(xhead, ymid + ydiff);

}
}

> and the calculations need to be in reverse, because not all of the applications use right to left layout and they allow right to left reading direction.

Direct2D allows changing the coordinate system with an ID2D1RenderTarget::SetTransform call. If that works, it allows much of the drawing code to stay the same. If the current coordinate system is used with each piece of drawing code responsible for subtracting positions from the width then there are going to be many fragile changes in the code.

Neil

[Neil Hodgson‏]

> Direct2D allows changing the coordinate system with an ID2D1RenderTarget::SetTransform call. If that works, it allows much of the drawing code to stay the same. If the current coordinate system is used with each piece of drawing code responsible for subtracting positions from the width then there are going to be many fragile changes in the code.

Here’s how to ask Direct2D to invert the X axis:

diff -r 431b814a54a6 win32/ScintillaWin.cxx
--- a/win32/ScintillaWin.cxx Fri Feb 02 14:34:55 2018 +1100
+++ b/win32/ScintillaWin.cxx Sat Feb 03 11:56:52 2018 +1100
@@ -544,6 +544,8 @@
HRESULT hr = pD2DFactory->CreateDCRenderTarget(&drtp, &pDCRT);
if (SUCCEEDED(hr)) {
pRenderTarget = pDCRT;
+ D2D1_MATRIX_3X2_F transform = D2D1::Matrix3x2F::Scale(-1.0f, 1.0f, D2D1::Point2F(size.width / 2, 0.0f));
+ pRenderTarget->SetTransform(transform);
} else {
Platform::DebugPrintf("Failed CreateDCRenderTarget 0x%x\n", hr);
pRenderTarget = NULL;
@@ -560,6 +562,8 @@
HRESULT hr = pD2DFactory->CreateHwndRenderTarget(drtp, dhrtp, &pHwndRenderTarget);
if (SUCCEEDED(hr)) {
pRenderTarget = pHwndRenderTarget;
+ D2D1_MATRIX_3X2_F transform = D2D1::Matrix3x2F::Scale(-1.0f, 1.0f, D2D1::Point2F(size.width / 2, 0.0f));
+ pRenderTarget->SetTransform(transform);
} else {
Platform::DebugPrintf("Failed CreateHwndRenderTarget 0x%x\n", hr);
pRenderTarget = NULL;

Neil

[Raghda Morsy‏]

Thank you for the new static void DrawTabArrow(Surface *surface, PRectangle rcTab, int ymid, const ViewStyle &vsDraw, bool headOnRight) , it's drawing the arrow as expected http://prntscr.com/i9ijj7 .

Unfortunately, scaling RenderTarget with negative x , draws the text as if in a mirror  http://prntscr.com/i9ifss 

[Raghda Morsy‏]

Hi Neil,

Would you please check the modified files https://drive.google.com/drive/folders/1Jb5j9mpH56niUnayCfRFrRdSXqT-N5WD?usp=sharing

The changes are mainly dependent on line width on the screen, using GetTextRectangle() , so SetReadingDirection can know the right and the left.

SetReadingDirection is used only once in DrawTextCommon 

if (BidiR2L)

{

pTextFormat->SetReadingDirection(DWRITE_READING_DIRECTION_RIGHT_TO_LEFT);

}

else

{

pTextFormat->SetReadingDirection(DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);

}

Any other function using pTextFormat will take the direction assigned in DrawTextCommon .

Please note that bidiR2L will be useful in normal text for editing R2L languages, and maybe latter when we have programming languages from right to left :)

For people who are used to read text from left to right , bidiR2L mode will be strange for them, and if they try bidiR2L with normal text they will see that weak chars are treated as R2L chars.

Thanks

Raghda

[Neil Hodgson‏]

Hi Raghda,

> Would you please check the modified files https://drive.google.com/drive/folders/1Jb5j9mpH56niUnayCfRFrRdSXqT-N5WD?usp=sharing

This directory should include the changed PositionCache.[h,cxx] files as including all changed files makes it easier for others to work on these changes.

> The changes are mainly dependent on line width on the screen, using GetTextRectangle() , so SetReadingDirection can know the right and the left.

OK, that is progressing but that also means some new problems.

For R2L mode It looks like the margins are being counted twice: there is an equivalent white space on the right hand side. Flipping the margins over to the right may have its own complexities, like left-right mirroring the folding shapes.

Some other features are misplaced in this mode. Turn on virtual space (in SciTE: virtual.space=3) and right arrow past the line end and extra carets appear going the wrong way in the text and they don’t get redrawn leading to garbage. Indicators like underlines aren’t flipped - in SciTE, turn on ‘highlight.current.word’ to see the effect.

It appears some of the code is incomplete: there is a change to the signature of PointInSelection to take a PRectangle (the text rectangle) but then that argument is not used.

> SetReadingDirection is used only once in DrawTextCommon
>
> if (BidiR2L)
> {

If it simplifies the code then it may be advantageous to add a bidiR2L field to SurfaceD2D instead of passing it into each call. Perhaps Platform gets a SetTextDirection method. Any drawing surface will only be used for one text drawing direction and surfaces will be recreated when needed to ensure this.

> For people who are used to read text from left to right , bidiR2L mode will be strange for them, and if they try bidiR2L with normal text they will see that weak chars are treated as R2L chars.

Yes, quite weird.

Neil

[Raghda Morsy‏]

Hi Neil,

Here are "PositionCache.cxx" and "PositionCache.h" files , actually I didn't change them.

For duplicate margines, do you suggest to remove vs.textStart from calculations of bidiR2L ?

What is virtual space? and how to change it ? should I use SetVirtualSpace(3)? Sorry I don't know where to find "virtual.space" in SciTE .

Is DrawMarkUnderline the only responsible function for drawing underlines ?

Thanks
Raghda

[Raghda Morsy‏]

Also how to enable underline ?

[Neil Hodgson‏]

Hi Raghda,

> Here are "PositionCache.cxx" and "PositionCache.h" files , actually I didn't change them.

They are the same as some I sent you but they differ from your previous upload and from any releases or the repository. Its difficult to keep things straight in a long development process so its best to include everything changed each time. Or publish a patch file from some known baseline like a numbered Scintilla release.

> For duplicate margines, do you suggest to remove vs.textStart from calculations of bidiR2L ?

I don’t know what the correct thing to do is. The big question is: Should the folding, line number, and symbol margins move from the left to the right side when in R2L mode?

> What is virtual space? and how to change it ? should I use SetVirtualSpace(3)? Sorry I don't know where to find "virtual.space" in SciTE .

Virtual space is the ability to move the caret past the end of the line but to only have that filled in with spaces if you type.
https://blogs.msdn.microsoft.com/zainnab/2010/02/28/understanding-virtual-space/
When I mention a SciTE setting like “virtual.space” its something you can change in one of SciTE’s property files (the user properties file is normally best) to change SciTE’s behaviour.
http://www.scintilla.org/SciTEDoc.html#property-virtual.space

> Is DrawMarkUnderline the only responsible function for drawing underlines ?

No, that’s for a marker which is over a whole line. Start with DrawIndicators for underlines and other indicators.

> Also how to enable underline ?

There are several features that show indicators like underline. The most used one is word highlighting where other occurrences of the word where the caret is are highlighted with an indicator.

For example, set these for a thick violet underline beneath each instance of the word under the caret.

highlight.current.word=1
highlight.current.word.indicator=style:compositionthick,colour:#7000CF

http://www.scintilla.org/SciTEDoc.html#IndicatorProperty

Neil

[Raghda Morsy‏]

Thank you for the explanation, I'll go through all the missing features to switch them to the right.

Regarding margins of folding , line numbers ,.. I think they should be switched to right too, but first I want to see SciTE behavior when they are at the right.

Where are those margins created ? I was thinking they are part of the editor, so I tried to change the editor layout to the right , but the margins stayed at the left!

Thanks

Raghda

[Neil Hodgson‏]

Hi Raghda,

> Regarding margins of folding , line numbers ,.. I think they should be switched to right too, but first I want to see SciTE behavior when they are at the right.
> Where are those margins created ? I was thinking they are part of the editor, so I tried to change the editor layout to the right , but the margins stayed at the left!

On Windows, the margins are part of the editor window and they are always drawn on the left. Scintilla doesn’t look at the window flags responsible for layout. They are elements of the ViewStyle object and are created in its initialization but are then altered by the application.

This positioning is implicit in much of the drawing and event code so switching to the right would require finding all this code and modifying it to be sensitive to layout direction.

Neil

[Raghda Morsy‏]

Hi Neil,

Actually  I tried to create the editor window with layout from the right http://prntscr.com/ii19ku at void SciTEWin::Creation()

           wEditor.SetID(::CreateWindowEx(

              /*0*/WS_EX_LAYOUTRTL/*|WS_EX_RTLREADING|WS_EX_RIGHTSCROLLBAR| WS_EX_RIGHT*/,

              TEXT("Scintilla"),

              TEXT("Source"),

              WS_CHILD | WS_VSCROLL | WS_HSCROLL | WS_CLIPCHILDREN | WS_CLIPSIBLINGS,

              0, 0,

              100, 100,

              HwndOf(wContent),

              HmenuID(IDM_SRCWIN),

              hInstance,

              0));

And even tried to create SciTE window with layout from the right  http://prntscr.com/ii17vq at void SciTEWin::CreateUI()

     wSciTE = ::CreateWindowEx(/*0*/WS_EX_LAYOUTRTL/* | WS_EX_RTLREADING | WS_EX_RIGHTSCROLLBAR | WS_EX_RIGHT*/,

             className,

             windowName.c_str(),

             WS_CAPTION | WS_SYSMENU | WS_THICKFRAME |

             WS_MINIMIZEBOX | WS_MAXIMIZEBOX |

             WS_CLIPCHILDREN ,

             left, top, width, height,

             NULL,

             NULL,

             hInstance,

             this);

In both cases the margins are still at the left!

Anyway, if the margins are following the window layout somewhere in SciTE not in Scintilla , then I shouldn't use xStart in calculating text rectangle, but maybe we should give the application developer someway to decide if he has a right to left layout to consider the text start position (like xStart) in bidiR2L case.

Thanks

Raghda

[Neil Hodgson‏]

Raghda Morsy:

> Anyway, if the margins are following the window layout somewhere in SciTE not in Scintilla ,

The margins are not currently following any window layout.

> then I shouldn't use xStart in calculating text rectangle, but maybe we should give the application developer someway to decide if he has a right to left layout to consider the text start position (like xStart) in bidiR2L case.

There could be a separate flag or it could be implied by the text reading direction.

I don’t think it is worthwhile allowing each individual margin to be assigned either to the left or right hand side as that has been examined before and would require a large amount of work.

Neil

[Raghda Morsy‏]

Hi Neil,

Here are the files that include all changes related to R2L support :

https://drive.google.com/drive/folders/1Cd2VQzjAsXwm-lEu4Iu1Ss1GWmT2zWsh?usp=sharing

I ignored xStart in case of bidiR2L, so the text will be written from the right without any margins.

The variable bool bidiR2L; has been added to class SurfaceD2D , I just give it value true in the constructor SurfaceD2D(); and give bidirectional value Bidirectional::bidiR2L ; in EditModel::EditModel() 

Now the virtual space and indicators can be used in bidiR2L mode.

Thanks

Raghda

[Neil Hodgson‏]

Hi Raghda,

> https://drive.google.com/drive/folders/1Cd2VQzjAsXwm-lEu4Iu1Ss1GWmT2zWsh?usp=sharing

There’s a compile problem with Editor::SPositionFromLocation when ENABLE_BIDIRECTIONAL is off since it still needs rcClient. The simplest fix is to remove the ENABLE_BIDIRECTIONAL guard around the declaration of rcClient. Its a little less efficient but that's OK for now.

> I ignored xStart in case of bidiR2L, so the text will be written from the right without any margins.

OK.

> The variable bool bidiR2L; has been added to class SurfaceD2D , I just give it value true in the constructor SurfaceD2D(); and give bidirectional value Bidirectional::bidiR2L ; in EditModel::EditModel()

The Surface bidi setting should really be set on Surface creation like the attached SetDirection.patch.

> Now the virtual space and indicators can be used in bidiR2L mode.

They work when a line is a single style.
http://www.scintilla.org/227-1.png
But with mixed styles, it gets confused.
http://www.scintilla.org/227-2.png

If anyone else wants to try these changed files, it looks like revision #6426 was their origin so retrieve that and then copy the files over that revision.

Neil