Opened 4 years ago

Last modified 22 months ago

#17804 reopened defect

wxSTC doesn't implement SetTechnology to switch to D2D/DirectWrite needed for fonts with ligatures

Reported by: paulclinger Owned by: GitHub <noreply@…>
Priority: normal Milestone: 3.1.1
Component: wxStyledText Version: dev-latest
Keywords: Cc: m0081@…, paulclinger@…
Blocked By: Blocking:
Patch: yes

Description

When I try SetTechnology/GetTechnology calls, I only get 0 returned (at least on Windows). The SetTechnology call doesn't do anything in Editor.cxx, as it needs to be handled in a platform-specific way:

	case SCI_SETTECHNOLOGY:
		// No action by default
		break;

SciTE provides the necessary Win-specific logic in scintilla/win32/ScintillaWin.cxx, which uses LoadD2D method implemented in scintilla/win32/PlatWin.cxx.

This is necessary to implement ligatures used in FiraCode (https://github.com/tonsky/FiraCode) and other similar fonts. There is a related ticket for Notepad++ (https://github.com/notepad-plus-plus/notepad-plus-plus/issues/2287), but it boils down to setting the right font and calling SetTechnology(1), which has no effect in wxwidgets.

We already have the logic to load necessary libraries in src/msw/graphicsd2d.cpp, but I don't have the grasp of the details to integrated this with wxSTC.

Attachments (13)

fira.png download (214.6 KB) - added by NewPagodi 4 years ago.
courier.png download (185.0 KB) - added by NewPagodi 4 years ago.
cpu_wx.png download (9.5 KB) - added by NewPagodi 4 years ago.
cpu_nat.png download (9.7 KB) - added by NewPagodi 4 years ago.
prof.jpg download (86.1 KB) - added by NewPagodi 4 years ago.
cpu_scrolling.png download (35.4 KB) - added by awi 4 years ago.
Performance report
cpu_scrolling_2.png download (66.5 KB) - added by awi 4 years ago.
Performance report #2
wstc-bad.png download (43.7 KB) - added by eran 4 years ago.
stc-ligatures.png download (110.3 KB) - added by eran 4 years ago.
Side by Side comparison for Fira Code. Left is wxSTC without the patch, On the right is the one with the patch
stc-good.png download (40.8 KB) - added by eran 4 years ago.
stcd2d.patch download (54.8 KB) - added by NewPagodi 4 years ago.
Create the SurfaceD2D class and accociated items to be used with wxSTC
stcd2d.2.patch download (54.0 KB) - added by eran 4 years ago.
Updated patch with MinGW-w64 build fixes (II)
17804-SetTechnology1.png download (9.4 KB) - added by paulclinger 2 years ago.
Large auto-complete popup with SetTechnology(1) set

Download all attachments as: .zip

Change History (47)

comment:1 follow-up: Changed 4 years ago by NewPagodi

  • Cc m0081@… added

I tried one approach to letting the SetTechnology function work as requested in this ticket here. Basically the approach was to use a wxD2DContext inside the class Scintilla uses to do all drawing and use the wxD2DContext to do the drawing/measuring. It does let wxSTC draw fonts with ligatures. I can post pictures if need be. However there are at least 2 issues:

It just draws some fonts (including Courier New) wrong. It also draws the Courier New font worse than the default drawing code. (Again I can post pictures if needed.)

Also the scrolling performance is quite poor. There is noticeable lag when using the scroll bar. When scrolling it spikes CPU usage to a quite high level, but not 100%. So there must be some other bottleneck in addition to CPU that's not letting the control scroll smoothly.

Because of those 2 issues, I think trying to use wxD2DContext is a dead end. Would it be acceptable to try to create a new drawing class only available on the windows platform using native winapi code to see if I can get better performance? Or would that be unacceptable since wxSTC is basically a generic control?

comment:2 Changed 4 years ago by vadz

It's very surprising if the overhead comes from wxD2DContext, I don't know its code that well, but it doesn't seem to be doing anything obviously stupid, so why would it be so slow? It would be really useful if you could profile this code to confirm that the problem is indeed due to it and not something else (in STC itself or wxSTC).

I.e. I don't think it's unacceptable to use native API directly if it's the only way to provide decent performance, but it would be great if we could fix wxD2DContext to provide it instead.

comment:3 in reply to: ↑ 1 Changed 4 years ago by paulclinger

  • Cc paulclinger@… added

Replying to NewPagodi:

It does let wxSTC draw fonts with ligatures. I can post pictures if need be. However there are at least 2 issues:

Yes, I think it will be useful to have the pictures posted. Thank you for giving it a try. Unfortunately, I can't help with the alternative solutions, but can probably test to see how it performs on my computer. What platform have you tested it on?

Paul.

Changed 4 years ago by NewPagodi

Changed 4 years ago by NewPagodi

Changed 4 years ago by NewPagodi

Changed 4 years ago by NewPagodi

Changed 4 years ago by NewPagodi

comment:4 Changed 4 years ago by NewPagodi

Here's an image showing some code using the Fira Code font. Ligatures are visible in lines 306, 311, and a few other lines. The left window is wxSTC with the default surface, the middles is wxSTC with the new D2D surface, and the right is a native Scintilla window. All 3 are configured identically with the SendMsg function.


Here's what happens with the Courier New font:


The font rendering on the d2d surface seems blurrier than on the default surface. It also compresses the fonts vertically. It compresses so much that it loses the "_" in "WX_PRECOMP" at line 16.

Here's what the CPU activity looks like during heavy scrolling with the d2d surface:


and for comparison, here's with the native window:


So it's hitting the cpu hard (harder than it should be), but not all the way to 100%. So I assume that means there's some other bottleneck in addition to the high cpu use.

Here a profile log taken again during heavy scrolling. (Sorry I can't figure out how to get this as a text table):


wxSTC obviously wouldn't normally call OnPaint that much, but when scrolled it has to constantly update the text and margins that are drawn which is why it's being hit so hard here.

comment:5 Changed 4 years ago by paulclinger

  • Cc paulclinger@… removed

Great summary; thank you for posting all the details!

I can't comment on why the size is different vertically, but it does seem like ascent and descent settings are affected. I get a similar result when I set SetExtraAscent and SetExtraDescent to values like -3 or -4. So, maybe the values are not read from the font or are not rendered correctly in this case?

I'm interested in what happens when the ligature symbols are deleted. For example, when backspace is pressed after !=, does it remove =, such that it replaces the ligature with ! that is left?

comment:6 Changed 4 years ago by vadz

I wonder if caching the context instead of recreating it every time might help?

comment:7 Changed 4 years ago by awi

I downloaded your implementation with wxD2D and tested it with STC sample. I haven't observed any lags while using the scrollbar. Also CPU utilzation was very low during scrolling - see attached screenshot of performance report. Tests were done on the quite ordinary machine - i7 CPU with embedded graphics card (Intel HD Graphics 530) and Win 10. So it seems that wxD2D itself shouldn't be blamed for poor performance.

I have some remarks regarding your implementaion:

  1. What @vadz also observed, the context is recreated very often (through te calls to SurfaceD2DImpl::CreateContext). It is constantly recreated even if the application is in the idle state (from the user perspective) - maybe this is the question of blinking cursor?
  2. What is seen on the report, the application spends relatively long time in SurfaceD2DImpl::Copy. It is apparently used to copy from one surface to another through the intermediate bitmap. Maybe this is a suboptimal solution?
  3. It seems that by default painting under wxMSW is manually double buffered with wxBufferedDC. Maybe this makes no sense in combination with wxD2D-driven surfaces and wxPaintDC should be used instead?

Performance report

Changed 4 years ago by awi

Performance report

comment:8 follow-up: Changed 4 years ago by NewPagodi

Thanks for looking into this. To clarify the scrolling issue, to reproduce what I'm seeing paste a large file into wxSTC and then scroll quickly (not super human speed, but fairly quickly) from top-to-bottom or bottom-to-top. When I do this, I see the scroll button will sometimes freeze in position for a bit and then jump a few centimeters, then freeze and jump, and so on. The scroll button can also lag behind the mouse cursor.

Because of those 2 things it makes the window feel like it's not responding like it should.


The calls to SurfaceD2DImpl::Copy are due to Scintilla's buffered drawing. That can be turned off by calling wxSTC::SetBufferedDraw(false). When you do that it does help the scrolling issue I described above, but doesn't completely get rid of it. Most people probably don't know to make that call though, so the surface should probably work well even when it isn't done.

The current implementation of SurfaceD2DImpl::Copy in my branch is very sub-optimal.


I don't know the inner workings of wxGraphicsContext/wxD2DContext so these may be stupid questions:

1) can a wxD2DContext be detached from a dc and attached to a new one? I suspect that most of the lag is due to creating/destroying a context in every surface, but that's 100% a hunch and based on no evidence.

2) is there an analogue of blitting for copying one graphics context into a second one. If there is, that would make the SurfaceD2DImpl::Copy method work much better.

