Wireshark-dev: Re: [Wireshark-dev] Misaligned columns in hex dump pane with Wireshark 1.10.0rc2

From: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
Date: Tue, 4 Jun 2013 23:51:58 +0200
Hi,

On Tue, Jun 04, 2013 at 10:28:39PM +0200, Jakub Zawadzki wrote:
> On Tue, Jun 04, 2013 at 10:06:05PM +0200, Reinhard Speyerer wrote:
> > I noticed today that columns in the hex dump pane are misaligned when
> > using wireshark 1.10.0rc2 built with the following configuration
> > 
> > [...]
> > 
> > In my preferences I use the default font (Monospace 10).
> > 
> > Has anybody else also observed this behaviour?
> 
> On not monospaced fonts, yes, still this is new for monospace one.
> Can you try some other fixed width font?
> 
> I had once patch to force "alignment" of hexdump for all fonts, 
> I'll try to find it.

Ok, found it. Can you try attached patch?
diff --git ui/gtk/bytes_view.c ui/gtk/bytes_view.c
index 55b9c0c..1bc6840 100644
--- ui/gtk/bytes_view.c
+++ ui/gtk/bytes_view.c
@@ -49,17 +49,26 @@
 
 static GtkWidgetClass *parent_class = NULL;
 
+struct PangoCairoGlyphs {
+	PangoGlyphInfo *glyphs;
+	PangoFont *font;
+	cairo_scaled_font_t *scfont;
+	int num_glyphs;
+};
+
 struct _BytesView
 {
 	GtkWidget widget;
 
-	PangoContext *context;
-
 	PangoFontDescription *font;
 	int font_ascent;
 	int font_descent;
 	int fontsize;
 
+	gboolean font_changed;
+	int font_width;
+	struct PangoCairoGlyphs glyphs[0x80];
+
 	GtkAdjustment *vadj;
 	GtkAdjustment *hadj;
 #if GTK_CHECK_VERSION(3, 0, 0)
@@ -101,7 +110,7 @@ static void bytes_view_adjustment_set(BytesView *);
 static void
 bytes_view_init(BytesView *bv)
 {
-	bv->context = NULL;
+	bv->font_changed = TRUE;
 
 	bv->encoding = PACKET_CHAR_ENC_CHAR_ASCII;
 	bv->format = BYTES_HEX;
@@ -135,10 +144,6 @@ bytes_view_destroy(BytesView *bv)
 		pango_font_description_free(bv->font);
 		bv->font = NULL;
 	}
-	if (bv->context) {
-		g_object_unref(bv->context);
-		bv->context = NULL;
-	}
 }
 
 #if GTK_CHECK_VERSION(3, 0, 0)
@@ -164,27 +169,109 @@ bytes_view_destroy_object(GtkObject *object)
 #endif
 
 static void
