TurboPack / SynEdit

SynEdit is a syntax highlighting edit control, not based on the Windows common controls.
221 stars 73 forks source link

Possible word wrap issue with special characters. #210

Closed MShark67 closed 2 years ago

MShark67 commented 2 years ago

I loaded up one of my older test files which has a single line like:

✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈ ✈✈✈✈✈✈

When I turn on word wrap mode the line doesn't wrap properly if the width of the SynEdit is more than half the width of the above line (but less than the full width of the line, of course.) The caret can be moved off the right edge as if it wasn't wrapped. Let me know if you need any other information to reproduce.

pyscripter commented 2 years ago

Is this code and the end of SynEditWordWrap causing this?

  if (PEnd - PStart) * CW < fMaxRowWidth   then
    // Optimization.  Assume line will fit!
    RowLengths := [SLine.Length]
  else

It was meant to be

  if (PEnd - PStart) * CW < fMaxRowWidth div 2  then
    // Optimization.  Assume line will fit!
    RowLengths := [SLine.Length]
  else

Does the change resolve the issue?

pyscripter commented 2 years ago

@MShark67 While we are at it could you do some benchmarking to see whether the Parallel for still makes sense in SynEditWordWrap and SynEditTextBuffer?

MShark67 commented 2 years ago

That change improves the position where the issue starts. I'd say that now the "breakpoint" is about 3/4 the width of the line instead of it being half. It does not appear to solve the issue. I'll look into the parallel for question. Thanks.

pyscripter commented 2 years ago

This is a single code point with a width greater than that of two normal characters.

image

If you change div 2 to div 3 it should work. This is contrived case of a line consisting solely of such wide characters. If you mix in some normal characters it should still work. In any case I am suggesting to use div 3 or drop the optimization altogether, but I 'd rather not do that.

MShark67 commented 2 years ago

This is absolutely a contrived case and it certainly wouldn't happen in the real world. I just wanted to report it since I just happened to load that file and noticed it. Whatever you decide is A-OK.

pyscripter commented 2 years ago

So does div 3 work in this case? If yes I will commit the change and close the issue.

MShark67 commented 2 years ago

Yes it does! Looks good!

MShark67 commented 2 years ago

Benchmarks for loading a very large file shows GetMaxWidth parallel still roughly 3-4 times faster than the old version. Was that expected to change? I haven't tested wordwrap performance (it's just a bit more involved.)

pyscripter commented 2 years ago

That's reassuring. Let's leave the threading in.