3) is it possible to have a graphics context operation to rasterize the present state that returns a wxBitmap (or something else that can be used to create a brush). That would help with another operation needed for the new surface.


The ligatures do work as described above. If you have the caret to the right of the "!=" ligature and hit backspace, it leaves "!".

comment:9 Changed 4 years ago by paulclinger

  • Cc paulclinger@… added

I think it's fine to turn of the buffered drawing if it helps this case. According to the Scintilla documentation (http://www.scintilla.org/ScintillaDoc.html#SCI_SETBUFFEREDDRAW): "Current platforms perform window buffering so it is almost always better for this option to be turned off. For Win32 and GTK+, client code should turn off buffering at initialisation." I'm not sure why it's "on" by default, but it may be a result of some earlier choices when it was a better option. The client code can turn it on as needed.

comment:10 Changed 4 years ago by awi

Here you are performance report for scrolling 16.5 MB file (~82000 lines).
Generally there is no dramatic visual differences with fast scrolling between the versions with and without D2D. Lags for D2D are a bit more seen but not extensively (for both version lags are more seen at the beginning, but after doing some scrolling back and forth they seem to decrease - caching?). CPU utilization is low and more less the same for both versions.
Performance report #2

Changed 4 years ago by awi

Performance report #2

comment:11 in reply to: ↑ 8 Changed 4 years ago by awi

