Tk Source Code

View Ticket
Login
Ticket UUID: 5869c270bdea425c066bf8a06807cc7a66aa9d41
Title: Aqua : canvas items are not always redrawn
Type: Bug Version: trunk
Submitter: nab Created on: 2024-07-23 12:39:48
Subsystem: 05. Canvas Items Assigned To: marc_culler
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2024-10-14 13:51:29
Resolution: Fixed Closed By: marc_culler
    Closed on: 2024-10-14 13:51:29
Description:
Hi,
on macOS12/Intel
the following script which create 3 toplevels with some rectangle items inside embed a 'push' button inside . 
there's a <KeyPress-Return>/<KeyRelease-Return> binding for the 'push' button.
When pressing/releasing Enter, not all canvas items are re-coloured.

if the update idletasks is removed, not all the keyPress/keyRelease events are honoured.

package require Tk

ttk::button .b -text push

set y 0
for {set y 1} {$y<4} {incr y 1} {
    set m$y [toplevel .n$y]
    canvas .n$y.c
    for {set x 1} {$x<500} {incr x 5} {
                .n$y.c create rectangle 0 $x 100 [expr {$x + 5}] -fill black -tags tag$y$x
    }
    grid .n$y.c -sticky news
    grid columnconfigure .n$y all -weight 1
    grid rowconfigure .n$y all -weight 1
}

grid .b -sticky news
focus -force .b

bind .b <ButtonPress-1> {doColorchange red}
bind .b <ButtonRelease-1> {doColorchange yellow}
bind . <KeyPress-Return> {doColorchange red}
bind . <KeyRelease-Return> {doColorchange yellow}

proc doColorchange {colour} {
        for {set x 1} {$x<500} {incr x} {
            .n1.c itemconfigure tag1$x -fill $colour
        }

        for {set x 1} {$x<500} {incr x} {
            .n2.c itemconfigure tag2$x -fill $colour
        }

        for {set x 1} {$x<500} {incr x} {
            .n3.c itemconfigure tag3$x -fill $colour
        }
        update idletasks
}


best regards,
nicolas
User Comments: marc_culler (claiming to be Marc Culler) added on 2024-10-14 13:51:29:
This is fixed by commit 47f17f3d.  Closing the ticket.

nab added on 2024-10-12 21:50:31:
yes that was it... sorry for the noise.

it seems very good to me now !!

best regards,
nicolas

marc_culler (claiming to be Marc Culler) added on 2024-10-12 21:48:36:
Nicolas, I think you must be calling wm geometry in two places.  Maybe
you could search you code for calls to wm geometry?

marc_culler (claiming to be Marc Culler) added on 2024-10-12 21:26:17:
Hi Nicolas,

That sounds very strange to me.  I can't imagine how the window could get
in the center of the screen.  The default geometry for the root window is
200x200+5+30.  That is in the upper left corner.  How could your root
window end up in the middle?  Also, if you withdraw or deiconify a window
the geometry doesn't change.

In my little script with the blue window if I use
wm geometry . 600x300
then the window appears in a different place, near the center, and I don't
see any movement of the window.

Could you print out the geometry before and after you deiconify the window?

nab added on 2024-10-12 21:12:43:
thank you Marc,
the root window does not flash anymore and the black window no longer appears.

they still one little thing though. I'm using wm geometry %s +%d+%d in order to display my main window at a desired position.

what I see is my app appearing at center of the screen, then moved to desired position. 

best regards,
Nicolas

marc_culler (claiming to be Marc Culler) added on 2024-10-12 20:04:58:
Hi Micolas, I think I finally was able to figure out how to make the root
window not flash on the screen when it was immediately withdrawn.

Can you please test with your app?

At least I can say that this works:

package require Tk
wm withdraw .
. configure -bg blue
wm geometry . 600x300
wm deiconify .

Also, the fossil diff -tk command no longer flashes a small root window on
the screen.

marc_culler (claiming to be Marc Culler) added on 2024-10-12 18:21:32:
Are you using . for the main window of your app?

Is the black color the background color of your main window, or do you
think it is black because nothing has been drawn on it?

nab added on 2024-10-12 18:07:38:
I think it's . that I'm seeing.
I'm using:
    Tcl_EvalEx(x->interp, "package require tk;\
               wm withdraw . ;\
               update idletasks;\
                ", -1, TCL_EVAL_DIRECT);

after withdraw the window size change and widgets are pushed in it.
when deiconify is called, . appears at the good size but black for a brief moment before widgets appears.

++

marc_culler (claiming to be Marc Culler) added on 2024-10-12 17:42:21:
Is the white window the same window which then turns black and which then
gets widgets drawin in it?

Does the white window change size as part of this process?

nab added on 2024-10-12 17:19:43:
Hi Marc,
woooow !! this is what I was waiting for a long time :)

my script is behaving right now, thanks.
but moreover in my app previously to this commit, I sometimes needs to move mouse over certain area for text/images to be displayed. Now it seems to not happen anymore.

