Hi all,

ok, it is broken since this commit:

http://git.postgresql.org/gitweb/?p=pgadmin3.git;a=patch;h=b0ecbbca7f77c0f07cff67bba3d2bca28954a1e2

-   EVT_STC_UPDATEUI(-1,  ctlSQLBox::OnPositionStc)
+   EVT_STC_PAINTED(-1,  ctlSQLBox::OnPositionStc)

It appears that in previous version of scintilla, there existed a SCN_POSCHANGED event, which then got obsolete and replaced by SCN_UPDATEUI, and probably that is why the name 'OnPositionStc' was chosen for the handler. So linking OnPositionStc() to STC_UPDATEUI was correct and in accordance with scintilla's manual. However the change from EVT_STC_UPDATEUI to EVT_STC_PAINTED was incorrect. These events are not quite equivalent. EVT_STC_PAINTED has a somewhat different purpose.

I've checked, reverting back to EVT_STC_UPDATEUI indeed fixed CPU load and lockup issues on windows. See also
http://www.scintilla.org/ScintillaDoc.html#SCN_PAINTED
http://www.scintilla.org/ScintillaDoc.html#SCN_UPDATEUI
http://www.scintilla.org/ScintillaHistory.html
http://web.mit.edu/jhawk/mnt/spo/sandbox/punya/anjuta-1.2.3/doc/ScintillaDoc.html (-- this is some older document with a more detailed explanation of SCN_UPDATEUI)

Some relevant excerptions:

"SCN_PAINTED: is to update some _other_ widgets based on a change."

"SCN_POSCHANGED: notification is deprecated as it was causing confusion. Use SCN_UPDATEUI instead."

"SCN_UPDATEUI, SCN_CHECKBRACE:
Either the text or styling [...] common use is to check whether the caret is next to a brace and set highlights on this brace and its corresponding matching brace."

So for me this all sounds clear sufficiently. I'll stick with EVT_STC_UPDATEUI and I can rebuild it myself if I have to. Whether fix git respectively or not is up to maintainers now.


Thank you,
Nikolai


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-support mailing list (pgadmin-support@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-support

Reply via email to