Replying to NewPagodi:

1) can a wxD2DContext be detached from a dc and attached to a new one? I suspect that most of the lag is due to creating/destroying a context in every surface, but that's 100% a hunch and based on no evidence.

Current implementation of wxD2DContext doesn't support re-attaching to another HDC. It could be implemented by adding an API exposing calls to the nativeID2D1DCRenderTarget::BindDC but how this re-binding would work in the real life, I don't know.

2) is there an analogue of blitting for copying one graphics context into a second one. If there is, that would make the SurfaceD2DImpl::Copy method work much better.

Essentially, wxD2DContext is a wrapper to ID2D1RenderTarget. AFAIK there is no native API to directly clone/copy it.

comment:12 Changed 4 years ago by NewPagodi

I've updated the branch above to code the surface methods using winapi calls. It's mostly the code from ScintillaWin.cxx adapted to draw on a wxPaintDC instead of an HWND. I spent a week learning D2D so that I could write the methods myself. Then I looked at that file for some tips for the ones I still couldn't figure out and realized that almost all of the methods have some individual tweaking that I probably wouldn't have figured out on my own.


The basic approach I used was to add 2 classes SurfaceData and SurfaceFontData that bundle up all of the D2D items needed for the drawing/measuring. I then added pointers to those classes to ScintillaWX and wxFontWithAscent respectively.

Other than the large block where the surface is coded, those classes are only referenced inside of #if blocks, and I think there's only 3 of those blocks.

Much cleanup and testing is still needed, but before I go any further, does this seem like an acceptable framework for resolving this ticket?

If so, should I leave everything in PlatWX.h/.cpp or should I move the code for the new surface to a new file in the /src/msw folder?