congrats !!

one side effect, when I launch my app a white Tk window briefly appears, and before to be drawn, my app appears full black (then widgets are displayed).

best regards,
Nicolas

marc_culler (claiming to be Marc Culler) added on 2024-10-12 16:52:05:
Nicolas, I think this is now fixed in the tip of the aqua_color branch.
Would you please test this script and your app with that branch? Thanks.

nab added on 2024-09-23 15:12:01:
Hi Marc,
I've reduced to 50 the number of displayed items and still the same result, not all items are yellow coloured on keyRelease.


it's true that with your proposal for the doColorchange proc rewrite, it's better.
But it's not as easy in real app to have update idletasks written as you did.

Now, I've rewrote the script and let my finger on the Return button.
the key repetition (so keyRelease) is not applied to all 3 toplevels and this is my real concern.
sometimes the two firsts toplevels receive the repetition, sometimes the two latest.





package require Tk

ttk::button .b -text push

set y 0
for {set y 1} {$y<4} {incr y 1} {
    set m$y [toplevel .n$y]
    canvas .n$y.c
    for {set x 1} {$x<50} {incr x 5} {
                .n$y.c create rectangle 0 [expr {($x*5) + 5}] 100 [expr {($x*5) + 15}] -fill black -tags tag$y$x
    }
    grid .n$y.c -sticky news
    grid columnconfigure .n$y all -weight 1
    grid rowconfigure .n$y all -weight 1
}

grid .b -sticky news
focus -force .b

bind .b <ButtonPress-1> {doColorchange red}
bind .b <ButtonRelease-1> {doColorchange yellow}
bind . <KeyPress-Return> {doColorchange red}
bind . <KeyRelease-Return> {doColorchange yellow}

proc doColorchange {color} {
    for {set x 1} {$x<50} {incr x} {
    .n1.c itemconfigure tag1$x -fill $color
    .n2.c itemconfigure tag2$x -fill $color
    .n3.c itemconfigure tag3$x -fill $color
    update idletasks
    }
}


++
Nicolas

nab added on 2024-08-02 10:57:50:
Hi Marc,
Thank you for your analysis.
I don't have access to a computer right now in order to test... but I'm not sure that the number of items is the culprit. 

When I'll come back to my computer I'll test with less items because maybe the issue is the multi toplevels.

In my real app I've splited toplevel as I've a timer which trigger the full toplevel update each seconds.
And it's under that circumstances that I saw that not all events are processed.

If all itemconfigure is triggering an update idletasks, maybe it would be nice to have a new command like itemconfigureList (like Csaba did for tableList).

Best regards, 
Nicolas

marc_culler (claiming to be Marc Culler) added on 2024-07-31 17:08:10:
I have changed my mind.  I think this is a canvas issue, after all.  The
script puts a huge load on the canvases by creating 500 tags on each canvas
and then asking all 1500 tags to be updated with a call to itemconfig.  I
think that will create 1500 idle tasks, which are then all processed in the
call to update idletasks.  I find that the following modified version of
the proc doColorChange behaves better.  It handles one idle task for each
canvas before issuing the next batch of 3 color changes.

proc doColorchange {color} {
    for {set x 1} {$x<500} {incr x} {
	.n1.c itemconfigure tag1$x -fill $color
	.n2.c itemconfigure tag2$x -fill $color
	.n3.c itemconfigure tag3$x -fill $color
        update idletasks
        }
}

I think when you are putting huge loads on your canvas widgets
you may have to provide some guidance in the form of calls to
update idletasks.

marc_culler added on 2024-07-29 13:45:42:
I don't think this has anything to do with the canvas widget
or with the recent drawing changes.  What I do suspect is a
macOS quirk.  Apple does not send a key release event when a key
repeats.  There is a separate field in the Apple event to
indicate that the key is repeating.  Tk adds a release Xevent
to the Tcl event queue.  It appears that those added events
can get dropped in the case where multiple windows have bindings
for them.  I would guess that the problem is in tkBind.c, which
unfortunately has become a black box.

marc_culler (claiming to be Marc Culler) added on 2024-07-29 11:21:32:
OK.  I see the issue now.  It is not that some of the small rectangles do
not change color correctly.  The entire window ignores the key press or
release.  This never happens with the button.

griffin added on 2024-07-29 04:07:23:

I suspect there is a race depending on the speed of your particular processor and the setting of the auto-repeat in your environment. I see subtly different behavior depending on the Key Repeat and Delay Until Repeat setting.

(Intel dual-core i7/macOS12.7.5)

In general, I find explicit use of "update" (in any form) is more often a problem instead of a solution.


marc_culler (claiming to be Marc Culler) added on 2024-07-29 02:26:40:
I can't reproduce this on my Intel macbook air running macOS 14 with the
current tips of the main branch of Tcl and Tk.

When I press the button all of the small rectangles turn red.  Wnen I
release it they all turn yellow.