* [RFC] volatile fields of struct frame
@ 2012-12-26 12:18 Dmitry Antipov
2012-12-27 17:59 ` Eli Zaretskii
0 siblings, 1 reply; 2+ messages in thread
From: Dmitry Antipov @ 2012-12-26 12:18 UTC (permalink / raw)
To: Emacs development discussions; +Cc: Eli Zaretskii, Jan Djärv
[-- Attachment #1: Type: text/plain, Size: 240 bytes --]
Since async input is gone, should we try to unify async_visible and
async_iconified fields of struct frame with their regular counterparts?
Here is what I'm now trying for X; comments around NS and W32 stuff
are highly appreciated.
Dmitry
[-- Attachment #2: frame_async.patch --]
[-- Type: text/plain, Size: 15072 bytes --]
=== modified file 'src/frame.c'
--- src/frame.c 2012-12-26 09:40:45 +0000
+++ src/frame.c 2012-12-26 11:55:23 +0000
@@ -499,9 +499,7 @@
tty_frame_count = 1;
fset_name (f, build_pure_c_string ("F1"));
-
- f->visible = 1;
- f->async_visible = 1;
+ frame_set_visible (f, 1);
f->output_method = terminal->type;
f->terminal = terminal;
@@ -539,9 +537,8 @@
Vframe_list = Fcons (frame, Vframe_list);
fset_name (f, make_formatted_string (name, "F%"pMd, ++tty_frame_count));
+ frame_set_visible (f, 1);
- f->visible = 1; /* FRAME_SET_VISIBLE wd set frame_garbaged. */
- f->async_visible = 1; /* Don't let visible be cleared later. */
f->terminal = terminal;
f->terminal->reference_count++;
#ifdef MSDOS
@@ -565,7 +562,7 @@
/* Set the top frame to the newly created frame. */
if (FRAMEP (FRAME_TTY (f)->top_frame)
&& FRAME_LIVE_P (XFRAME (FRAME_TTY (f)->top_frame)))
- XFRAME (FRAME_TTY (f)->top_frame)->async_visible = 2; /* obscured */
+ frame_set_visible (XFRAME (FRAME_TTY (f)->top_frame), 2);
FRAME_TTY (f)->top_frame = frame;
@@ -806,8 +803,8 @@
{
if (FRAMEP (FRAME_TTY (XFRAME (frame))->top_frame))
/* Mark previously displayed frame as now obscured. */
- XFRAME (FRAME_TTY (XFRAME (frame))->top_frame)->async_visible = 2;
- XFRAME (frame)->async_visible = 1;
+ frame_set_visible (XFRAME (FRAME_TTY (XFRAME (frame))->top_frame), 2);
+ frame_set_visible (XFRAME (frame), 1);
FRAME_TTY (XFRAME (frame))->top_frame = frame;
}
@@ -1231,7 +1228,7 @@
fset_root_window (f, Qnil);
Vframe_list = Fdelq (frame, Vframe_list);
- FRAME_SET_VISIBLE (f, 0);
+ frame_set_visible (f, 0);
/* Allow the vector of menu bar contents to be freed in the next
garbage collection. The frame object itself may not be garbage
=== modified file 'src/frame.h'
--- src/frame.h 2012-12-26 09:40:45 +0000
+++ src/frame.h 2012-12-26 11:54:55 +0000
@@ -353,46 +353,30 @@
unsigned int external_menu_bar : 1;
#endif
- /* visible is nonzero if the frame is currently displayed; we check
+ /* Next two bitfields are mutually exclusive. They might both be
+ zero if the frame has been made invisible without an icon. */
+
+ /* Nonzero if the frame is currently displayed; we check
it to see if we should bother updating the frame's contents.
- DON'T SET IT DIRECTLY; instead, use FRAME_SET_VISIBLE.
Note that, since invisible frames aren't updated, whenever a
- frame becomes visible again, it must be marked as garbaged. The
- FRAME_SAMPLE_VISIBILITY macro takes care of this.
+ frame becomes visible again, it must be marked as garbaged.
On ttys and on Windows NT/9X, to avoid wasting effort updating
visible frames that are actually completely obscured by other
windows on the display, we bend the meaning of visible slightly:
- if greater than 1, then the frame is obscured - we still consider
+ if equal to 2, then the frame is obscured - we still consider
it to be "visible" as seen from lisp, but we don't bother
updating it. We must take care to garbage the frame when it
- ceases to be obscured though.
-
- iconified is nonzero if the frame is currently iconified.
-
- Asynchronous input handlers should NOT change these directly;
- instead, they should change async_visible or async_iconified, and
- let the FRAME_SAMPLE_VISIBILITY macro set visible and iconified
- at the next redisplay.
-
- These should probably be considered read-only by everyone except
- FRAME_SAMPLE_VISIBILITY.
-
- These two are mutually exclusive. They might both be zero, if the
- frame has been made invisible without an icon. */
+ ceases to be obscured though. See frame_set_visible below. */
unsigned visible : 2;
+
+ /* Nonzero if the frame is currently iconified. Do not
+ set this directly, use frame_set_iconified instead. */
unsigned iconified : 1;
- /* Let's not use bitfields for volatile variables. */
-
- /* Asynchronous input handlers change these, and
- FRAME_SAMPLE_VISIBILITY copies them into visible and iconified.
- See FRAME_SAMPLE_VISIBILITY, below. */
- volatile char async_visible, async_iconified;
-
/* Nonzero if this frame should be redrawn. */
- volatile char garbaged;
+ unsigned garbaged : 1;
/* True if frame actually has a minibuffer window on it.
0 if using a minibuffer window that isn't on this frame. */
@@ -722,9 +706,10 @@
/* Nonzero if frame F is currently iconified. */
#define FRAME_ICONIFIED_P(f) (f)->iconified
-#define FRAME_SET_VISIBLE(f,p) \
- ((f)->async_visible = (p), FRAME_SAMPLE_VISIBILITY (f))
+/* Mark frame F as currently garbaged. */
#define SET_FRAME_GARBAGED(f) (frame_garbaged = 1, f->garbaged = 1)
+
+/* Nonzero if frame F is currently garbaged. */
#define FRAME_GARBAGED_P(f) (f)->garbaged
/* Nonzero means do not allow splitting this frame's window. */
@@ -870,39 +855,6 @@
#define FRAME_MESSAGE_BUF_SIZE(f) (((int) FRAME_COLS (f)) * 4)
-/* Emacs's redisplay code could become confused if a frame's
- visibility changes at arbitrary times. For example, if a frame is
- visible while the desired glyphs are being built, but becomes
- invisible before they are updated, then some rows of the
- desired_glyphs will be left marked as enabled after redisplay is
- complete, which should never happen. The next time the frame
- becomes visible, redisplay will probably barf.
-
- Currently, there are no similar situations involving iconified, but
- the principle is the same.
-
- So instead of having asynchronous input handlers directly set and
- clear the frame's visibility and iconification flags, they just set
- the async_visible and async_iconified flags; the redisplay code
- calls the FRAME_SAMPLE_VISIBILITY macro before doing any redisplay,
- which sets visible and iconified from their asynchronous
- counterparts.
-
- Synchronous code must use the FRAME_SET_VISIBLE macro.
-
- Also, if a frame used to be invisible, but has just become visible,
- it must be marked as garbaged, since redisplay hasn't been keeping
- up its contents.
-
- Note that a tty frame is visible if and only if it is the topmost
- frame. */
-
-#define FRAME_SAMPLE_VISIBILITY(f) \
- (((f)->async_visible && (f)->visible != (f)->async_visible) ? \
- SET_FRAME_GARBAGED (f) : 0, \
- (f)->visible = (f)->async_visible, \
- (f)->iconified = (f)->async_iconified)
-
#define CHECK_FRAME(x) \
CHECK_TYPE (FRAMEP (x), Qframep, x)
@@ -942,6 +894,38 @@
} \
} while (0)
+/* Set visibility of frame F, marking F garbaged if
+ needed. Return an old value of F's visibility. */
+
+FRAME_INLINE int
+frame_set_visible (struct frame *f, int v)
+{
+ int oldv = f->visible;
+
+ eassert (0 <= v && v <= 2);
+
+ f->visible = v;
+ if ((oldv == 0 || oldv == 2) && v == 1)
+ SET_FRAME_GARBAGED (f);
+ return oldv;
+}
+
+/* Likewise for iconified, but never mark F as garbaged. */
+
+FRAME_INLINE int
+frame_set_iconified (struct frame *f, int i)
+{
+ int oldi = f->iconified;
+
+ eassert (0 <= i && i <= 1);
+
+ f->iconified = i;
+ return oldi;
+}
+
+/* No-op just now. */
+#define FRAME_SAMPLE_VISIBILITY(f) ((void) f)
+
extern Lisp_Object Qframep, Qframe_live_p;
extern Lisp_Object Qtty, Qtty_type;
extern Lisp_Object Qtty_color_mode;
=== modified file 'src/gtkutil.c'
--- src/gtkutil.c 2012-12-25 15:07:59 +0000
+++ src/gtkutil.c 2012-12-26 11:42:03 +0000
@@ -983,7 +983,7 @@
size as fast as possible.
For unmapped windows, we can set rows/cols. When
the frame is mapped again we will (hopefully) get the correct size. */
- if (f->async_visible)
+ if (f->visible)
{
/* Must call this to flush out events */
(void)gtk_events_pending ();
=== modified file 'src/term.c'
--- src/term.c 2012-12-04 15:15:30 +0000
+++ src/term.c 2012-12-26 11:55:51 +0000
@@ -2374,7 +2374,7 @@
t->display_info.tty->output = 0;
if (FRAMEP (t->display_info.tty->top_frame))
- FRAME_SET_VISIBLE (XFRAME (t->display_info.tty->top_frame), 0);
+ frame_set_visible (XFRAME (t->display_info.tty->top_frame), 0);
}
@@ -2444,7 +2444,7 @@
get_tty_size (fileno (t->display_info.tty->input), &width, &height);
if (width != old_width || height != old_height)
change_frame_size (f, height, width, 0, 0, 0);
- FRAME_SET_VISIBLE (XFRAME (t->display_info.tty->top_frame), 1);
+ frame_set_visible (XFRAME (t->display_info.tty->top_frame), 1);
}
set_tty_hooks (t);
=== modified file 'src/xterm.c'
--- src/xterm.c 2012-12-12 15:33:30 +0000
+++ src/xterm.c 2012-12-26 11:54:17 +0000
@@ -6106,10 +6106,9 @@
/* Gnome shell does not iconify us when C-z is pressed. It hides
the frame. So if our state says we aren't hidden anymore,
treat it as deiconified. */
- if (! f->async_iconified)
+ if (!frame_set_iconified (f, 0))
SET_FRAME_GARBAGED (f);
- f->async_visible = 1;
- f->async_iconified = 0;
+ frame_set_visible (f, 1);
f->output_data.x->has_been_visible = 1;
f->output_data.x->net_wm_state_hidden_seen = 0;
inev.ie.kind = DEICONIFY_EVENT;
@@ -6150,12 +6149,10 @@
event.xexpose.width, event.xexpose.height,
FALSE);
#endif
- if (f->async_visible == 0)
+ if (frame_set_visible (f, 1) == 0)
{
- f->async_visible = 1;
- f->async_iconified = 0;
+ frame_set_iconified (f, 0);
f->output_data.x->has_been_visible = 1;
- SET_FRAME_GARBAGED (f);
}
else
expose_frame (f,
@@ -6233,17 +6230,14 @@
/* While a frame is unmapped, display generation is
disabled; you don't want to spend time updating a
display that won't ever be seen. */
- f->async_visible = 0;
/* We can't distinguish, from the event, whether the window
has become iconified or invisible. So assume, if it
was previously visible, than now it is iconified.
But x_make_frame_invisible clears both
the visible flag and the iconified flag;
and that way, we know the window is not iconified now. */
- if (FRAME_VISIBLE_P (f) || FRAME_ICONIFIED_P (f))
+ if (frame_set_visible (f, 0) || frame_set_iconified (f, 1))
{
- f->async_iconified = 1;
-
inev.ie.kind = ICONIFY_EVENT;
XSETFRAME (inev.ie.frame_or_window, f);
}
@@ -6268,7 +6262,7 @@
because that stops redrawing on Expose events. This looks
bad if we are called from a recursive event loop
(x_dispatch_event), for example when a dialog is up. */
- if (! f->async_iconified)
+ if (!f->iconified)
SET_FRAME_GARBAGED (f);
/* Check if fullscreen was specified before we where mapped the
@@ -6276,11 +6270,10 @@
if (!f->output_data.x->has_been_visible)
x_check_fullscreen (f);
- f->async_visible = 1;
- f->async_iconified = 0;
+ frame_set_visible (f, 1);
f->output_data.x->has_been_visible = 1;
- if (f->iconified)
+ if (frame_set_iconified (f, 0))
{
inev.ie.kind = DEICONIFY_EVENT;
XSETFRAME (inev.ie.frame_or_window, f);
@@ -8527,7 +8520,7 @@
static void
XTfullscreen_hook (FRAME_PTR f)
{
- if (f->async_visible)
+ if (f->visible)
{
block_input ();
x_check_fullscreen (f);
@@ -8791,7 +8784,7 @@
/* But the ConfigureNotify may in fact never arrive, and then this is
not right if the frame is visible. Instead wait (with timeout)
for the ConfigureNotify. */
- if (f->async_visible)
+ if (f->visible)
x_wait_for_event (f, ConfigureNotify);
else
{
@@ -8903,7 +8896,7 @@
x_raise_frame (struct frame *f)
{
block_input ();
- if (f->async_visible)
+ if (f->visible)
XRaiseWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f));
XFlush (FRAME_X_DISPLAY (f));
@@ -8915,7 +8908,7 @@
static void
x_lower_frame (struct frame *f)
{
- if (f->async_visible)
+ if (f->visible)
{
block_input ();
XLowerWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f));
@@ -8931,7 +8924,7 @@
{
/* See XEmbed Protocol Specification at
http://freedesktop.org/wiki/Specifications/xembed-spec */
- if (f->async_visible)
+ if (f->visible)
xembed_send_message (f, CurrentTime,
XEMBED_REQUEST_FOCUS, 0, 0, 0);
}
@@ -8945,7 +8938,8 @@
http://freedesktop.org/wiki/Specifications/wm-spec */
struct x_display_info *dpyinfo = FRAME_X_DISPLAY_INFO (f);
- if (f->async_visible && wm_supports (f, dpyinfo->Xatom_net_active_window))
+
+ if (f->visible && wm_supports (f, dpyinfo->Xatom_net_active_window))
{
Lisp_Object frame;
XSETFRAME (frame, f);
@@ -9227,10 +9221,8 @@
So we can't win using the usual strategy of letting
FRAME_SAMPLE_VISIBILITY set this. So do it by hand,
and synchronize with the server to make sure we agree. */
- f->visible = 0;
- FRAME_ICONIFIED_P (f) = 0;
- f->async_visible = 0;
- f->async_iconified = 0;
+ frame_set_visible (f, 0);
+ frame_set_iconified (f, 0);
x_sync (f);
@@ -9251,7 +9243,7 @@
if (FRAME_X_DISPLAY_INFO (f)->x_highlight_frame == f)
FRAME_X_DISPLAY_INFO (f)->x_highlight_frame = 0;
- if (f->async_iconified)
+ if (f->iconified)
return;
block_input ();
@@ -9269,10 +9261,8 @@
gtk_widget_show_all (FRAME_GTK_OUTER_WIDGET (f));
gtk_window_iconify (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)));
- f->iconified = 1;
- f->visible = 1;
- f->async_iconified = 1;
- f->async_visible = 0;
+ frame_set_visible (f, 0);
+ frame_set_iconified (f, 1);
unblock_input ();
return;
}
@@ -9289,10 +9279,8 @@
/* The server won't give us any event to indicate
that an invisible frame was changed to an icon,
so we have to record it here. */
- f->iconified = 1;
- f->visible = 1;
- f->async_iconified = 1;
- f->async_visible = 0;
+ frame_set_visible (f, 0);
+ frame_set_iconified (f, 1);
unblock_input ();
return;
}
@@ -9305,9 +9293,8 @@
if (!result)
error ("Can't notify window manager of iconification");
- f->async_iconified = 1;
- f->async_visible = 0;
-
+ frame_set_visible (f, 0);
+ frame_set_iconified (f, 1);
block_input ();
XFlush (FRAME_X_DISPLAY (f));
@@ -9356,8 +9343,8 @@
XMapRaised (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f));
}
- f->async_iconified = 1;
- f->async_visible = 0;
+ frame_set_visible (f, 0);
+ frame_set_iconified (f, 1);
XFlush (FRAME_X_DISPLAY (f));
unblock_input ();
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [RFC] volatile fields of struct frame
2012-12-26 12:18 [RFC] volatile fields of struct frame Dmitry Antipov
@ 2012-12-27 17:59 ` Eli Zaretskii
0 siblings, 0 replies; 2+ messages in thread
From: Eli Zaretskii @ 2012-12-27 17:59 UTC (permalink / raw)
To: Dmitry Antipov; +Cc: jan.h.d, emacs-devel
> Date: Wed, 26 Dec 2012 16:18:59 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: Eli Zaretskii <eliz@gnu.org>, Jan Djärv
> <jan.h.d@swipnet.se>
>
> Since async input is gone, should we try to unify async_visible and
> async_iconified fields of struct frame with their regular counterparts?
Yes.
> === modified file 'src/xterm.c'
> --- src/xterm.c 2012-12-12 15:33:30 +0000
> +++ src/xterm.c 2012-12-26 11:54:17 +0000
> @@ -6106,10 +6106,9 @@
> /* Gnome shell does not iconify us when C-z is pressed. It hides
> the frame. So if our state says we aren't hidden anymore,
> treat it as deiconified. */
> - if (! f->async_iconified)
> + if (!frame_set_iconified (f, 0))
The old code was only testing the flag, the new one also resets it to
zero. Is that correct?
> @@ -6233,17 +6230,14 @@
> /* While a frame is unmapped, display generation is
> disabled; you don't want to spend time updating a
> display that won't ever be seen. */
> - f->async_visible = 0;
> /* We can't distinguish, from the event, whether the window
> has become iconified or invisible. So assume, if it
> was previously visible, than now it is iconified.
> But x_make_frame_invisible clears both
> the visible flag and the iconified flag;
> and that way, we know the window is not iconified now. */
> - if (FRAME_VISIBLE_P (f) || FRAME_ICONIFIED_P (f))
> + if (frame_set_visible (f, 0) || frame_set_iconified (f, 1))
> {
> - f->async_iconified = 1;
> -
Same here.
> - f->async_visible = 1;
> - f->async_iconified = 0;
> + frame_set_visible (f, 1);
The old code was setting only the async_visible flag; why do we need
to set the visible flag instead? why not just remove these 2 lines?
> - if (f->iconified)
> + if (frame_set_iconified (f, 0))
Again, you reset where the old code only tested the flag.
> @@ -9305,9 +9293,8 @@
> if (!result)
> error ("Can't notify window manager of iconification");
>
> - f->async_iconified = 1;
> - f->async_visible = 0;
> -
> + frame_set_visible (f, 0);
> + frame_set_iconified (f, 1);
Why not remove those 2 lines without replacing them?
> @@ -9356,8 +9343,8 @@
> XMapRaised (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f));
> }
>
> - f->async_iconified = 1;
> - f->async_visible = 0;
> + frame_set_visible (f, 0);
> + frame_set_iconified (f, 1);
Same question here.
> Here is what I'm now trying for X; comments around NS and W32 stuff
> are highly appreciated.
Any reason why NS or w32 stuff shouldn't get the same changes?
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-12-27 17:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-26 12:18 [RFC] volatile fields of struct frame Dmitry Antipov
2012-12-27 17:59 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).