Currently, I've creating the d2d factory objects using code cut and pasted from src/msw/graphicsd2d.cpp.

Would it be possible to expose the global functions created in that file that create the factory objects so that I can reuse them? I've created a really simple branch that shows what I have in mind here.

If exposing the functions isn't desirable, another idea I had was to move the declaration of wxD2DRenderer to a separate header. That way the result of wxGraphicsRenderer::GetDirect2DRenderer() could be dynamic casted. wxD2DRenderer already has a method for returning the D2D factory. A method for returning the DWrite factory would need to be added.


One side effect of creating the surface this way is that font weights other than bold can be used with wxSTC (provided the font supports it). Also the font quality (antialiased, cleartype, etc) can be set which is currently impossible.

comment:13 Changed 4 years ago by vadz

I don't know what would be the best way to organize wxSTC code, but I think it would be preferable to keep all of it under src/stc, i.e. if we do want to create a new file, it should be src/stc/PlatWXMSW.cpp or something like that.

Concerning the addition of wx/msw/private/d2d.h -- sure, why not, as long as it's private there is no problem whatsoever with adding it. I think that it might be interesting to make (some of?) these functions public, but we don't have to do it now.

The only small problem I have with the current code in your d2dheader branch is that it seems a bit strange to create gs_D2DRenderer just because we need the factory object. I can't see it leading to any actual problems right now, but it would be arguably cleaner to have a separate gs_d2dFactory global (which would need to be reset by the module, as is done with the other globals already) and just share it between the new function you're adding and wxGraphicsRenderer::GetDirect2DRenderer(). But, again, this is rather minor.

comment:14 Changed 4 years ago by NewPagodi

  • Patch set

I'm submitting a patch as discussed above. I've checked it with every drawing operation I can think of: text styling, line wrapping, zooming, indentation guides, margin styling, every marker, every indicator, making white space visible, the long line indicator, carets, selection, and hotspots.

I checked both with Scintilla's buffered drawing enabled and disabled.

I think that's everything. But if I missed checking any drawing items, since all of the above is working, I would hope those things work as they should as well.


As discussed above I've created the wx/msw/private/graphicsd2d.h file and gs_d2dFactory global. I've checked the drawing sample to make sure I didn't break anything.

One slight change, is that I used the factory function created in wxDirect2D instead of the standard D2D1CreateFactory function that was used before. The wxDirect2D method lets factory options be set. Really there is only 1 option - the debugLevel. However according Microsoft, direct2D debugging requires a dll only available with windows 8 and later and requires visual studio 2012 or later. So I tried to add a check to set the debugging options in those cases.

I can state from sad experience that turning on d2d debugging can be helpful. But if this is a problem, I can change it to always use D2D1_DEBUG_LEVEL_NONE which should match the previous behavior.


The new surface can draw on 2 types of windows: the wxSTC itself and call tip windows. To let the surface know which type of window it's working of, I set the window name in wxSTCCallTip to "wxSTCCallTip". Then I checked the name in the init methods for the surface.

If there's a better or preferred way to check which type of window is being used, I can change that.


The new surface can make use of the wxSTC::Get/SetFontQuality methods, so I've modified gen_iface.py to let them be generated, but I've altered the docstrings to note that those methods have no effect except with the new surface.

I've also slightly altered the docstring of wxSTC::SetTechnology to note that it can only be used with windows.


The coding style varies in the files I've modified. When adding to existing code, I've tried to match the style of the surrounding code. But when adding an entirely new block, I've tried to follow the wxWidgets coding conventions. I hope that's OK.

Changed 4 years ago by eran

comment:15 follow-up: Changed 4 years ago by eran

  • Cc eran.ifrah@… added

FYI:
I applied this patch and after some struggling (I am using MinGW-w64 7.1 compiler on Windows 7) I was able to compile and link wxWidgets successfully.