-bytes_view_ensure_layout(BytesView *bv)
+bytes_view_recalc_font(BytesView *bv)
 {
-	if (bv->context == NULL) {
-		bv->context = gtk_widget_get_pango_context(GTK_WIDGET(bv));
-		g_object_ref(bv->context);
-
-		{
-		PangoLanguage *lang;
-		PangoFontMetrics *metrics;
-
-			/* vte and xchat does it this way */
-			lang = pango_context_get_language(bv->context);
-			metrics = pango_context_get_metrics(bv->context, bv->font, lang);
-			bv->font_ascent = pango_font_metrics_get_ascent(metrics) / PANGO_SCALE;
-			bv->font_descent = pango_font_metrics_get_descent(metrics) / PANGO_SCALE;
-			pango_font_metrics_unref(metrics);
-
-			bv->fontsize = bv->font_ascent + bv->font_descent;
-		}
-		g_assert(bv->context);
-		bytes_view_adjustment_set(bv);
+	PangoContext *context;
+
+	PangoAttrList *attrs;
+	PangoAttrIterator *iter;
+	unsigned char ch;
+
+	int font_width;
+
+	context = gtk_widget_get_pango_context(GTK_WIDGET(bv));
+	g_object_ref(context);
+
+	{
+	PangoLanguage *lang;
+	PangoFontMetrics *metrics;
+
+		/* vte and xchat does it this way */
+		lang = pango_context_get_language(context);
+		metrics = pango_context_get_metrics(context, bv->font, lang);
+		bv->font_ascent = pango_font_metrics_get_ascent(metrics) / PANGO_SCALE;
+		bv->font_descent = pango_font_metrics_get_descent(metrics) / PANGO_SCALE;
+		pango_font_metrics_unref(metrics);
+
+		bv->fontsize = bv->font_ascent + bv->font_descent;
+	}
+
+	attrs = pango_attr_list_new();
+	pango_attr_list_insert_before(attrs, pango_attr_font_desc_new(bv->font));
+	iter = pango_attr_list_get_iterator(attrs);
+
+	font_width = 0;
+
+	for (ch = 0x20; ch < 0x7f; ch++) {
+		GList *run_list;
+		GList *tmp_list;
+		int glyph_width = 0;
+
+		run_list = pango_itemize(context, &ch, 0, 1, attrs, iter);
+
+		g_assert(run_list->next == NULL);
+
+		for (tmp_list = run_list; tmp_list; tmp_list = tmp_list->next) {
+			PangoGlyphString *glyphs;
+			PangoItem *run_item = tmp_list->data;
+			cairo_scaled_font_t *scaled_font;
+			PangoFont *font;
+			int i;
+
+			/* XXX pango_layout_get_item_properties(run_item, &state->properties); */
+
+			g_assert(run_item->offset == 0);
+
+			glyphs = pango_glyph_string_new();
+			pango_shape(&ch, run_item->length, &run_item->analysis, glyphs);
+
+			for (i = 0; i < glyphs->num_glyphs; i++)
+				glyph_width += glyphs->glyphs[i].geometry.width;
+
+			g_object_ref(run_item->analysis.font);
+
+			font = run_item->analysis.font;
+			scaled_font = pango_cairo_font_get_scaled_font((PangoCairoFont *)font);
+			g_assert(scaled_font != NULL && cairo_scaled_font_status(scaled_font) == CAIRO_STATUS_SUCCESS);
+
+			bv->glyphs[ch].font = font;
+			bv->glyphs[ch].scfont = scaled_font;
+			bv->glyphs[ch].glyphs = glyphs->glyphs;
+			bv->glyphs[ch].num_glyphs = glyphs->num_glyphs;
+
+			pango_item_free(run_item);
+ 		}
+
+		if (font_width < glyph_width)
+			font_width = glyph_width;
+
+		g_list_free(run_list);
+	}
+
+	for (ch = 0x20; ch < 0x7f; ch++) {
+		int i;
+
+		// XXX
+		for (i = 0; i < bv->glyphs[ch].num_glyphs; i++)
+			bv->glyphs[ch].glyphs[i].geometry.width = font_width / bv->glyphs[ch].num_glyphs;
+	}
+
+	bv->font_width = font_width;
+
+	pango_attr_iterator_destroy(iter);
+	pango_attr_list_unref(attrs);
+
+	bytes_view_adjustment_set(bv);
+
+	g_object_unref(context);
+}
+
+static void
+bytes_view_ensure_font(BytesView *bv)
+{
+	if (bv->font_changed) {
+		bv->font_changed = FALSE;
+		bytes_view_recalc_font(bv);
 	}
 }
 
@@ -244,18 +331,12 @@ bytes_view_realize(GtkWidget *widget)
 #else
 	widget->style = gtk_style_attach(widget->style, win);
 #endif
-	bytes_view_ensure_layout(bv);
+	bytes_view_ensure_font(bv);
 }
 
 static void
 bytes_view_unrealize(GtkWidget *widget)
 {
-	BytesView *bv = BYTES_VIEW(widget);
-
-	if (bv->context) {
-		g_object_unref(bv->context);
-		bv->context = NULL;
-	}
 	/* if there are still events in the queue, this'll avoid segfault */
 #if !GTK_CHECK_VERSION(3, 8, 0)
 	gdk_window_set_user_data(gtk_widget_get_window(widget), NULL);
@@ -351,67 +432,128 @@ static GSList *
 _pango_runs_build(BytesView *bv, const char *str, int len)
 {
 	GSList *runs = NULL;
+	int i, j;
 
-	PangoAttrList *attrs;
-	PangoAttrIterator *iter;
+	for (i = 0; i < len;) {
+		const struct PangoCairoGlyphs *cg = &bv->glyphs[(int) str[i]];
+		PangoFont *cur_font = cg->font;
 
-	GList *run_list;
-	GList *tmp_list;
+		struct PangoCairoGlyphs *new_cg = g_slice_new(struct PangoCairoGlyphs);
+		int num_glyphs = 0;
 
-	attrs = pango_attr_list_new();
-	pango_attr_list_insert_before(attrs, pango_attr_font_desc_new(bv->font));
+		for (j = i; j < len; j++) {
+			const struct PangoCairoGlyphs *cjg = &bv->glyphs[(int) str[j]];
 
-	iter = pango_attr_list_get_iterator(attrs);
+			if (cur_font != cjg->font)
+				break;
 
-	run_list = pango_itemize(bv->context, str, 0, len, attrs, iter);
+			num_glyphs += cjg->num_glyphs;
+		}
 
-	for (tmp_list = run_list; tmp_list; tmp_list = tmp_list->next) {
-		PangoLayoutRun *run = g_slice_new(PangoLayoutRun);
-		PangoItem *run_item = (PangoItem *)tmp_list->data;
+		g_object_ref(cur_font);
 
-		run->item = run_item;
+		new_cg->font = cur_font;
+		new_cg->scfont = cg->scfont;
+		new_cg->num_glyphs = num_glyphs;
+		new_cg->glyphs = g_malloc(num_glyphs * sizeof(PangoGlyphInfo));
 
-		/* XXX pango_layout_get_item_properties(run_item, &state->properties); */
+		num_glyphs = 0;
+		for (j = i; j < len; j++) {
+			const struct PangoCairoGlyphs *cjg = &bv->glyphs[(int) str[j]];
+			int k;
 
-		run->glyphs = pango_glyph_string_new();
-		pango_shape(str + run_item->offset, run_item->length, &run_item->analysis, run->glyphs);
+			if (cur_font != cjg->font)
+				break;
 
-		runs = g_slist_prepend(runs, run);
+			for (k = 0; k < cjg->num_glyphs; k++)
+				new_cg->glyphs[num_glyphs + k] = cjg->glyphs[k];
+			num_glyphs += cjg->num_glyphs;
+		}
+		i = j;
+
+		runs = g_slist_prepend(runs, new_cg);
 	}
 
-	g_list_free(run_list);
+	return g_slist_reverse(runs);
+}
+ 
+static int
+_pango_glyph_string_to_pixels(const struct PangoCairoGlyphs *glyphs)
+{
+	int width = 0;
+	int i;
 
-	pango_attr_iterator_destroy(iter);
-	pango_attr_list_unref(attrs);
+	for (i = 0; i < glyphs->num_glyphs; i++)
+		width += glyphs->glyphs[i].geometry.width;
 
-	return g_slist_reverse(runs);
+	return width/PANGO_SCALE;
+}
+
+#define STACK_BUFFER_SIZE (512 * sizeof (int))
+#define STACK_ARRAY_LENGTH(T) (STACK_BUFFER_SIZE / sizeof(T))
+
+static void
+_pango_cairo_renderer_draw_unknown_glyph (cairo_t *cr _U_,
+					  PangoFont          *font _U_,
+					  PangoGlyphInfo     *gi _U_,
+					  double              cx _U_,
+					  double              cy _U_)
+{
+	g_warning("_pango_cairo_renderer_draw_unknown_glyph()\n");
 }
 
 static int
-_pango_glyph_string_to_pixels(PangoGlyphString *glyphs, PangoFont *font _U_)
+_pango_cairo_renderer_draw_glyphs(cairo_t *cr, PangoGlyphInfo *glyphs, gint num_glyphs, double base_x, double base_y)
 {
-#if PANGO_VERSION_MAJOR == 1 && PANGO_VERSION_MINOR >= 14
-	return pango_glyph_string_get_width(glyphs) / PANGO_SCALE;
-#else
-	PangoRectangle logical_rect;
+	cairo_glyph_t stack_glyphs[STACK_ARRAY_LENGTH(cairo_glyph_t)];
+	cairo_glyph_t *cairo_glyphs = stack_glyphs;
+
+	int x_position = 0;
+	int i, count;
+
+	cairo_save(cr);
+
+	if (num_glyphs > (int) G_N_ELEMENTS (stack_glyphs))
+		cairo_glyphs = g_new(cairo_glyph_t, num_glyphs);
+
+	count = 0;
+	for (i = 0; i < num_glyphs; i++) {
+		PangoGlyphInfo *gi = &glyphs[i];
+
+		if (gi->glyph != PANGO_GLYPH_EMPTY) {
+			double cx = base_x + (double)(x_position + gi->geometry.x_offset) / PANGO_SCALE;
+			double cy = gi->geometry.y_offset == 0 ?
+				base_y :
+				base_y + (double)(gi->geometry.y_offset) / PANGO_SCALE;
+
+			if ((gi->glyph & PANGO_GLYPH_UNKNOWN_FLAG) == 0) {
+				cairo_glyphs[count].index = gi->glyph;
+				cairo_glyphs[count].x = cx;
+				cairo_glyphs[count].y = cy;
+				count++;
+			} else
+				_pango_cairo_renderer_draw_unknown_glyph(cr, NULL, gi, cx, cy);
+		}
+		x_position += gi->geometry.width;
+	}
+	cairo_show_glyphs(cr, cairo_glyphs, count);
 
-	pango_glyph_string_extents(glyphs, font, NULL, &logical_rect);
- 	/*  pango_extents_to_pixels(&logical_rect, NULL); */
+	if (cairo_glyphs != stack_glyphs)
+		g_free(cairo_glyphs);
 
-	return (logical_rect.width / PANGO_SCALE);
-#endif
+	cairo_restore(cr);
+	return x_position / PANGO_SCALE;
 }
+ 
 
 static int
 xtext_draw_layout_line(cairo_t *cr, gint x, gint y, GSList *runs)
 {
 	while (runs) {
-		PangoLayoutRun *run = (PangoLayoutRun *)runs->data;
+		struct PangoCairoGlyphs *cg = runs->data;
 
-		cairo_move_to(cr, x, y);
-		pango_cairo_show_glyph_string(cr, run->item->analysis.font, run->glyphs);
-
-		x += _pango_glyph_string_to_pixels(run->glyphs, run->item->analysis.font);
+		cairo_set_scaled_font(cr, cg->scfont);
+		x += _pango_cairo_renderer_draw_glyphs(cr, cg->glyphs, cg->num_glyphs, x, y);
 		runs = runs->next;
 	}
 	return x;
@@ -423,9 +565,9 @@ _pango_runs_width(GSList *runs)
 	int width = 0;
 
 	while (runs) {
-		PangoLayoutRun *run = (PangoLayoutRun *)runs->data;
+		const struct PangoCairoGlyphs *cg = runs->data;
 
-		width += _pango_glyph_string_to_pixels(run->glyphs, run->item->analysis.font);
+		width += _pango_glyph_string_to_pixels(cg);
 		runs = runs->next;
 	}
 	return width;
@@ -437,11 +579,11 @@ _pango_runs_free(GSList *runs)
 	GSList *list = runs;
 
 	while (list) {
-		PangoLayoutRun *run = (PangoLayoutRun *)list->data;
-
-		pango_item_free(run->item);
-		pango_glyph_string_free(run->glyphs);
-		g_slice_free(PangoLayoutRun, run);
+		struct PangoCairoGlyphs *cg = list->data;
+ 
+		g_object_unref(cg->font);
+		g_free(cg->glyphs);
+		g_slice_free(struct PangoCairoGlyphs, cg);
 
 		list = list->next;
 	}
@@ -502,61 +644,31 @@ bytes_view_flush_render(BytesView *bv, void *data, int x, int y, const char *str
 }
 
 static int
-_pango_runs_find_index(GSList *runs, int x_pos, const char *str)
-{
-	int start_pos = 0;
-
-	while (runs) {
-		PangoLayoutRun *run = (PangoLayoutRun *)runs->data;
-		int width;
-
-		width = _pango_glyph_string_to_pixels(run->glyphs, run->item->analysis.font);
-
-		if (x_pos >= start_pos && x_pos < start_pos + width) {
-			gboolean char_trailing;
-			int pos;
-
-			pango_glyph_string_x_to_index(run->glyphs,
-					(char *) str + run->item->offset, run->item->length,
-					&run->item->analysis,
-					(x_pos - start_pos) * PANGO_SCALE,
-					&pos, &char_trailing);
-
-			return run->item->offset + pos;
-		}
-
-		start_pos += width;
-		runs = runs->next;
-	}
-	return -1;
-}
-
-static int
 bytes_view_flush_pos(BytesView *bv, void *data, int x, int search_x, const char *str, int len)
 {
-	int *pos_x = (int *)data;
-	GSList *line_runs;
+	int *pos_x = data;
 	int line_width;
+	int i;
 
 	if (len < 1)
 		return 0;
 
-	line_runs = _pango_runs_build(bv, str, len);
-
-	line_width = _pango_runs_width(line_runs);
+	line_width = 0;
 
-	if (x <= search_x && x + line_width > search_x) {
-		int off_x = search_x - x;
-		int pos_run;
+	for (i = 0; i < len; i++) {
+		const struct PangoCairoGlyphs *cg = &bv->glyphs[(int) str[i]];
+		gboolean before_check, after_check;
 
-		if ((pos_run = _pango_runs_find_index(line_runs, off_x, str)) != -1)
-			*pos_x = (-*pos_x) + pos_run;
+		before_check = (x + line_width <= search_x);
+		line_width += _pango_glyph_string_to_pixels(cg);
+		after_check = (x + line_width <= search_x);
 
-		return -1;	/* terminate */
-	} else
-		*pos_x -= len;
-
-	_pango_runs_free(line_runs);
+		if (before_check && !after_check) {
+			*pos_x = (-*pos_x) + i;
+			return -1;
+		}
+	}
+	*pos_x -= len;
 
 	return line_width;
 }
@@ -790,7 +902,7 @@ bytes_view_render(BytesView *bv, cairo_t *cr, GdkRectangle *area)
 	if (!gtk_widget_get_realized(GTK_WIDGET(bv)))
 		return;
 
-	bytes_view_ensure_layout(bv);
+	bytes_view_ensure_font(bv);
 
 #if GTK_CHECK_VERSION(3, 0, 0)
 	width = gtk_widget_get_allocated_width(GTK_WIDGET(bv));
@@ -944,9 +1056,9 @@ bytes_view_adjustment_set(BytesView *bv)
 	if (bv->vadj == NULL || bv->hadj == NULL)
 		return;
 
-	if (bv->context == NULL) {
-		bytes_view_ensure_layout(bv);
-		/* bytes_view_ensure_layout will call bytes_view_adjustment_set() again */
+	if (bv->font_changed) {
+		bytes_view_ensure_font(bv);
+		/* bytes_view_ensure_font will call bytes_view_adjustment_set() again */
 		return;
 	}
 
@@ -1303,7 +1415,7 @@ bytes_view_byte_from_xy(BytesView *bv, int x, int y)
 	if (x < MARGIN)
 		return -1;
 
-	bytes_view_ensure_layout(bv);
+	bytes_view_ensure_font(bv);
 
 	char_y = (int) gtk_adjustment_get_value(bytes_view_ensure_vadj(bv)) + (y / bv->fontsize);
 	off_y = char_y * bv->per_line;
@@ -1366,11 +1478,8 @@ bytes_view_set_font(BytesView *bv, PangoFontDescription *font)
 	bv->font = pango_font_description_copy(font);
 	bv->max_width = 0;
 
-	if (bv->context) {
-		g_object_unref(bv->context);
-		bv->context = NULL;
-		bytes_view_ensure_layout(bv);
-	}
+	bv->font_changed = TRUE;
+	bytes_view_ensure_font(bv);
 }
 
 void