Better Unicode Handling #26

[pyscripter]

The DirectWrite version of SynEdit in branch DWrite is close to completion! So far the following changes were committed:

• GDI text painting was replaced with DirectWrite painting
• SynEditPrint was implemented with DirectWrite (now with a new fix to performance issues, @MShark67 please test again)
• SynEditWordWrap implemented almost from scratch using DirectWrite
• Cursor positioning (Pixel... methods) reimplemented using Direct Write
• SynEditTextDrawer.pas was replaced with SynDWrite.pas and SynUsp10.pas was removed

The only thing left to implement is the update of editor commands (ecLeft, ecRight, ecDeleteLastChar, etc) and SetCaret to account for multi-code point characters. And of course more testing, testing, testing...

The code now deals with all the issues described in #26, including bidirectional text. SynEdit should now be on a par with, if not better than, the best editors around with respect to Unicode handling. The codebase has also been very much streamlined, with some of the most horrendously complex parts of the code (text painting and word wrap) replaced with much simpler and easier to maintain code and many of the weird parts of the code removed (wide-glyph and hard tab handling). Also the code is more than 1000 lines of code shorter.

But of course there may well be many bugs that need to be addressed before the DWrite branch becomes production ready. So
please test all aspects of SynEdit usage (@MShark67, @JaFi-cz and whoever else reads this discussion) including

• Painting
  • Syntax highlighting
  • Hard tabs
  • Showing special characters
  • Active line and special line colors
  • Unicode (wide glyphs such as Chinese, multi-codepoint graphemes such as emojis, bidi text etc.)
  • Indentation guides
  • Gutter painting
• Word wrap
• Printing
• Text quality and performance issues
• Handling of different fonts
• Backward compatibility
• Whatever else you can think

[MShark67]

Nice! I'll get some testing going. Should issues be reported in the issues section or here?

[pyscripter]

Here to start with.

[JaFi-cz]

I'l be busy till end of this month. Then I will have some time to prepare test machine and start testing.

[MShark67]

I've been testing this today. You've done an amazing job on it. All the issues I've found are directly related to caret positioning and scrolling when using wide characters (which you pointed out you already know about.) One very small issue that I found (and I'm not sure it's even a bug) is that the text is being drawn one pixel down from the older version. I found this out when using PaintTransient to redraw brackets, I needed to adjust TextOut down a pixel from the y position given by RowColumnToPixels.

As a bit of a lark I bypassed the monospaced font test and loaded up Arial to see how it handled it. It worked very well! As you've noted anything using FCharWidth calculations is an issue, but once the hscrolling is switched to pixels instead of chars it should be amazing. I've also tested with a bunch of different unicode chars, wide single byte ones and also multibyte ones. All the painting was perfect as far as I could tell.

I haven't really tested performance yet, but in my testing with some pretty large files it looked good. I'll report back if I find more. Great job.

[MShark67]

I'll test printing tomorrow (packing up for today.) I'm not sure about the answer on PaintTransient, as mentioned I had to adjust my TextOut drawing position by one to match the new display. I don't think it's a major issue, but I know that others use very similar PaintTransient code, so they'd have adjust it too. I can give a quick code example to show it, I'll try to do that tomorrow. I did find one actual issue. The PixelsToColumn function will return zero if the inputs are an empty string and FLastPosX is zero. That causes an assert error when the new column position is checked.

-Mark

[pyscripter]

FIxed PixelsToColumn issue.

Regarding the PaintTransient. I have changed the default value for ExtraLineSpacing to 2 see above, but unless you reinstall SynEdit, you would see in your form designer the value as 0, but in fact when the component is created its value will become 2! You need to reinstall the component and change the value to 0 and save the form. Note also that this value is PPI scaled, so your PaintTransient code needs to move things down by ExtraLineSpacing div 2.

I am still trying to replace CharsInWindow. It is mostly used in scrolling. I have introduced a new property TextAreaWidth (see CalcTextAreaWidth) to replace CharsInWindow. It would be nice if you could merge your scrolling code with this branch and use TextAreaWidth instead of CharsInWindow. Just place the handler class in SynEditMiscClasses.
I think we can leave LeftChar as is, so that we scroll horizontally in increments of CharWidth.

[MShark67]

