From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: storm@cua.dk (Kim F. Storm) Newsgroups: gmane.emacs.devel Subject: Re: Status of MAC/W32/X consolidation -- first major patch committed. Date: 17 Mar 2003 12:40:04 +0100 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: <5xsmtmp6gb.fsf@kfs2.cua.dk> References: <5xadg38lnj.fsf@kfs2.cua.dk> <5xheaaq93k.fsf@kfs2.cua.dk> <5xbs0bq8en.fsf_-_@kfs2.cua.dk> <5x3clmrhgj.fsf@kfs2.cua.dk> NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: main.gmane.org 1047897702 8685 80.91.224.249 (17 Mar 2003 10:41:42 GMT) X-Complaints-To: usenet@main.gmane.org NNTP-Posting-Date: Mon, 17 Mar 2003 10:41:42 +0000 (UTC) Cc: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Mon Mar 17 11:41:40 2003 Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by main.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 18us3o-0002Fd-00 for ; Mon, 17 Mar 2003 11:41:40 +0100 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 18usTR-0005IU-00 for ; Mon, 17 Mar 2003 12:08:09 +0100 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.10.13) id 18us4C-0003rv-01 for emacs-devel@quimby.gnus.org; Mon, 17 Mar 2003 05:42:04 -0500 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.10.13) id 18us3o-0003Zf-00 for emacs-devel@gnu.org; Mon, 17 Mar 2003 05:41:40 -0500 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.10.13) id 18us3a-00032w-00 for emacs-devel@gnu.org; Mon, 17 Mar 2003 05:41:27 -0500 Original-Received: from mail.filanet.dk ([195.215.206.179]) by monty-python.gnu.org with esmtp (Exim 4.10.13) id 18us3M-0002eq-00; Mon, 17 Mar 2003 05:41:12 -0500 Original-Received: from kfs2.cua.dk.cua.dk (kfs2.local.filanet.dk [192.168.1.182]) by mail.filanet.dk (Postfix) with SMTP id DC7F67C012; Mon, 17 Mar 2003 11:41:10 +0100 (CET) Original-To: Jason Rumney In-Reply-To: Original-Lines: 129 User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3.50 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1b5 Precedence: list List-Id: Emacs development discussions. List-Help: List-Post: List-Subscribe: , List-Archive: List-Unsubscribe: , Errors-To: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Xref: main.gmane.org gmane.emacs.devel:12402 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:12402 Jason Rumney writes: > storm@cua.dk (Kim F. Storm) writes: > > > I swapped the names of "w32_get_glyph_overhangs" and > > "x_get_glyph_overhangs" to make the X and W32 code less different... > > That could be dangerous. I agree in general, but not in this case... read on. > Usually if I changed the x_ prefix to w32_, > I did so because the interface or effect of the function was > different. In this case, the interface was as follows. > > static void w32_get_glyph_overhangs P_ ((HDC hdc, struct glyph *, > struct frame *, > int *, int *)); Yes, I understand that. However, in this specific case, I really didn't see any reason for a w32-specific version, as the change to the interface (adding `hdc' arg) seemed unnecessary -- as the function didn't reference `hdc' at all. Here is the original version: static void w32_get_glyph_overhangs (hdc, glyph, f, left, right) HDC hdc; struct glyph *glyph; struct frame *f; int *left, *right; { *left = *right = 0; if (glyph->type == CHAR_GLYPH) { XFontStruct *font; struct face *face; wchar_t char2b; XCharStruct *pcm; face = x_get_glyph_face_and_encoding (f, glyph, &char2b, NULL); font = face->font; if (font && (pcm = w32_per_char_metric (font, &char2b, glyph->w32_font_type))) { if (pcm->rbearing > pcm->width) *right = pcm->rbearing - pcm->width; if (pcm->lbearing < 0) *left = -pcm->lbearing; } } } > > x_get_glyph_overhangs was probably a wrapper to present the same > interface as the x equivalent, but less efficient due to needing to > get the hdc on every call (it is called for every glyph displayed, so > that makes a big difference). > Again, since the original w32_get_glyph_overhangs didn't actually reference the hdc argument, but the original x_get_glyph_overhangs (and the new w32_get_glyph_overhangs) still did the extra work of getting the hdc, I thought that it might be required to do so due to side-effects of the get_frame_dc. So I believe my changes are safe! We may further simplify the code by getting rid of the new w32_get_glyph_overhangs (if the assumption about required side-effects is false). Then just discard w32_get_glyph_overhangs and use x_get_glyph_overhangs directly in its place. BTW, this leads me to another strange thing about the merged version of this code (based on the code from xterm.c): void x_get_glyph_overhangs (glyph, f, left, right) struct glyph *glyph; struct frame *f; int *left, *right; { *left = *right = 0; if (glyph->type == CHAR_GLYPH) { XFontStruct *font; struct face *face; struct font_info *font_info; XChar2b char2b; XCharStruct *pcm; face = get_glyph_face_and_encoding (f, glyph, &char2b, NULL); font = face->font; font_info = FONT_INFO_FROM_ID (f, face->font_info_id); if (font /* ++KFS: Should this be font_info ? */ && (pcm = rif->per_char_metric (font, &char2b, glyph->font_type))) { if (pcm->rbearing > pcm->width) *right = pcm->rbearing - pcm->width; if (pcm->lbearing < 0) *left = -pcm->lbearing; } } } Notice that the code finds the `font_info' for the font, but doesn't actually use that for anything. I have a suspicion that the test that reads if (font && ...) was really meant to read if (font_info && ...) I see three indications why this may be true: 1) Why find the font_info (and not using it) ? 2) Is font (== face->font) ever NULL ? (sounds strange to me) 3) Other similar parts of the code which call per_char_metric actually check the font_info rather than the font. If anyone can sched some light on this, it would be appreciated! -- Kim F. Storm http://www.cua.dk