However, the result, are not good.

  1. it's almost impossible to type in the editor
  2. The fon't size is HUGE (I am using Fira Code size: 10 + 4K screen display)
  3. Clicking with the mouse inside the editor is placing the carets in a wrong position :(
  4. When scrolling there are drawing artifacts
  5. The font is NOT smooth (it is smooth when disabling your patch)

I suspect that most of the above errors are caused because I am using HiRes display with 150% text size.

I have added an attachment: wxstc-bad.png to show some of the problems

comment:16 in reply to: ↑ 15 Changed 4 years ago by NewPagodi

  • Cc m0081@… removed

Replying to eran:

FYI:
I applied this patch and after some struggling (I am using MinGW-w64 7.1 compiler on Windows 7) I was able to compile and link wxWidgets successfully.

I'll look into this. Can tell me how you compiled it. Most of the new items are guarded with #if wxUSE_GRAPHICS_DIRECT2 and I thought wxUSE_GRAPHICS_DIRECT2D was 0 with the mingw compilers.

When I compile with msys2 (gcc.exe (Rev1, Built by MSYS2 project) 7.2.0), the call to SetTechnology simply fails.

comment:17 follow-up: Changed 4 years ago by eran

  • Cc eran.ifrah@… removed

The steps I did:

  1. Applied your patch
  2. Edit setup.h and defined wxUSE_GRAPHICS_DIRECT2D -> 1 (my local version of wxWidgets is already built with wxUSE_GRAPHICS_CONTEXT set to 1
  3. Added missing include to <float.h> (in PlatWX.cpp IIRC)
  4. I locally modified makefile.gcc and added -ld2d1 -lwindowscodecs
  5. I had to manually define DWRITE_E_NOFONT to

#define DWRITE_E_NOFONT _HRESULT_TYPEDEF_(0x88985002 (as described here: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/b6a3d116957f975d1a7ef9ae91b525c25162ff93/tree/mingw-w64-headers/include/winerror.h#l3472)

After these changes, the library compiles and link successfully
I am building from a CMD terminal, like this:

cd build\msw
mingw32-make -j8 -f Makefile.gcc SHARED=1 UNICODE=1 BUILD=debug VENDOR=cl CXXFLAGS="-fno-keep-inline-dllexport -std=c++11"

comment:18 in reply to: ↑ 17 Changed 4 years ago by NewPagodi

  • Cc m0081@… added

Replying to eran:

I haven't been able to compile with mingw yet, but I did notice the same problem with vc when text size is set to something other than 100%. It turned out I was having the scale factor applied twice.

I've updated the patch not to do that (and also tweaked a few other small things). Can you try it again when you get a chance?

Changed 4 years ago by eran

Side by Side comparison for Fira Code. Left is wxSTC without the patch, On the right is the one with the patch

comment:19 Changed 4 years ago by eran

  • Cc eran.ifrah@… added

The patch is better now.

However, some noticeable problems:
It looks like that the font lost its "anti-aliasing" feature

I added a screenshot of the same file (stctest.cpp): On the left, the file is opened in CodeLite (wxSTyledTextCtrl without the patch) on the right with wxSTC sample.

You can clearly see the "pixelated" font on the right side (ligatures enabled)

Some notes:

  • I used "Fira Code Retina", size 10
  • I am using a 4K display with text size set to 150%

The good points:

  • No more CPU spike
  • Mouse hit where it should
  • No CPU spike when editing or scrolling

comment:20 follow-up: Changed 4 years ago by eran

  • Cc eran.ifrah@… removed

update:

After spending quite some time debugging the "non-antialising" font.
I found the root cause:
A new method was added: "SetFontQuality"

Adding a call SetFontQuality(wxSTC_EFF_QUALITY_ANTIALIASED);
Fixes this. Notice that this was not needed prior to this patch.

So my question: what is the purpose of "SetUseAntiAliasing(true)" (it has no effect)

Maybe we can fix this method so it will internally call "SetFontQuality(wxSTC_EFF_QUALITY_ANTIALIASED)" ? what do you think?

Attached is an updated screenshot

Changed 4 years ago by eran

comment:21 in reply to: ↑ 20 Changed 4 years ago by NewPagodi

Replying to eran:

update:

After spending quite some time debugging the "non-antialising" font.
I found the root cause:
A new method was added: "SetFontQuality"

Adding a call SetFontQuality(wxSTC_EFF_QUALITY_ANTIALIASED);
Fixes this. Notice that this was not needed prior to this patch.

I was just about to post the same thing. The default surface implimentation doesn't have a way to set the quality, so the Get/SetFontQuality were deactivated. The d2d surface can use them, so I reactivated those methods as part of this patch.

So my question: what is the purpose of "SetUseAntiAliasing(true)" (it has no effect)

I'm also 90% sure SetUseAntiAliasing doesn't do anything. I'm not sure why it's included.

Maybe we can fix this method so it will internally call "SetFontQuality(wxSTC_EFF_QUALITY_ANTIALIASED)" ? what do you think?

In addition to wxSTC_EFF_QUALITY_ANTIALIASED, there's also wxSTC_EFF_QUALITY_DEFAULT and wxSTC_EFF_QUALITY_LCD_OPTIMIZED.

wxSTC_EFF_QUALITY_ANTIALIASED uses gray-scale anti-aliasing.

wxSTC_EFF_QUALITY_LCD_OPTIMIZED uses clear type.

I'm not sure what method wxSTC_EFF_QUALITY_DEFAULT uses. The documentation isn't very helpful.

You would think that when the technology is set, wxSTC_EFF_QUALITY_DEFAULT would be used, but due to some other settings in wxSTC, wxSTC_EFF_QUALITY_NON_ANTIALIASED comes up instead.

I would prefer to leave it so that the SetTechnology method only impliments the SCI_SETTECHNOLOGY message and doesn't both set the technology and the font quality. Maybe I could note in the documentation that if the user calls SetTechnology(wxSTC_TECHNOLOGY_DIRECTWRITE), they should follow that with a call the SetFontQuality method.

On the other hand the aliased drawing really does look terrible, so I'm not completely opposed to setting the font quality when the technology is set. But I would guess, based only on the name, if that's done, wxSTC_EFF_QUALITY_DEFAULT should be used instead wxSTC_EFF_QUALITY_ANTIALIASED.

comment:22 follow-up: Changed 4 years ago by eran

  • Cc eran.ifrah@… added

Will you accept an updated version of the patch that allows compilation for MinGW w64? (GCC7.1)
Atm, I need to fix the code so it will compile successfully...

Changed 4 years ago by NewPagodi

Create the SurfaceD2D class and accociated items to be used with wxSTC

comment:23 Changed 4 years ago by NewPagodi

  • Cc m0081@… removed

I just submitted a new version of the patch with one small change. I added

#if wxUSE_GRAPHICS_DIRECT2D

SetFontQuality(wxSTC_EFF_QUALITY_DEFAULT);

#endif

to the constructor for wxSTC. Unless the technology is set to use the new surface, this won't actually do anything. But if the call is made to use the new surface, some anti-aliasing is applied without having to call SetFontQuality. I think it's reasonable to initially set the font quality to something called wxSTC_EFF_QUALITY_DEFAULT since it has 'default' in its name.

By wrapping it in the #if block, users that can't use the new surface won't get have the call to the SetFontQuality method that does nothing.

comment:24 in reply to: ↑ 22 Changed 4 years ago by NewPagodi

  • Cc m0081@… added

Replying to eran:

Will you accept an updated version of the patch that allows compilation for MinGW w64? (GCC7.1)
Atm, I need to fix the code so it will compile successfully...

Sure. I'm not sure how to do that though. Can you just submit a new patch with your corrections? Although if you do that, can you apply the latest version. I just submitted a new version that I hope solves the issue of aliased font drawing.

Changed 4 years ago by eran

Updated patch with MinGW-w64 build fixes (II)

comment:25 Changed 4 years ago by eran

  • Cc eran.ifrah@… removed

I have uploaded a modified version of the patch (with some missing includes, defines etc)

I have tested it with wxSTC sample seems OK to me.
One thing to note: when loading the wxSTC sample, there is a noticeable flicker when the file is loaded.

Adding a call to wxStyleTextCtrl::SetDoubleBuffered(true) fixes this.
This is only needed when using SetTechnology(wxSTC_TECHNOLOGY_DIRECTWRITE)

comment:26 Changed 4 years ago by eran

  • Cc eran.ifrah@… added

Update:
I am now running CodeLite IDE with this patch applied with no visible issues (I am actually very pleased with the results).

One minor issue:

Calling

SetUseAntiAliase(true);

Actually does the opposite effect - it removes the font anti-aliasing...
So it might be a good idea to deprecate this function, or make it an alias to

SetFontQuality(wxSTC_EFF_QUALITY_ANTIALIASED);

comment:27 Changed 4 years ago by paulclinger

  • Cc paulclinger@… removed

Do any of these changes help with ligature support on non-Windows platforms? Or was it already supported on macOS and Linux?

comment:28 Changed 4 years ago by eran

  • Cc eran.ifrah@… removed

Ligature are supported on Linux and OSX even before these changes
Only Windows was broken

comment:29 Changed 4 years ago by paulclinger

  • Cc paulclinger@… added

@vadz, is this good to merge? I think it will be a fine addition to 3.1.1.

comment:30 Changed 4 years ago by vadz

  • Milestone set to 3.1.1
  • Status changed from new to confirmed

I've applied a couple of very minor cosmetic fixes and created this PR, if all the CI builds pass, I am going to merge it.

Thanks to all involved!

comment:31 Changed 4 years ago by GitHub <noreply@…>

  • Owner set to GitHub <noreply@…>
  • Resolution set to fixed
  • Status changed from confirmed to closed

In 9554f878b/git-wxWidgets:

Add support for using Direct2D in wxStyledTextCtrl

Use D2D if available, notably for better ligatures support.

See https://github.com/wxWidgets/wxWidgets/pull/689

Closes #17804.

Changed 2 years ago by paulclinger

Large auto-complete popup with SetTechnology(1) set

comment:32 Changed 2 years ago by paulclinger

  • Cc paulclinger@… added
  • Resolution fixed deleted
  • Status changed from closed to reopened

I unfortunately have to re-open this ticket, as I see a strange effect with the size of the auto-complete popup being much larger than expected (and the text in the popup also being larger). As can be seen in the attached image (https://trac.wxwidgets.org/attachment/ticket/17804/17804-SetTechnology1.png), the font of the popup with SetTechnology(0) matches the font in the editor, but with SetTechnology(1) the font is couple of points larger and the popup is increased as well to fit the text. The bottom of the screenshot shows that not all text is even fit due to the increased font size.

This is compiled using a recent wxwidgets commit (v3.1.2-1162-gb3ef78124c) using gcc 9.2 running on Windows 8.1 and the only difference between the two tabs is that SetTechnology(1) is called for wxSTC component shown in the left tab (four different screenshots were combined into one).

The font shown is Consolas, but I tried different fonts (include Fira Code that shows ligatures) and the effect is the same (in terms of the increased font and popup sizes with SetTechnology(1) called). In fact, I also noticed that the line height changes with some of the fonts (with Fira Code for example, but not with Consolas) making the height couple of pixels larger with SetTechnology(1).

I'm not sure why the impact on the font in the popup and have been wondering if Eran or NewPagodi has seen the same effect with their builds.

comment:33 Changed 22 months ago by paulclinger

@eran, @vadz, @NewPagodi, do you see the same effect with large auto-complete popup (as shown in the screenshot attached to the previous comment) when d2d is using on Windows? Any idea what may be causing this or how it can be fixed?

comment:34 Changed 22 months ago by paulclinger

  • Cc paulclinger@… removed

I found a workaround: calling editor:SetTechnology(0) right before calling editor:UserListShow() and then calling editor:SetTechnology(1) again after the call, makes the popup shown to be the correct size.

I suspect vs.lineHeight may be set incorrectly when used from ScintillaBase::AutoCompleteStart (as the height of the popup is wrong, but the width doesn't seem to be).

Note: See TracTickets for help on using tickets.