Mercurial > hg > xemacs-beta
changeset 4968:4d35e52790f8
fix crash in glyph-cachels
-------------------- ChangeLog entries follow: --------------------
src/ChangeLog addition:
2010-02-03 Ben Wing <ben@xemacs.org>
* frame.c (Fmake_frame):
* glyphs.c:
* glyphs.c (NUM_PRECACHED_GLYPHS):
* glyphs.c (get_glyph_cachel_index):
* glyphs.c (FROB):
* glyphs.c (mark_glyph_cachels_as_not_updated):
* redisplay.c (regenerate_window):
* redisplay.c (redisplay_window):
When creating a frame, call reset_glyph_cachels on the minibuffer
window as well as the root window. Fixes a crash due to other
glyphs (e.g. the gutter glyph) getting in the glyph cachel before
the pre-cached glyphs that are supposed to have fixed indices
(continuation-glyph, truncation-glyph, etc.).
Add a bunch of asserts to make sure that the glyph cachels always
properly contain the pre-cached glyphs.
author | Ben Wing <ben@xemacs.org> |
---|---|
date | Wed, 03 Feb 2010 21:06:14 -0600 |
parents | 0d4c9d0f6a8d |
children | cbe181529c34 |
files | src/ChangeLog src/frame.c src/glyphs.c src/redisplay.c |
diffstat | 4 files changed, 110 insertions(+), 18 deletions(-) [+] |
line wrap: on
line diff
--- a/src/ChangeLog Wed Feb 03 20:51:18 2010 -0600 +++ b/src/ChangeLog Wed Feb 03 21:06:14 2010 -0600 @@ -1,3 +1,22 @@ +2010-02-03 Ben Wing <ben@xemacs.org> + + * frame.c (Fmake_frame): + * glyphs.c: + * glyphs.c (NUM_PRECACHED_GLYPHS): + * glyphs.c (get_glyph_cachel_index): + * glyphs.c (FROB): + * glyphs.c (mark_glyph_cachels_as_not_updated): + * redisplay.c (regenerate_window): + * redisplay.c (redisplay_window): + When creating a frame, call reset_glyph_cachels on the minibuffer + window as well as the root window. Fixes a crash due to other + glyphs (e.g. the gutter glyph) getting in the glyph cachel before + the pre-cached glyphs that are supposed to have fixed indices + (continuation-glyph, truncation-glyph, etc.). + + Add a bunch of asserts to make sure that the glyph cachels always + properly contain the pre-cached glyphs. + 2010-02-03 Ben Wing <ben@xemacs.org> * device-x.c (x_get_resource_prefix):
--- a/src/frame.c Wed Feb 03 20:51:18 2010 -0600 +++ b/src/frame.c Wed Feb 03 21:06:14 2010 -0600 @@ -585,6 +585,8 @@ update_frame_window_mirror (f); + /* #### Do we need to be calling reset_face_cachels here, and then again + down below? */ if (initialized && !DEVICE_STREAM_P (d)) { if (!NILP (f->minibuffer_window)) @@ -642,8 +644,19 @@ things. */ init_frame_toolbars (f); #endif + /* Added this assert recently (2-1-10); seems there should be only + two windows, root and minibufer. Probably we should just be + calling reset_*_cachels on the root window directly instead of the + selected window, but I want to make sure they are always the + same. --ben */ + assert (EQ (FRAME_SELECTED_WINDOW (f), f->root_window)); reset_face_cachels (XWINDOW (FRAME_SELECTED_WINDOW (f))); reset_glyph_cachels (XWINDOW (FRAME_SELECTED_WINDOW (f))); + if (!NILP (f->minibuffer_window)) + { + reset_face_cachels (XWINDOW (f->minibuffer_window)); + reset_glyph_cachels (XWINDOW (f->minibuffer_window)); + } change_frame_size (f, f->height, f->width, 0); }
--- a/src/glyphs.c Wed Feb 03 20:51:18 2010 -0600 +++ b/src/glyphs.c Wed Feb 03 21:06:14 2010 -0600 @@ -4276,9 +4276,19 @@ /***************************************************************************** - * glyph cachel functions * + * glyph cachel functions * *****************************************************************************/ +#define NUM_PRECACHED_GLYPHS 6 +#define LOOP_OVER_PRECACHED_GLYPHS \ + FROB (Vcontinuation_glyph, CONT_GLYPH_INDEX) \ + FROB (Vtruncation_glyph, TRUN_GLYPH_INDEX) \ + FROB (Vhscroll_glyph, HSCROLL_GLYPH_INDEX) \ + FROB (Vcontrol_arrow_glyph, CONTROL_GLYPH_INDEX) \ + FROB (Voctal_escape_glyph, OCT_ESC_GLYPH_INDEX) \ + FROB (Vinvisible_text_glyph, INVIS_GLYPH_INDEX) + + /* #### All of this is 95% copied from face cachels. Consider consolidating. @@ -4352,6 +4362,27 @@ Dynarr_add (w->glyph_cachels, new_cachel); } +#ifdef ERROR_CHECK_GLYPHS + +/* The precached glyphs should always occur in slots 0 - 5, with each glyph in the + slot reserved for it. Meanwhile any other glyphs should always occur in slots + 6 or greater. */ +static void +verify_glyph_index (Lisp_Object glyph, glyph_index idx) +{ + if (0) + ; +#define FROB(glyph_obj, gindex) \ + else if (EQ (glyph, glyph_obj)) \ + assert (gindex == idx); + LOOP_OVER_PRECACHED_GLYPHS + else + assert (idx >= NUM_PRECACHED_GLYPHS); +#undef FROB +} + +#endif /* ERROR_CHECK_GLYPHS */ + glyph_index get_glyph_cachel_index (struct window *w, Lisp_Object glyph) { @@ -4367,6 +4398,9 @@ if (EQ (cachel->glyph, glyph) && !NILP (glyph)) { +#ifdef ERROR_CHECK_GLYPHS + verify_glyph_index (glyph, elt); +#endif /* ERROR_CHECK_GLYPHS */ update_glyph_cachel_data (w, glyph, cachel); return elt; } @@ -4381,12 +4415,10 @@ reset_glyph_cachels (struct window *w) { Dynarr_reset (w->glyph_cachels); - get_glyph_cachel_index (w, Vcontinuation_glyph); - get_glyph_cachel_index (w, Vtruncation_glyph); - get_glyph_cachel_index (w, Vhscroll_glyph); - get_glyph_cachel_index (w, Vcontrol_arrow_glyph); - get_glyph_cachel_index (w, Voctal_escape_glyph); - get_glyph_cachel_index (w, Vinvisible_text_glyph); +#define FROB(glyph_obj, gindex) \ + get_glyph_cachel_index (w, glyph_obj); + LOOP_OVER_PRECACHED_GLYPHS +#undef FROB } void @@ -4394,19 +4426,18 @@ { int elt; + /* A previous bug resulted from the glyph cachels never getting reset + in the minibuffer window after creation, and another glyph added before + we got a chance to add the six normal glyphs that should go first, and + we got called with only one glyph present. */ + assert (Dynarr_length (w->glyph_cachels) >= NUM_PRECACHED_GLYPHS); /* We need to have a dirty flag to tell if the glyph has changed. We can check to see if each glyph variable is actually a completely different glyph, though. */ #define FROB(glyph_obj, gindex) \ update_glyph_cachel_data (w, glyph_obj, \ - Dynarr_atp (w->glyph_cachels, gindex)) - - FROB (Vcontinuation_glyph, CONT_GLYPH_INDEX); - FROB (Vtruncation_glyph, TRUN_GLYPH_INDEX); - FROB (Vhscroll_glyph, HSCROLL_GLYPH_INDEX); - FROB (Vcontrol_arrow_glyph, CONTROL_GLYPH_INDEX); - FROB (Voctal_escape_glyph, OCT_ESC_GLYPH_INDEX); - FROB (Vinvisible_text_glyph, INVIS_GLYPH_INDEX); + Dynarr_atp (w->glyph_cachels, gindex)); + LOOP_OVER_PRECACHED_GLYPHS #undef FROB for (elt = 0; elt < Dynarr_length (w->glyph_cachels); elt++) @@ -4449,7 +4480,7 @@ /***************************************************************************** - * subwindow cachel functions * + * subwindow cachel functions * *****************************************************************************/ /* Subwindows are curious in that you have to physically unmap them to not display them. It is problematic deciding what to do in @@ -4546,7 +4577,7 @@ } /***************************************************************************** - * subwindow exposure ignorance * + * subwindow exposure ignorance * *****************************************************************************/ /* when we unmap subwindows the associated window system will generate expose events. This we do not want as redisplay already copes with
--- a/src/redisplay.c Wed Feb 03 20:51:18 2010 -0600 +++ b/src/redisplay.c Wed Feb 03 21:06:14 2010 -0600 @@ -5471,6 +5471,17 @@ Dynarr_reset (dla); w->max_line_len = 0; + /* Added 2-1-10 -- we should never have empty face or glyph cachels + because we initialized them at startup and the only way to reduce + their number is through calling reset_face_cachels() or + reset_glyph_cachels(), which as a side effect sets up a number of + standard entries */ + assert (Dynarr_length (w->face_cachels)); + assert (Dynarr_length (w->glyph_cachels)); + +#if 0 + /* #### Delete this code sometime later than 2-1-10 when we're sure it's + not needed */ /* Normally these get updated in redisplay_window but it is possible for this function to get called from some other points where that update may not have occurred. This acts as a safety check. */ @@ -5478,6 +5489,7 @@ reset_face_cachels (w); if (!Dynarr_length (w->glyph_cachels)) reset_glyph_cachels (w); +#endif Fset_marker (w->start[type], make_int (start_pos), w->buffer); Fset_marker (w->pointm[type], make_int (point), w->buffer); @@ -6288,10 +6300,22 @@ } Fset_marker (w->pointm[DESIRED_DISP], make_int (pointm), the_buffer); + /* Added 2-1-10 -- we should never have empty face or glyph cachels + because we initialized them at startup and the only way to reduce + their number is through calling reset_face_cachels() or + reset_glyph_cachels(), which as a side effect sets up a number of + standard entries */ + assert (Dynarr_length (w->face_cachels)); + assert (Dynarr_length (w->glyph_cachels)); + /* If the buffer has changed we have to invalidate all of our face cache elements. */ if ((!echo_active && b != window_display_buffer (w)) +#if 0 + /* #### Delete this code sometime later than 2-1-10 when we're sure it's + not needed */ || !Dynarr_length (w->face_cachels) +#endif || f->faces_changed) reset_face_cachels (w); else @@ -6301,7 +6325,12 @@ the cache purely because glyphs have changed - this is now handled by the dirty flag.*/ if ((!echo_active && b != window_display_buffer (w)) - || !Dynarr_length (w->glyph_cachels) || f->faces_changed) +#if 0 + /* #### Delete this code sometime later than 2-1-10 when we're sure it's + not needed */ + || !Dynarr_length (w->glyph_cachels) +#endif + || f->faces_changed) reset_glyph_cachels (w); else mark_glyph_cachels_as_not_updated (w);