I can do that PR shortly. I choose to add a new unit SynEditScrollBars.pas and just add a ISynEditScrollBars interface to SynEditTypes, very similar to how SynEditUndo is done. My desire was to cut down on circular unit refs as much as possible, but let me know if you want it integrated in a different way. Also, of course, feel free to change anything I submit.

I've left the horizontal scrolling as CharWidth based units for now (since LeftChar is CharWidth based as well.) I think there might be some FCharWidth/ FCharsInWindow caret positioning/ScrollWindow issues to handle when characters can be smaller or larger than FCharWidth. If we want to switch to pixel based units I'd just need a pixel based LeftChar equivalent (maybe there's already such a thing or I'm off base on this.)

[pyscripter]

Thanks Mark,

Using interfaces is a good idea. You still need to have a SynEdit dependence in the implementation section I take it . What I did with undo/redo and WordWrap is to add the interface definition in SynEditTypes. You still need the SynEditUndo dependence in the Implementation section of SynEdit to get access to the factory method. Such a dependence could be eliminated using some kind of dependency injection, which could be used for scrolling/undo/wrapping all based on interfaces. If you do make it a separate unit, packages also need to be updated. Updating the C++ packages is hard for me, but we have to do it anyway since I have added one unit (SynDWrite) and removed two other (SynUni10 and SynTextDrawer).

I would like to remove CharsInWindow altogether. I want to break the connection between width and number of chars, which may lead to errors now and later. As implemented now, LeftChar corresponds to pixels LeftPixels := (LeftChar - 1) * CharWidth. It does not necessarily imply that there are LeftChar characters on the left. It could be replaced by LeftPixels (or LeftWidth) at some point, but I do not want to do all the changes at once. Same as CharWidth, it is a kind of average char width (most characters including spaces will have such a width), but it does not imply that all characters will have such a width.
If in your implementation of Scrolling if you could use LeftChar * CharWidth and TextAreaWidth and TSynEditStringList.MaxWidth that would help.

Did my comment on ExtraLineSpacing explain the discrepency in PaintTransient?

[MShark67]

