Performance degradation in 3.1.6 comparing to 3.1.5 when scrolling a big binary file loaded to wxStyledTextCtrl when the HiDPI awareness is set. (Issue #22450)
wadim-al
Describe the bug
When a huge binary file is loaded to the wxStyledTextCtrl, the contents can be scrolled quickly when compiled against wxWidgets 3.1.5, but since the wxWidgets 3.1.6 if the HiDPI awareness is set in the manifest, the scrolling is considerably slower. OS scaling is set to 125%, monitor resolution is 2560x1440.
To Reproduce
- Compile the attached snippet against 3.1.6 on Windows with high res monitor and set hidpi awareness in the manifest.
- Run the example
- Drag the scroll bar
- See that redraw is visibly slow (under 1 fps)
- Repeat all steps, but compile against 3.1.5
- Compare scrolling performance between 3.1.5 and 3.1.6
Platform and version information
- wxWidgets version you use: 3.1.6
- wxWidgets port you use: wxMSW
- OS and its version: Windows 10 x64 Pro 21H2 19044.1645
- Compiler: VC 2019
Expected vs observed behaviour
I expected the same or better scrolling performance, but 3.1.6 is considerably slower.
Video comparison side by side 3.1.5-3.1.6
Image for loading:
image
Min repro source: wxstctest.cpp.txt
Patch or snippet allowing to reproduce the problem:
Code Example (click to expand)#include <wx/wx.h> #include <wx/stc/stc.h> #include <wx/file.h> class MyApp : public wxApp { public: virtual bool OnInit(); }; class MyFrame : public wxFrame { public: MyFrame(); }; wxIMPLEMENT_APP(MyApp); bool MyApp::OnInit() { MyFrame* frame = new MyFrame(); frame->Show(true); return true; } MyFrame::MyFrame() : wxFrame(NULL, wxID_ANY, "STC scrolling issue") { wxBoxSizer* sizer = new wxBoxSizer(wxVERTICAL); this->SetSizer(sizer); wxStyledTextCtrl* stc = new wxStyledTextCtrl(this); stc->SetScrollWidthTracking(true); sizer->Add(stc, 1, wxEXPAND, 0); this->Layout(); this->Maximize(); wxMemoryBuffer memBuff; wxFile f_in("img.jpg"); wxFileOffset fLen = f_in.Length(); char tmp; for (wxFileOffset i = 0; i < fLen; ++i) { f_in.Read(&tmp, 1); memBuff.AppendByte(tmp); memBuff.AppendByte(0); } stc->AddStyledText(memBuff); }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
PB
FWIW, I can confirm the issue, when comparing 3.1.5 and master, the former is redrawing noticeably faster even when just using PgUp/PgDn.
Both built with MSVS 2022 as x64 static release, using PMv2 DPI awareness.
Windows 10 21H2, primary screen 2560x1440 at 125% DPI, Geforce RTX 2060.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks for reporting this!
Would it be possible to run git-bisect to find which commit caused this regression?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
wadim-al
@vadz , I'm sorry, I couldn't finish the bisect flow without problems. Here are bisect logs, I hope these might help.
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect start
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good v3.1.5
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect bad v3.1.6
Bisecting: 865 revisions left to test after this (roughly 10 steps)
[efb82320c63f40ccfd050e565f097d2d611c417f] Merge branch 'imaglist-destroy'
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 434 revisions left to test after this (roughly 9 steps)
[4217573a3ff61687600673ee02613f932bf8d527] Fix wxGrid::SetCurrentCell() with Wayland
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 217 revisions left to test after this (roughly 8 steps)
[e25b47ee32ab23c6105a6e689956c2ead379d324] Allow tweaking parameters of wxImage::Scale() benchmarks
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 108 revisions left to test after this (roughly 7 steps)
[af9e7fd4609381e2086a13a603e5e46e64718a77] Fix truncation of translated "Finish" button label in wxWizard
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 54 revisions left to test after this (roughly 6 steps)
[6d6e5cde21969f91f9f4df85a72f0ae68e423cb1] Enhance wxUILocale and wxLocaleIdent
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect bad
Bisecting: 26 revisions left to test after this (roughly 5 steps)
[03bf61be3c4a5e39adfdee97a12e48c53eb86785] Fix drawing caret on GTK3 with GDK_SCALE=2
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 13 revisions left to test after this (roughly 4 steps)
[7e45373e16f61ad63c668257af4a80ff7bcd30e7] Add a simple workflow for updating HTML docs online
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 8 revisions left to test after this (roughly 3 steps)
[4a63bae1c8cf74afc98e5bd2db938243c7f37eb6] Use wxRecursionGuard to set and reset wxGTK g_inSizeAllocate
C:\Users\VZ\Desktop\wx\wxWidgets>git bisect good
Bisecting: 4 revisions left to test after this (roughly 2 steps)
error: Your local changes to the following files would be overwritten by checkout:
build/msw/wx_aui.vcxproj.filters
build/msw/wx_base.vcxproj.filters
build/msw/wx_core.vcxproj.filters
build/msw/wx_custom_build.vcxproj.filters
build/msw/wx_gl.vcxproj.filters
build/msw/wx_html.vcxproj.filters
build/msw/wx_media.vcxproj.filters
build/msw/wx_net.vcxproj.filters
build/msw/wx_propgrid.vcxproj.filters
build/msw/wx_qa.vcxproj.filters
build/msw/wx_ribbon.vcxproj.filters
build/msw/wx_richtext.vcxproj.filters
build/msw/wx_stc.vcxproj.filters
build/msw/wx_webview.vcxproj.filters
build/msw/wx_wxexpat.vcxproj.filters
build/msw/wx_wxjpeg.vcxproj.filters
build/msw/wx_wxpng.vcxproj.filters
build/msw/wx_wxregex.vcxproj.filters
build/msw/wx_wxscintilla.vcxproj.filters
build/msw/wx_wxtiff.vcxproj.filters
build/msw/wx_wxzlib.vcxproj.filters
build/msw/wx_xml.vcxproj.filters
build/msw/wx_xrc.vcxproj.filters
samples/minimal/minimal.vcxproj.filters
Please commit your changes or stash them before you switch branches.
Aborting
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
This is annoying and must be due to (correct, on its own, but resulting in such problems) e3535d6 (Mark MSVS *.vcxproj.filters files as using CR LF as well, 2021-11-24). I already ran into this myself and I don't remember how did I fix it, but I think just removing all these files and hard-resetting should do it.
If you don't manage to fix it, could you please show what does git bisect visualize say at the current stage? TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
wadim-al
git bisect finished
9e5c8a802700f3fef6c1fda6d3b375e3e21e42cc is the first bad commit
commit 9e5c8a802700f3fef6c1fda6d3b375e3e21e42cc
Author: Vadim Zeitlin <va...@wxwidgets.org>
Date: Sat Mar 26 01:38:59 2022 +0100
Respect bitmap content scale factor in wxMSW wxMemoryDC
Apply it manually because MSW doesn't do it automatically for us and
also adjust the font size in wxMemoryDC as the base class version only
does it for the device contexts associated with a window, but we also
need to do it when using a wxMemoryDC for a bitmap using scale factor
different from that of the main display.
As the result of these changes, contents drawn on wxMemoryDC, both
directly via its own methods, or via wxGraphicsContext (using either
GDI+ or Direct2D) created from it, it appears the same as in wxWindowDC
(e.g. wxPaintDC) created for a window using the same scale.
Closes #22130.
Closes #22234.
include/wx/msw/dcmemory.h | 2 ++
samples/drawing/drawing.cpp | 3 ++-
src/msw/dc.cpp | 2 +-
src/msw/dcmemory.cpp | 19 +++++++++++++++++++
src/msw/graphics.cpp | 12 ++++++++++--
src/msw/graphicsd2d.cpp | 8 +++++---
6 files changed, 39 insertions(+), 7 deletions(-)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
Seems to be caused by many many calls to scaledFont.WXAdjustToPPI(GetPPI());, that creates a new HFONT every time. This can be avoided by already having a font with a size based on the correct DPI.
This improved performance a lot for me:
src/stc/PlatWX.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stc/PlatWX.cpp b/src/stc/PlatWX.cpp index 0ac2ae46ab..182f28e842 100644 --- a/src/stc/PlatWX.cpp +++ b/src/stc/PlatWX.cpp @@ -299,7 +299,7 @@ void SurfaceImpl::InitPixMap(int width, int height, Surface *surface, WindowID w if (width < 1) width = 1; if (height < 1) height = 1; bitmap = new wxBitmap(); - bitmap->CreateWithDIPSize(width, height,(GETWIN(winid))->GetContentScaleFactor()); + bitmap->CreateWithDIPSize(width, height,(GETWIN(winid))->GetDPIScaleFactor()); mdc->SelectObject(*bitmap); }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks @wadim-al for running the bisection to the end!
Using DIP scale factor rather than as the content scale factor as parameter to CreateWithDIPSize() makes sense to me and I think it's the right thing to do for MSW as the original code, which comes from a4f6fe4 (support retina display, 2013-06-25), was written with macOS in mind where the two are the same anyhow.
@wadim-al Could you please confirm that this patch brings the performance back to (roughly) the previous level?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
wadim-al
@vadz Yes, the patch helps indeed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #22450 as completed via 4f8b49c.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
FYI, this fix seems to introduce artifacts visible in the stc sample:
We also need to provide DIP size to CreateWithDIPSize instead of width and height:
diff --git "a/src/stc/PlatWX.cpp" "b/src/stc/PlatWX.cpp" index 182f28e842..76af652ddd 100644 --- "a/src/stc/PlatWX.cpp" +++ "b/src/stc/PlatWX.cpp" @@ -293,13 +293,13 @@ void SurfaceImpl::InitPixMap(int width, int height, Surface *surface, WindowID w wxMemoryDC* mdc = surface ? new wxMemoryDC(static_cast<SurfaceImpl*>(surface)->hdc) : new wxMemoryDC(); - mdc->GetImpl()->SetWindow(GETWIN(winid)); hdc = mdc; hdcOwned = true; if (width < 1) width = 1; if (height < 1) height = 1; bitmap = new wxBitmap(); - bitmap->CreateWithDIPSize(width, height,(GETWIN(winid))->GetDPIScaleFactor()); + wxWindow* win = GETWIN(winid); + bitmap->CreateWithDIPSize(win->ToDIP(width), win->ToDIP(height), win->GetDPIScaleFactor()); mdc->SelectObject(*bitmap); }
I can create a PR with this fix, and also remove the now obsolete mdc->GetImpl()->SetWindow.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
That's ok, I can commit the patch above, thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
It's not complete, still artifacts at 175%
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
It's not complete, still artifacts at 175%
Just to be clear, those artefacts were not present without 4f8b49c? Or were they always there?
Anyhow, I'd appreciate a PR if there is going to be more than one commit, TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
They were indeed introduced by 4f8b49c
It creates a too big bitmap (width and height are multiplied with DPI scale factor). Scintilla only draws part of it, the rest remains uninitialized (the black area).
ToDIP doesn't work because it rounds values, causing the bitmap sometimes to become too big. For example, if called with InitPixMap(8, 8, ...), it creates a wxBitmap of 5x5, which is then multiplied with 1.75 resulting in a 9x9 wxBitmap. Which will have the black border artifact.
I'm looking at just using:
bitmap = new wxBitmap(width, height); bitmap->SetScaleFactor(win->GetDPIScaleFactor());
but I want to verify if this works on macOS too. Or if we should multiply width and height with GetContentScaleFactor.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
This doesn't work on macOS. There the bitmap size should actually be multiplied to get a high-res/retina bitmap. And in addition, SetScaleFactor() does nothing to the actual bitmap context on macOS.
The best solution I can come up with is this. You can commit it if you want, otherwise I'll create a PR tomorrow.
diff --git "a/src/stc/PlatWX.cpp" "b/src/stc/PlatWX.cpp" index 182f28e842..04e3682c48 100644 --- "a/src/stc/PlatWX.cpp" +++ "b/src/stc/PlatWX.cpp" @@ -293,13 +293,17 @@ void SurfaceImpl::InitPixMap(int width, int height, Surface *surface, WindowID w wxMemoryDC* mdc = surface ? new wxMemoryDC(static_cast<SurfaceImpl*>(surface)->hdc) : new wxMemoryDC();
- mdc->GetImpl()->SetWindow(GETWIN(winid));
hdc = mdc;
hdcOwned = true;
if (width < 1) width = 1;
if (height < 1) height = 1;
+#ifdef wxHAS_DPI_INDEPENDENT_PIXELS
bitmap = new wxBitmap();
- bitmap->CreateWithDIPSize(width, height,(GETWIN(winid))->GetDPIScaleFactor());
+ bitmap->CreateWithDIPSize(width, height, GETWIN(winid)->GetContentScaleFactor()); +#else + bitmap = new wxBitmap(width, height); + bitmap->SetScaleFactor(GETWIN(winid)->GetDPIScaleFactor()); +#endif mdc->SelectObject(*bitmap); }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.