Hi all,
04.10.2015 7:32, J.F. Oster wrote:
Hello Nikolai,
Appreciate your research!
I suggest you to send a *.patch (git diff) attachmeht to the list
for the team members and others interested to test it in their
environments.
Well, technically I can, but it will be ugly, see below.
Problem is, the change of STC_UPDATEUI to STC_PAINTED in pgadmin was not
intended to break pgadmin on Windows, but rather to fix it on Mac. This
change was obviously incorrect, but simply reverting it would probably
cause regressions for Mac users. As I personally don't use Mac, I can
not develop and test a proper fix for it. Therefore, all what I can
suggest myself is this:
--- ctlSQLBox.cpp.orig Fri Sep 25 13:20:24 2015
+++ ctlSQLBox.cpp Sun Oct 04 15:47:17 2015
@@ -40,7 +40,11 @@
EVT_MENU(MNU_FIND, ctlSQLBox::OnSearchReplace)
EVT_MENU(MNU_AUTOCOMPLETE, ctlSQLBox::OnAutoComplete)
EVT_KILL_FOCUS(ctlSQLBox::OnKillFocus)
+#ifdef __WXMAC__
EVT_STC_PAINTED(-1, ctlSQLBox::OnPositionStc)
+#else
+ EVT_STC_UPDATEUI(-1, ctlSQLBox::OnPositionStc)
+#endif
EVT_STC_MARGINCLICK(-1, ctlSQLBox::OnMarginClick)
EVT_END_PROCESS(-1, ctlSQLBox::OnEndProcess)
END_EVENT_TABLE()
Thank you,
Nikolai
Sunday, October 4, 2015, 1:36:45 AM, you wrote:
NZ> Hi all,
NZ> ok, it is broken since this commit:
NZ>
http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=patch;h=b0ecbbca7f77c0f07cff67bba3d2bca28954a1e2
NZ> - EVT_STC_UPDATEUI(-1, ctlSQLBox::OnPositionStc)
NZ> + EVT_STC_PAINTED(-1, ctlSQLBox::OnPositionStc)
NZ> It appears that in previous version of scintilla, there existed a
NZ> SCN_POSCHANGED event, which then got obsolete and replaced by
NZ> SCN_UPDATEUI, and probably that is why the name 'OnPositionStc' was
NZ> chosen for the handler. So linking OnPositionStc() to STC_UPDATEUI was
NZ> correct and in accordance with scintilla's manual.
NZ> However the change from EVT_STC_UPDATEUI to EVT_STC_PAINTED was
NZ> incorrect. These events are not quite equivalent. EVT_STC_PAINTED has a
NZ> somewhat different purpose.
NZ> I've checked, reverting back to EVT_STC_UPDATEUI indeed fixed CPU load
NZ> and lockup issues on windows. See also
NZ> http://www.scintilla.org/ScintillaDoc.html#SCN_PAINTED
NZ> http://www.scintilla.org/ScintillaDoc.html#SCN_UPDATEUI
NZ> http://www.scintilla.org/ScintillaHistory.html
NZ>
http://web.mit.edu/jhawk/mnt/spo/sandbox/punya/anjuta-1.2.3/doc/ScintillaDoc.html
NZ> (-- this is some older document with a more detailed explanation of
NZ> SCN_UPDATEUI)
NZ> Some relevant excerptions:
NZ> "SCN_PAINTED: is to update some _other_ widgets based on a change."
NZ> "SCN_POSCHANGED: notification is deprecated as it was causing confusion.
NZ> Use SCN_UPDATEUI instead."
NZ> "SCN_UPDATEUI, SCN_CHECKBRACE:
NZ> Either the text or styling [...] common use is to check whether the
NZ> caret is next to a brace and set highlights on this brace and its
NZ> corresponding matching brace."
NZ> So for me this all sounds clear sufficiently. I'll stick with
NZ> EVT_STC_UPDATEUI and I can rebuild it myself if I have to. Whether fix
NZ> git respectively or not is up to maintainers now.
NZ> Thank you,
NZ> Nikolai
NZ> 03.10.2015 21:56, I wrote:
Hi all,
Ok, I've verified that ctlSQLBox::OnPositionStc() is broken.
If I comment the code for highlighting in it (that is, almost all of its
body), the problem goes away.
I've found quite some problems observed with it and attempted to fix
previously, like e.g. the one from 2011:
---------------
+ // Ensure we don't recurse through any paint handlers
+ Freeze();
UpdateLineNumber();
+ Thaw();
// Clear all highlighting
---------------
That was most probably only a partial fix, and it was subsequently
reverted.
I'd like to also note that 'OnPositionStc' name is pretty much
misleading. If it was named 'OnPaintedStc' it would be more consistent
and more hinting where the problem is. I'm aware that renaming alone
don't usually fix bugs, but it might help humans better catch ones.
Ok, anyway, I've got no real fix for now, but I'm going on.
Thank you,
Nikolai
....
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
