[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: x-display-pixel-width/height inconsistency

From: Eli Zaretskii
Subject: Re: x-display-pixel-width/height inconsistency
Date: Thu, 09 May 2013 23:03:15 +0300

> Date: Thu, 09 May 2013 09:09:33 +0900
> From: YAMAMOTO Mitsuharu <address@hidden>
> Cc: address@hidden
> Thanks for testing.  I updated the w32fns.c part below.

I have a couple more comments.

> + static BOOL CALLBACK
> + w32_monitor_enum (HMONITOR monitor, HDC hdc, RECT *rcMonitor, LPARAM dwData)
> + {
> +   Lisp_Object *monitor_list = (Lisp_Object *) dwData;
> + 
> +   *monitor_list = Fcons (make_save_pointer (monitor), *monitor_list);
> + 
> +   return TRUE;
> + }

It makes me nervous to have consing inside a Win32 API callback.  Are
we even sure the callback gets called in the context of our main
tread?  If not, and if consing causes GC, we could blow up the stack.

And actually, I don't see why you need the list of monitor HMONITOR
handles, you get them from MonitorFromWindow anyway.  All you need is
the count of the monitors, and then you can create the vector in
monitor_frames by simply inserting every monitor you get from
MonitorFromWindow, and leave the rest at nil.  Or am I missing

So could we please get rid of the consing inside the callback, and
just count them instead?

> +       if (FRAME_W32_P (f) && FRAME_W32_DISPLAY_INFO (f) == dpyinfo
> +       && !EQ (frame, tip_frame))

The test against dpyinfo is redundant, it will always be true: there's
only one dpyinfo on Windows.  I would like to remove it, because it's
confusing.  (There's one more such test in another place in the

> +       name = make_unibyte_string (mi.szDevice, strlen (mi.szDevice));

Why make_unibyte_string?  Is szDevice documented to be an ASCII-only

> +   attributes = Fcons (Fcons (Qname, build_string ("combined screen")),
> +                   attributes);

And using build_string here with a pure ASCII string is at least
inconsistent.  (But I actually suggest to use build_string in both


reply via email to

[Prev in Thread] Current Thread [Next in Thread]