Tk Source Code

HI,
using this version of TK:
https://core.tcl-lang.org/tk/info/d5989e7ea9e31bd4

when moving mouse over the following script, my CPU goes to 30% when TK is compiled as a framework.
when Tk is linked against X11, CPU is 2%.
maybe I'm doing something wrong, but can you please have a look...
thanks

package require Tk
package require Ttk
ttk::setTheme clam
ttk::style configure num.TButton -foreground black \
	-relief [list {pressed !disabled} solid] -highlightbackground black -activebackground black
ttk::style configure num.TButton -width 5
ttk::style map num.TButton -foreground [list active white]
ttk::style map num.TButton -background [list pressed yellow active black !disabled gray80]
ttk::style map num.TButton -relief [list !disabled solid]
pack [frame .n] -side top
pack [frame .n.n] -side top
pack [frame .n.n.n] -side top
pack [frame .n.n.n.n] -side top
pack [frame .n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n.n.n.n.n] -side top
pack [frame .n.n.n.n.n.n.n.n.n.n.n] -side top
set this [frame .n.n.n.n.n.n.n.n.n.n.n.n]
pack [frame $this.2 -bg white] -side left -fill both -expand 1
pack [ttk::button $this.2.bLevel -text W -style num.TButton] -side top -fill x -expand 1
pack [ttk::button $this.2.b7 -text 7 -style num.TButton -command { puts {7} ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.2.b4 -text 4 -style num.TButton -command { puts {4} ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.2.b1 -text 1 -style num.TButton -command { puts {1} ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.2.bfull -text T -style num.TButton -command {puts {T} ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.2.bPlus -text \u002B -style num.TButton] -side top -fill x -expand 1
pack [frame $this.3] -side left -fill both -expand 1
pack [ttk::button $this.3.bto -text F -style num.TButton] -side top -fill x -expand 1
pack [ttk::button $this.3.b8 -text 8 -style num.TButton -command { puts {8} ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.3.b5 -text 5 -style num.TButton -command { puts {5}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.3.b2 -text 2 -style num.TButton -command { puts {2}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.3.b0 -text 0 -style num.TButton -command { puts {0}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.3.bpoint -text \u2022 -style num.TButton -command { puts {.}  ; break;}] -side top -fill x -expand 1
pack [frame $this.4] -side left -fill both -expand 1
pack [ttk::button $this.4.bto -text A -style num.TButton] -side top -fill x -expand 1
pack [ttk::button $this.4.b9 -text 9 -style num.TButton -command { puts {9}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.4.b6 -text 6 -style num.TButton -command { puts {6}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.4.b3 -text 3 -style num.TButton -command { puts {3}  ; break;}] -side top -fill x -expand 1
pack [ttk::button $this.4.bc -text C -style num.TButton] -side top -fill x -expand 1
pack [ttk::button $this.4.bmoins -text \u2212 -style num.TButton] -side top -fill x -expand 1
pack $this -side top

This seems to be addressed in the bug fix branch for [8793e78bf0] so I am
closing this ticket and marking it as a duplicate of [8793e78bf0].

Hi Marc,
I fully agree that we all know what's going wrong, this is why I've asked on the IRC if it was possible to hire someone to code for this purpose.

best regards,
Nicolas

Kevin, yes I agree that only widgets which meet the damage region get updated. That is exactly what I said below. The catch is that, currently, the damage region gets reported as the entire bounding rectangle of the main NSView.

If you look at the bug fix branch for this ticket you will my attempt from last April to generate more efficient damage regions. That did increase the efficiency of the redraws quite a bit -- @nab was no longer able to generate 100% CPU usage with his app. But that fix broke other things. Notably, the scrollbars in the canvas items demo became unusable. I don't know the full extent of the collateral damage, but that one was sufficient to rule out merging the bug fix branch as is. And I was not able to pinpoint why the scrollbars were affected that way.

This thread has now gone through a full cycle, covering all of the same topics that were covered in the first round and saying the exact same things over again. If people are interested in pursuing this (and I think it would be a nice improvement) then I suggest starting where I left off and figuring out what is going on with the scrollbars in the canvas items demo. It won't help to keep saying the same things over and over again.

Hi Kevin,
the thing is if I mouse the mouse anywhere in the main window, all the entire window (and so all the widgets) is tagged to be refreshed.
as generateUpdate is initially called by generateExposeEvents() which rely on TkWindow *winPtr = TkMacOSXGetTkWindow([self window]); (which is the main window)
in turns,  generateUpdate() generate Expose for every one.

++

If I'm reading the GenerateUpdates function correctly (I didn't write this code), expose events are sent to the regions that are the intersection of the NSView bounds and the bounds of the HIShape damage region. So while Tk does have to survey the entire window space, redrawing only occurs in those damaged regions, NOT the entire NSView or window. That means that configure events should NOT be that computationally intensive. boundsRgn = HIShapeCreateWithRect(&bounds); damageRgn = HIShapeCreateIntersection(updateRgn, boundsRgn); HIShapeGetBounds(damageRgn, &damageBounds); are the relevant lines. Marc, do you agree?

HI Marc,
ok for the statement from Apple, I didn't knew.
regarding the example above, my app doesn't contain nested frame like that... it was the quickest way I've found to show you rising CPU usage.
people don't do crazy mouse move in real life... but my app is gridding several canvas/frame/buttons (but not too much nested) and even a simple itemconfigure is triggering all the window to update..
but as you said it's because the damaged region is set to the entire main window.

in your opinion what would be the requirement for the damaged region to reflect the real needs?
I'm not a Tcl neither a Tk expert, but when I write .x.y itemconfigure $myItem -text 0, I would like to see only .x.y to be updated.

anyway, thank you for your time.
best regards,
nicolas

The generateUpdates function does not loop over all subwindows. It only loops over those that meet the damage region. However, at the moment the damage region is being set to the entire bounding rectangle of the NSView. So the way to increasing performance is to be more careful about defining the damage region.

That said, it is not true that, for example, your test script above is laggy. The only way to generate high CPU usage is to do something no real user would do, namely to furiously move the mouse back and forth over the grid of buttons. As a comparison, I tried doing the same furious mouse movement over a QT app which highlights buttons when the mouse enters. (The app is VirtualBox.) I see comparable CPU usage to what I see in your example. So, while I would like to see someone improve the computation of the damage region and I think that would be a nice improvement to Tk, I do not see it as a critical issue. Certainly it is not at the priority of other issues that remain unresolved.

For the record, this is what Apple says:

Avoid the Overuse of Views

NSView offers tremendous flexibility in managing the content of your windows and provides the basic canvas for drawing your application’s content. However, when you consider the design of your windows, think carefully about how you use views. Although views are a convenient way to organize content inside a window, if you create a complex, deeply nested hierarchy of views, you might experience performance problems.

Although Cocoa windows can manage a relatively large number of views (around one hundred) without suffering noticeable performance problems, this number includes both your custom views and the standard system controls and subviews you use. If your window has hundreds of custom visual elements, you probably do not want to implement them all as subclasses of NSView. Instead, you should consider writing your own custom classes that can be managed by a higher-level NSView subclass. The drawing code of your NSView subclass can then be optimized to handle your custom objects.

A good example of when to use custom objects is a photo browser that displays thumbnail images of hundreds or even thousands of photos. Wrapping each photo in an NSView instance would both be prohibitively expensive and inefficient. Instead, you would be better off by creating a lightweight class to manage one or more photos and a custom view to manage that lightweight class.

ok.
so please just to confirm, as long as generateUpdates() in tkMacOSXWindowEvent.c loops over all displayed items, there's absolutely no chance to know which par of the main NSView needs to be updated and so, toplevel with many gridded widgets will be laggy.
 
Each command/MouseOver/whatever on the main window will trigger  Tk_QueudWindowEvent() for every displayed widget...
generateUpdates() do the same with contained windows, so it's pointless to split my app in different toplevels and glue them in a canvas.

damned !!!

No, those design changes go back four or five years. The older design of Tk under Cocoa used private API calls as a hack to stabilize the rendering of widgets under Tk and its performance; buttons, scrolling, etc. used the native Cocoa API's and had Tk wrappers. This was exceptionally complex and did not play well with Tk's drawing and event model, and using private API's is really a no-no per Apple. So we removed the private API's, replaced the NSButton and NSScroller calls with HITheme calls (native drawing but NOT additional NSViews), and left the main NSView for a toplevel to do all that drawing on. Still not perfect but much improved.

Hi Kevin,
thank you for your answer.
Was it when Mojave was involved?
because with Mojave it seems that only one NSView is catastrophic for toplevel that handle many widgets...

++

Nicolas

It really isn't possible to change the design of using a single NSView for a toplevel - that's an architectural decision all the way down and is based on painful experience using multiple NSViews in windows. Whatever fix for this issue might be implemented will not involve this suggestion.

This happens mainly because (I think) the whole main window is only one NSView and whenever an item needs to be updated the callback check on every items of the NSView.

would it be possible that every gridded frame of the main window are created as a NSStackView?
Thus, maybe, with the provided example when user move the mouse over the buttons, only the latest View is repaired.

best regards,
nicolas

still happens with latest core8.6...
CPU rise >100% only moving the mouse over my app

Hi,
as each time an update should happen it's all the window that is marked to be regenerate (so with a large amount of widgets, it quickly consume a lot of CPU), I've tried to place some parts of my app inside frame with option  -container 1.
my app is now several toplevel gridded together in frames... but no luck...
updates are still propagated to all widget displayed.

:(

I wouldn't want the broken canvas scrollbar to appear in the release, and I
don't know yet how to fix it.  If a robust fix emerges before the release then
we can worry about how to deal with it.

Hi Marc,
as dgp tagged 8.6.10_rc, is there's something that can be done with this bug before the release?

best regards,
nicolas

I repeated my experiment with logging calls to drawRect and, indeed, I am wrong. It is not true that every mouse movement triggers a call to drawRect. With a script like the one above, where the window is covered with widgets that change appearance on Enter and Leave events it is close to happening with every mouse movement, But if you watch carefully you see that some mouse movements occur with no redrawing.

Thanks, @chrstphrchvz.

Hi Marc,
if you need testing, I'll be available next week.

best regards,
Nicolas

I really don't know who or what is to blame. What I observe, by logging calls to [NSView drawRect], is that the drawRect method gets called for each mouse movement. In the main branch, that results in a redraw of the entire window because drawRect is passed the bounding rectangle of the NSView as the region to draw. The bugfix branch for this bug was attempting to limit the redrawing to a smaller rectangle.

Cc'ing myself.

Apple's window manager redraws a window for each mouse movement, to repair the damage caused by rendering the cursor.

I could be having SIWOTI syndrome, but to my knowledge Macs use hardware cursors, so I don't understand why redrawing the window to "repair damage" is always necessary. This is not something I deal with though, so I trust that others here know better than I do.

Unfortunately, I am seeing glitches.  The ttk::scrollbar does not update
correctly.  You can see this in the "canvas items" demo.  I don't know why
that is happening.

I need to put more update idlestasks, but that's ok
CPU does no more reach 100%, well done :)
CPU is still going to 90% on some heavy graphics updates with a lot of canvas text item, but maybe that's part of another optimization to come...
otherwise I didn't see any glitch with this branch

++

@Marc,
THIS IS COOOOOL!!

@nab - Can you please test the tip of the bug-fix branch?  I think that the
recent change to XMapWindow may have fixed the problem you saw with windows not
being updated until the mouse is moved.  Certainly it did fix a similar problem
that I was seeing with ttk::notebook widgets.

The bug fix branch does significantly reduce the load on the CPU, which used to
go over 25% whenever a progress bar was running, for example.  If it does not
introduce new graphics artifacts it would be worth merging into the core.

Hi Marc,
sorry for the late answer, I've some daily work to do.
I don't think I can provide a simple script as CPU rising is due to deep deep stuff in my app, but I can send you my app...

best regards,
nicolas

Could you please provide a simple script that shows high CPU usage when
scrolling a text widget in a canvas?

Hi marc,
thank you very much for looking into that :)
for the mouse craziness it's a lot better !! before your commit, scrolling a canvas text item leads to 100%, now it's at 30%. bravo...

It looks like there's now more subtle needs to [update] to trigger stuff... for example I have a canvas that displays several items (text and rectangle), I have a 'config' button that update what needs to be display in the canvas and with this commit, canvas is not always graphically updated until I move the mouse .

also I've posted these spindump (from Activity Monitor), first one is from Tk compiled as a framework, second one from Tk is linked against X11.
https://gist.github.com/sl1200mk2/42055d950bb9d4c611590df578d09c3a
https://gist.github.com/sl1200mk2/4055d9b79aa5f8c0f0d63918ed1b9790
for the same operation TK is rising 100% when compiled as framework, 5% when linked against X11

hope this helps
best regards,
Nicolas

@nab -- OK. I looked at it. And, amazingly enough, I found something. It turns out that it is not Apple's fault that [NSView drawRect] is always called with the full bounding rectangle of the NSView. It is my fault. I think I might have fixed that in the branch bug-06f3922f8b.

When I removed all of the strange and unnecessary nesting from the script you posted in the report and simply gridded all of the buttons as children of one frame then I was not able to get the CPU usage much above 10% even when wildly moving the mouse around like no real user would ever do. The same kind of crazy mouse action in a terminal window uses 5% CPU. So I don't think we are going to do much better than that. I'll attach the simplified script.

WARNING: this needs a lot of testing in a lot of different situations before it can be merged. We need to make sure we aren't creating new graphics artifacts. My first attempt would create garbage when moving canvas items quickly. There could be lots of other things like that.

Hi Marc,
I've changed (almost) all my widgets to be grid instead of pack (as DKF suggested on irc) but nearly same results... rising close to 100%
is there's a chance you can investigate on this?

best regards,
nicolas

Hi François,
yes it's exactly the same issue.

++

Is this the same issue as what is discussed here in comp.lang.tcl?

well, that's true for the mouse pointer, but we also want that canvas itemconfigure id to only redraw the desired item and not the entire canvas, neither the entire NSView.

best regards

Yes. What you saw was consistent with what I said.  The Apple window manager
is passing drawRect the bounding rectangle for the entire NSView.  We would like
to get just a small damage rectangle covering the old mouse pointer.  I would
guess that X11 does something equivalent to that.

Hi Mark,
in my real app this is what I see:
https://pastebin.com/aVgsiPHJ

it seems that for any mouse mouvement over any frame, window . is passed, and thus every frame/button/canvas is passed through GenerateUpdates()

best regards

To Apple's Window manager a Tk window consists of one NSWindow containing one
NSView.  Tk is responsible for drawing its widgets inside the View. When you
move your mouse inside the View, Apple asks the View to redraw a damage region,
by calling the drawRect method of the view.  As I understand it, what happens is
that Apple passes the bounding rectangle of the View to its drawRect method.  So
Tk responds by redrawing everything that meets that rectangle, i.e. by redrawing
everything inside the window.  If Apple were to ask for updates only in a small
region containing the old pointer image, then maybe we could avoid sending expose
events for the children which do not meet that small damage region.

It is true that I have not spent any time trying to do this sort of optimization,
since other things were more urgent.  Maybe we could do better. If you can figure 
out how to get more detailed damage information from Apple, please let us know!

I think my problem is in tkMacOSXWindowEvent.c and particulary the recursion in GenerateUpdates() for each child of winPtr.
in my real app, there's more or less the script I've posted, but there's also plenty of other frame/buttons/canvas...
if the mouse is over button 6 for example and I move it to button 7,  GenerateUpdates() is triggered 400 times!
which in turns trigger 400 times the event loop due to Tk_QueueWindowEvent()...

so it seems not to be a problem in Apple's window manager

many thanks for your help with that.

++

Hi Marc,
thank you for your answer.

it should also means that Apple's window manager redraw window for each canvas itemconfigure invocation and this is more problematic...

Different window managers are different.  Apple's window manager redraws a window
for each mouse movement, to repair the damage caused by rendering the cursor.
Even the Terminal app can use 10% CPU while you move the mouse rapidly inside
the window.  X11 probably has a more efficient way of redrawing the cursor.  But
real users don't sit at an application moving their mouse randomly around the
window as fast as they can.  So it probably isn't that important.