Thanks! I'm aware of the dependency stuff (I've had it done for days and used the Undo stuff as a guide.) Completely decoupling would require an interface on TCustomSynEditor (which I've already worked a bit on) but it requires adding getter functions to a lot of the "direct read" properties and I didn't want to wreak too much havoc at once. Full on dependency injection is outside my skill area, though I find it very interesting. I think this way is a good compromise. I'll make those requested changes and get a PR done today. I think some adjustments will definitely be necessary as the whole "variable character width" stuff shakes out. I haven't re-looked at PaintTransient yet but I'll report back once I do.

[pyscripter]

Other changes to consider:

• Alpha blending the selection area
• Full row painting of selection or not
• Decide on the Indentation lines
• ...

[MShark67]

I'm seeing problems with caret horizontal movement where LeftChar should be changed (i.e. scrolling past the end of the visible area.) To see the issue just resize the SynEdit such that moving the caret would change LeftChar to keep the caret visible, but it doesn't. When I turn off the gutter completely, it seems to work better. If doesn't work at all with eoScrollPastEol (it looks like it uses the current line to do the calculating, but with the EOL setting on it would need to adjust the values.) Let me know if you need more steps. Thanks!

[pyscripter]

Horizontal mouse wheel scrolling does not work here. I think we need a WM_MOUSEHWHEEL handler which is not handled by VCL. It could simply call DoMouseWheel.

[MShark67]

Fix is good. It looks like KeepCaretX is off by one on blank lines when combined with AllowCaretPastEOL. I think the fix is in PixelsToColumns:

  if S = '' then
    Result := Max((ax div fCharWidth) + 1, 1)     <-- I added the +1
  else

I think there might be a similar issue in PixelsToRowColumn (just because I notice it uses similar code, I haven't seen an actual issue.)

[pyscripter]

Fixed. Please check.

[MShark67]

Horizontal mouse wheel scrolling does not work here. I think we need a WM_MOUSEHWHEEL handler which is not handled by VCL. It could simply call DoMouseWheel.

I'll check that out. I thought you just meant the standard "hold shift and wheel up down" which is what I handled. I'll report back or submit a pr once I've learned stuff!

[pyscripter]

hold shift and wheel up down

Nice! I did not know about this.
Some mouses have horizontal scroll functionality (a separate wheel or the ability to push the vertical wheel left or right). I think this is what WM_MOUSEHWHEEL handles.

[pyscripter]

Fixed movement commands and removed CharsInWindow. Now they should work well. Next I need to fix ecChar handling. It is getting close.

[MShark67]

Testing here shows things working very well. The recent change to TSynEditPrint.SetLines has an issue in that it doesn't clear FLines before adding the new strings.

[pyscripter]

Fixed.

[pyscripter]

I have committed a number of fixes, the most important of which relate to entering complex graphemes (ecChar, ecIme) and properly handling overwrite mode. It should now work correctly. Try entering emojis with the Windows button + '.'. It results in an ecIme command.
There were also a number of fixes related to WordWrap. Setting fCaretEOL correctly was not easy.

I have now run out of things to fix so it is up to you (@MShark67, @JaFi-cz ) to check and discover the bugs. Until then I can now try to polish things in relation to:

• Indentation lines
• Alpha blending the selection area
• Full row painting of selection (or not)

[MShark67]

• Printing quality and performance (does it actually print to printers ok?)

Quality and performance both seem excellent. On my mid-level business class pc it paginates and displays the first preview page of a 250,000 line SQL script in about 5 seconds (5000 pages.) Paging though is almost instant. Printing works and the quality looks perfect.

• the PaintTransient issue (Was it actually due to the ExtraLineSpacing? - see my earlier comment)

I finally looked at this. You are correct! Changing the ExtraLineSpacing to 1 "fixes" and explains the issue perfectly and I can take that setting into account in my code.

• overall text rendering quality and performance.

Rendering seems great. I can clearly see the pepperonis on the pizza emoticon! I can load the SQL script file mentioned above in about 2 seconds, including the time to determine MaxLineWidth. After that things are very snappy. I do notice that scrolling that same file with IndentGuides on is a bit choppy, but only in Debug mode (I have range and overflow checking on.) When I run in release mode it's much faster.

[pyscripter]

250,000 line SQL script

Are you serious?

Thanks for the feedback. Pls keep on trying to break it.

[MShark67]

250,000 Lines of highly bracketed SQL queries that mostly return shark and pizza emoticons! So, you know, real world conditions.... 🍕🦈

[MShark67]

There's a non-critical range check error in GetWordAtRowCol if the caret is at EOL or beyond. I fixed it with:

    Start := Min(XY.Char, Len);

[pyscripter]

Fixed.

[pyscripter]

Refactored Indentation guides and decoupled it from code folding. New SynEdit property IndentGuides with style and color customization (solid/dotted). You need to reinstall SynEdit and load/save forms.

[JaFi-cz]

This is great feature. It will help a lot. Thank you. I follow you guys, even I don't have time for testing, but I hope in near future I will help too.

[MShark67]

Love the new options. Very cool.

[pyscripter]

Implemented selection alpha blending. Try it!!
TSynSelectedColor has two new properties. Alpha and PaintFullLines. Set Alpha to 1 to have the old behavior. The constructor sets it to 0.4.

Also vastly reduced flicker when extending the selection with the mouse resulting in scrolling, especially with ActiveLineColor set. Try it before and after.
PaintFullLines has not yet been implemented.

[MShark67]

Looks great! It seems to work just fine with VCL themes (with some alpha tweaking based on the theme colors.) Once implemented, what will PaintFullLInes do?

[pyscripter]

If false "non full selection painting"

[MShark67]

Aha! Thanks, I should have realized that.

[pyscripter]

PaintFullLInes renamed to FillWholeLines and implemented. Give it a go!

@MShark67 I am now finished with #26. Keep testing and when we feel it is production ready we can merge it with the master branch. One thing that needs to be done is to update the packages (+ 2 units - 2 units). I wonder whether you can help with that.

[MShark67]

That's awesome! I notice that empty lines show as "one char selected". That may be intentional so that the selection still shows as contiguous. Does it make sense to follow the VSCode convention and do that on every fully selected line? Basically, the selection would be as if eoShowSpecialChars is on, even if the option isn't on (I mention that just because I toggle back and forth between those a lot. and noticed that the selection matches VSCode.) I hope that makes sense! Either way, nice one! I'll look at the packages.

[MShark67]

Looks amazing. The only thing I notice is that the special "hard tab" character (SynTabGlyph) isn't painted when selected and eoShowSpecialChars is on (or possibly it's painted in the selection color.)

[pyscripter]

Fixed

[MShark67]

[Removed bug report in error. I did something to my repos.] Sorry about that!

[pyscripter]

So is is now OK?

[MShark67]

Working flawlessly. Great feature.

[MShark67]

I just wanted to double check on the package cleanup stuff:
Add SynDWrite.pas
Add SynEditScrollBars.pas
Remove SynTextDrawer.pas
Remove SynUsp10.pas

The current package lists don't include the following highlighters that are in the source directory. Should they be added?
SynHighlighterLLVM
SynHighlighterWebIDL

Some other files of interest:
This file isn't used at all as far as I can tell:
SynEditWildcardSearch.pas
This file is used depending on a define in SpellCheck, but is old and won't compile.
SynSpellCheckMetaphone.pas

[pyscripter]

I just wanted to double check on the package cleanup stuff: Add SynDWrite.pas Add SynEditScrollBars.pas Remove SynTextDrawer.pas Remove SynUsp10.pas

Yes

The current package lists don't include the following highlighters that are in the source directory. Should they be added? SynHighlighterLLVM SynHighlighterWebIDL

Let's add them

Some other files of interest: This file isn't used at all as far as I can tell: SynEditWildcardSearch.pas

This is a TSynEditSearchCustom decendent so it could be used by some users as a Search engine. Let's leave it in the repo. By the way the RTL has a lesser known wildcard search function called MatchesMask which is more comprehensive for wildcard search.

This file is used depending on a define in SpellCheck, but is old and won't compile. SynSpellCheckMetaphone.pas

I have committed changes to make it compile. Let's keep it. (@JaFi-cz is this file needed?)

Another unit that is not used anywhere is SynEditAutoComplete.pas. This unit defines a component called TSynAutoComplete, but the newer unit SynCompletionProposal also defines a component with the same name which is added to Delphi. I think this is an opportunity to remove that unit from the repo. Having two units with the same component name can only cause confusion (it confused me at some point - which one should I use?). This will not affect the packages since SynEditAutoComplete is not included in the packages. @JaFi-cz Is it OK to remove this file?

[MShark67]

Note that SynEditAutoComplete is in the Delphi packages (along with SynCompletionProposal.) It seems that I should remove it from the packages?

[pyscripter]

I see. But it does not register anything in SynEditReg.pas, so please remove it.

[pyscripter]

Packages merged.

Do you know anything about the C++ packages? I don't even have C++Builder installed. I see that the directories include the Delphi dpk files. I am not sure they are needed there but at least those should be replaced.

When do you think it would be appropriate to merge with the master branch?

[MShark67]

I know nothing about the C++ Builder files. I did take a look just in case it was a simple list of files that I could modify, but that didn't seem to be the case. It seems to me that the merge could happen soon. Maybe a tag could mark the switchover (although I guess it's easy enough for people to get the preDwrite version in case they need XP support or for some other reason.

[pyscripter]

Merged with the master.
Your name does not shown as a Contributor and this is a shame. I searched for answers and I found this. Apparently you need to add your email to your github account.

[JaFi-cz]

Update please description:
add D11
put there information, that master doesn't suport Windows XP and would be nice to have branche with WinXP support

[pyscripter]

@JaFi-cz I made this realease with GDI support https://github.com/TurboPack/SynEdit/releases/tag/LastGDIbased. I probably should have made a branch as well.

Could you please provide a PR for the documentation?
Please also have a look at my comment in #51

Update: Created branch GDI which contains the latest version with GDI. I am not planning to do any work on that but you can still submit PR for that branch. I am also removing the DWrite branch to avoid confusion. Please switch to master.

[JaFi-cz]

I registered and read all adds. I hope I will have more time from next week. It was amazing job and lot of work and time you both did last 2 weeks.

[pyscripter]

https://en.delphipraxis.net/topic/6277-synedit-just-got-a-major-uplift/

[pyscripter]

The update of the C++ packages is being taken care off.
One thing that needs updating is the demos. (Any volunteers?)

And talking of demos @JaFi-cz one of the overlooked components is the spell-checking. Would it be possible to add a demo showing off the spell checking component?