From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17632 invoked by alias); 17 Feb 2015 10:23:46 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17484 invoked by uid 89); 17 Feb 2015 10:23:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 17 Feb 2015 10:23:43 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1HANcN3004697 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 17 Feb 2015 05:23:39 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t1HANawm011325; Tue, 17 Feb 2015 05:23:37 -0500 Message-ID: <54E316A8.9070705@redhat.com> Date: Tue, 17 Feb 2015 10:23:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Patrick Palka CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys References: <1420689885-31156-1-git-send-email-patrick@parcs.ath.cx> <1420689885-31156-2-git-send-email-patrick@parcs.ath.cx> <54AE6A19.9070901@redhat.com> <54AE848B.1050606@redhat.com> <54E27241.6010302@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-02/txt/msg00421.txt.bz2 On 02/17/2015 12:52 AM, Patrick Palka wrote: > On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves wrote: > Good point... When leaving TUI mode, we disable ncurses via endwin(). > When re-entering TUI mode, we are re-enabling ncurses through the > first call to refresh(). If we manage to sync ncurses' knowledge of > the terminal dimensions (via the call to resize_terminall() in > tui_resize_all() I believe) before re-enabling ncurses then a > KEY_RESIZE should not get placed into the queue. Exactly. > >> >> ISTM that we're just resizing too late still. Indeed, the patch below, >> which makes us resize earlier, fixes the issue for me too. Are you >> aware of any other use case that might cause KEY_RESIZE? > > Nice, it fixes the issue for me too. No more KEY_RESIZE keys. And I > am not aware of another way to trigger KEY_RESIZE keys. Alright, great. >> (Note: Whatever the order of calls in tui_enable, we'll potentially be >> showing stale contents momentarily and then overwriting with correct ones. >> We get that today too. IMO, today's even worse, as it can show windows with >> the wrong size for a moment, and that might be more visible flicker than showing >> the wrong contents with the correct border sizes already. ISTM the fix for >> that would be to decouple "update windows' contents" from actually >> wrefresh/display'ing windows. That looks like much more work than worth >> it though. I can only see this if I step through the code within tui_enable. >> In a normal not-being-debugged run, I can't notice any flicker.) > > I have never noticed such flickering. But it is does not surprise me > that TUI has the potential to flicker like hell, see > https://sourceware.org/bugzilla/show_bug.cgi?id=13378 Yeah... > The patch makes a lot of sense to me... I appreciate your taking such > an in-depth look at such a tedious issue! :-) These discussions have resulted in several fixes (from you), so I think it's been well worth it, IMO. Thank _you_ for poking at the TUI. > Is it OK to commit patches > 1 and 3 of this series once you commit the below patch? Yes please. I've now pushed mine in. > > (As a side note it boggles my mind that [vertically] resizing the > terminal while inside TUI is horribly broken, yet indirectly resizing > TUI by temporarily exiting TUI, resizing within the CLI then > re-entering TUI works just fine. Both of these methods ought to be > running the same resizing logic! Something tells me most of > tui_resize_all() is unnecessary and broken.) I think that that's missing is integrating SIGWINCH handling with the event loop. That is, nothing is waking up the event loop to react to SIGWINCH and do the resize, so the resize ends up happening on next IO (next key press). Something along these lines: In the TUI setup code where we install the SIGWINCH signal handler create a new event loop source for the SIGWINCH signal: sigwinch_token = create_async_signal_handler (async_sigwinch_handler, NULL); Have the real SIGWINCH handler mark that async event source: static void handle_sigwinch (int sig) { mark_async_signal_handler (sigwinch_token); } And then when that event source's callback is called by the event loop, resize the TUI: static void async_sigwinch_handler (gdb_client_data arg) { /* Trigger TUI resize here. */ } Would you like to try that? Thanks, Pedro Alves