Menu
▾
▴
Re: [MACTCL] Request for testing--Tk-Cocoa
|
From: Youness A. <kak...@ka...> - 2009-08-18 00:02:13
|
Hi Daniel,
On Fri, Aug 14, 2009 at 9:26 PM, Daniel A. Steffen <
da...@us...> wrote:
> Hi Youness,
>
> On Fri, Aug 14, 2009 at 22:06, Daniel A.
> Steffen<da...@us...> wrote:
> > On Fri, Aug 14, 2009 at 19:26, Youness
> > Alaoui<kak...@ka...> wrote:
> >> 1 - drawing widgets seems extremely slow compared to TkCarbon, try the
> >> preferences window and change from one tab to another, you'll see it
> draw
> >> each widget one at a time, while on Carbon, it was instantenuous.
> >
> > I haven't observed slowness
>
> with the workaround for 2. below in place, I have now also observed
> this as well.
> It turns out to be related to the idle-time redrawing behaviour of Tk,
> the only items affected are widgets drawn via native NSViews, i.e.
> push/check/radiobuttons, scrollbars and menubuttons.
>
> At idle time when these are drawn, NSWindow flushing is enabled
> (whereas in the ordinary AppKit draw loop it is disabled) and thus
> their drawing is immediately flushed to screen.
> To workaround this it suffices to disable/enable flushing for the
> containing window in TkMacOSXSetup/RestoreDrawingContext(), c.f. patch
> below (against de-carbon-8-5, but should apply to HEAD as well).
> Note that the window is always flushed at the next passage through the
> event loop anyway (via -tkDisplayIfNeeded), or upon explicit call to
> [update] (via XSync()).
>
> The patch avoids the issue and AFAICT doesn't appear to break
> anything, but I would appreciate some wider testing before I commit.
Hi, I've tested it and seems to work just fine, the slowness compeltely
disappeared now! Good job!
>
>
> >> 2 - At one point, when I opened the preferences window, aMSN froze, I
> used
> >> Command-, and the 'aMSN' menu stayed highlited, and it looked as if the
> >> runloop had stopped because it didn't handle my mouse events anymore
> (focus
> >> on windows, or the close/minimize/maximize buttons stopped getting
> >> hovered..). It doesn't always happen, but it's still easily
> reproducable...
> >
> > I have seen that, breaking into the debugger at that point shows that
> > this is caused by aMSN calling [update] or [vwait] at inopportune
> > times (e.g. I have seen these called from an [after] event handler and
> > from a channel notification handler), doing this causes recursive
> > entry of the runloop at locations where Tk and AppKit do not expect
> > this to happen...
>
> I cannot do anything about the nested [vwait]s, but I have been able
> to avoid the lockups from nested [update]s, c.f. patch to tcl notifier
> below. It should only affect code running with the notifier in
> embedded mode (i.e. probably only TkAqua Cocoa so far), certainly the
> tcl test suite appears unaffected.
>
> The fix consists mainly of making sure that event loops entered
> recursively from the runloop observer's Tcl_ServiceAll() processing
> (e.g. via some event handler calling [update]) run in the
> tcl-events-only runloop mode, and that polling Tcl_WaitForEvent()'s
> called at that time wake up the notifier thread in all cases.
> The one tricky thing is that a second runloop observer is needed for
> the tcl-events-only runloop mode (with the same observer handler as
> the original), as it turns out that CFRunLoop prevents the handler of
> a given observer from being entered recursively...
>
> As this is a slightly more invasive change, I would appreciate if a
> few people could do some testing and running of the tcl testsuite to
> make sure this is not breaking something unexpected, I don't have time
> to do extensive testing myself ATM...
I tested de-carbon-8-5 (to which you already committed these patches) and it
worked nicely.I tried the preferences window a couple of times and it worked
correctly, and I haven't experienced a freeze so far. Good job Daniel!
By the way, slightly off-topic, I've heard all the rumors about the [update]
command to avoid, etc.. but it's a bit hard to do that, we use it a lot in a
lot of cases, and in all cases, we had absolutely no choice... and
considering that any function call in Tk is the result of an 'after' or a
file event, or a UI interaction, etc.. it would be useless to have 'update'
but not be able to use it in some obscure situations...
About [vwait], I didn't know we used that... Would it make a difference to
use [tkwait variable] or is that just the exact same thing? There are some
use cases where we also have no choice but to use that command ([tkwait
variable]), so...
In any case, so far so good, it seems to work fine, thanks!
Thanks again,
Youness.
>
>
> Thanks!
>
> Cheers,
>
> Daniel
>
> --
> ** Daniel A. Steffen **
> ** <mailto:da...@ca...> **
>
>
> diff --git tk/macosx/tkMacOSXDraw.c tk/macosx/tkMacOSXDraw.c
> index 83492e8..23dd70e 100644
> --- tk/macosx/tkMacOSXDraw.c
> +++ tk/macosx/tkMacOSXDraw.c
> @@ -1565,6 +1565,7 @@ TkMacOSXSetupDrawingContext(
> if (dontDraw) {
> goto end;
> }
> + [[view window] disableFlushWindow];
> dc.view = view;
> dc.context = [[NSGraphicsContext currentContext] graphicsPort];
> dc.portBounds = NSRectToCGRect([view bounds]);
> @@ -1690,6 +1691,7 @@ TkMacOSXRestoreDrawingContext(
> {
> if (dcPtr->context) {
> CGContextSynchronize(dcPtr->context);
> + [[dcPtr->view window] enableFlushWindow];
> if (dcPtr->focusLocked) {
> [dcPtr->view unlockFocus];
> } else {
> diff --git tcl/macosx/tclMacOSXNotify.c tcl/macosx/tclMacOSXNotify.c
> index 706c988..193a46e 100644
> --- tcl/macosx/tclMacOSXNotify.c
> +++ tcl/macosx/tclMacOSXNotify.c
> @@ -246,6 +246,8 @@ typedef struct ThreadSpecificData {
> * performed. */
> int runLoopRunning; /* True if this thread's Tcl runLoop
> is running */
> int runLoopNestingLevel; /* Level of nested runLoop invocations */
> + int runLoopServicingEvents; /* True if this thread's runLoop is
> servicing
> + * tcl events */
> /* Must hold the notifierLock before accessing the following fields: */
> /* Start notifierLock section */
> int onList; /* True if this thread is on the
> waitingList */
> @@ -277,7 +279,7 @@ typedef struct ThreadSpecificData {
> /* Any other thread alerts a notifier that
> an
> * event is ready to be processed by
> signaling
> * this CFRunLoopSource. */
> - CFRunLoopObserverRef runLoopObserver;
> + CFRunLoopObserverRef runLoopObserver, runLoopObserverTcl;
> /* Adds/removes this thread from waitingList
> * when the CFRunLoop starts/stops. */
> CFRunLoopTimerRef runLoopTimer;
> @@ -453,7 +455,7 @@ Tcl_InitNotifier(void)
> CFRunLoopSourceRef runLoopSource;
> CFRunLoopSourceContext runLoopSourceContext;
> CFRunLoopObserverContext runLoopObserverContext;
> - CFRunLoopObserverRef runLoopObserver;
> + CFRunLoopObserverRef runLoopObserver, runLoopObserverTcl;
>
> bzero(&runLoopSourceContext, sizeof(CFRunLoopSourceContext));
> runLoopSourceContext.info = tsdPtr;
> @@ -477,12 +479,21 @@ Tcl_InitNotifier(void)
> "CFRunLoopObserver");
> }
> CFRunLoopAddObserver(runLoop, runLoopObserver,
> kCFRunLoopCommonModes);
> - CFRunLoopAddObserver(runLoop, runLoopObserver,
> + runLoopObserverTcl = CFRunLoopObserverCreate(NULL,
> + kCFRunLoopEntry|kCFRunLoopExit|kCFRunLoopBeforeWaiting,
> TRUE,
> + LONG_MIN, UpdateWaitingListAndServiceEvents,
> + &runLoopObserverContext);
> + if (!runLoopObserverTcl) {
> + Tcl_Panic("Tcl_InitNotifier: could not create "
> + "CFRunLoopObserver");
> + }
> + CFRunLoopAddObserver(runLoop, runLoopObserverTcl,
> tclEventsOnlyRunLoopMode);
>
> tsdPtr->runLoop = runLoop;
> tsdPtr->runLoopSource = runLoopSource;
> tsdPtr->runLoopObserver = runLoopObserver;
> + tsdPtr->runLoopObserverTcl = runLoopObserverTcl;
> tsdPtr->runLoopTimer = NULL;
> tsdPtr->waitTime = CF_TIMEINTERVAL_FOREVER;
> tsdPtr->tsdLock = SPINLOCK_INIT;
> @@ -710,6 +721,9 @@ Tcl_FinalizeNotifier(
> CFRunLoopObserverInvalidate(tsdPtr->runLoopObserver);
> CFRelease(tsdPtr->runLoopObserver);
> tsdPtr->runLoopObserver = NULL;
> + CFRunLoopObserverInvalidate(tsdPtr->runLoopObserverTcl);
> + CFRelease(tsdPtr->runLoopObserverTcl);
> + tsdPtr->runLoopObserverTcl = NULL;
> if (tsdPtr->runLoopTimer) {
> CFRunLoopTimerInvalidate(tsdPtr->runLoopTimer);
> CFRelease(tsdPtr->runLoopTimer);
> @@ -1150,9 +1164,8 @@ int
> Tcl_WaitForEvent(
> Tcl_Time *timePtr) /* Maximum block time, or NULL. */
> {
> - int result, polling;
> + int result, polling, runLoopRunning;
> CFTimeInterval waitTime;
> - CFStringRef runLoopMode;
> SInt32 runLoopStatus;
> ThreadSpecificData *tsdPtr;
>
> @@ -1206,16 +1219,12 @@ Tcl_WaitForEvent(
> * added to the common run loop modes might get lost.
> */
>
> - if (tsdPtr->runLoopRunning) {
> - runLoopMode = tclEventsOnlyRunLoopMode;
> - } else {
> - runLoopMode = kCFRunLoopDefaultMode;
> - tsdPtr->runLoopRunning = 1;
> - }
> - runLoopStatus = CFRunLoopRunInMode(runLoopMode, waitTime, TRUE);
> - if (runLoopMode == kCFRunLoopDefaultMode) {
> - tsdPtr->runLoopRunning = 0;
> - }
> + runLoopRunning = tsdPtr->runLoopRunning;
> + tsdPtr->runLoopRunning = 1;
> + runLoopStatus = CFRunLoopRunInMode(tsdPtr->runLoopServicingEvents ||
> + runLoopRunning ? tclEventsOnlyRunLoopMode :
> kCFRunLoopDefaultMode,
> + waitTime, TRUE);
> + tsdPtr->runLoopRunning = runLoopRunning;
>
> LOCK_NOTIFIER_TSD;
> tsdPtr->polling = 0;
> @@ -1333,19 +1342,22 @@ UpdateWaitingListAndServiceEvents(
> {
> ThreadSpecificData *tsdPtr = (ThreadSpecificData*) info;
>
> + if (tsdPtr->sleeping) {
> + return;
> + }
> switch (activity) {
> case kCFRunLoopEntry:
> tsdPtr->runLoopNestingLevel++;
> - if (tsdPtr->runLoopNestingLevel == 1 && !tsdPtr->sleeping &&
> - (tsdPtr->numFdBits > 0 || tsdPtr->polling)) {
> + if (tsdPtr->numFdBits > 0 || tsdPtr->polling) {
> LOCK_NOTIFIER;
> - OnOffWaitingList(tsdPtr, 1, 1);
> + if (!OnOffWaitingList(tsdPtr, 1, 1) && tsdPtr->polling) {
> + write(triggerPipe, "", 1);
> + }
> UNLOCK_NOTIFIER;
> }
> break;
> case kCFRunLoopExit:
> - if (tsdPtr->runLoopNestingLevel == 1 && !tsdPtr->sleeping &&
> - (tsdPtr->numFdBits > 0 || tsdPtr->polling)) {
> + if (tsdPtr->runLoopNestingLevel == 1) {
> LOCK_NOTIFIER;
> OnOffWaitingList(tsdPtr, 0, 1);
> UNLOCK_NOTIFIER;
> @@ -1353,9 +1365,11 @@ UpdateWaitingListAndServiceEvents(
> tsdPtr->runLoopNestingLevel--;
> break;
> case kCFRunLoopBeforeWaiting:
> - if (!tsdPtr->sleeping && tsdPtr->runLoopTimer &&
> + if (tsdPtr->runLoopTimer && !tsdPtr->runLoopServicingEvents &&
> (tsdPtr->runLoopNestingLevel > 1 ||
> !tsdPtr->runLoopRunning)) {
> + tsdPtr->runLoopServicingEvents = 1;
> while (Tcl_ServiceAll() && tsdPtr->waitTime == 0) {}
> + tsdPtr->runLoopServicingEvents = 0;
> }
> break;
> default:
>
|