Status of MAC/W32/X consolidation and some questions.

Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

[There are a couple of MAC and W32 specific questions in this mail.
Those who know these ports well, please take a look at them.]


As you may have seen on emacs-devel, I'm in the process of
consolidating the various ports of CVS emacs for X, W32, and MAC.

Most of this work is to identify (and merge) code which basically only
differs in "names" rather than "structure", e.g. by using FRAME_X_P in
the X version, and FRAME_MAC_P in the MAC version, but otherwise is
identical.

AFAICS, the best option for maximum code sharing is to extend the
current approach in the MAC and W32 ports of using part of the X port
name space, e.g. by defining a partial x_output structure to satisfy
the needs of the basic code which expects an x_output structure to
exist, or by defining some of the FRAME_X_* macros to point to the
output_data.mac structure.

Now, what I'm currently working on is to replace all mac* and w32*
specific names with the corresponding x* names, e.g. FRAME_MAC_WINDOW
is replaced by FRAME_X_WINDOW, mac_output is replaced by x_output,
f->output_data.mac becomes f->output_data.x, etc. etc.

As I progress in this regard, the amount of code which can then be
shared directly between the ports gets larger and larger.

One major drawback of this approach is that while the current
"overloading of the X* specific names makes it difficult to make a
version which supports both MAC and X simultaneously (i.e. in the same
executable), extending this "overloading" to cover all the MAC and W32
specific code makes it practically impossible to support such
cross-GUI systems that later on.

However, I got the message from RMS, that this limitation was
acceptable (and I agree), so I'm doing just that, as it gives the
maximum benefits with the least amount of work (still it *is* a lot of
tedious work :-)

However, one of the "merged" macros is giving me problems: FRAME_MAC_P
and FRAME_W32_P cannot simply be merged into FRAME_X_P (for various
reasons), but I'll find a way :-)


On W32, FRAME_W32_P seems to be true even when HAVE_NTGUI is not
defined (at lease Fframe_parameter in frame.c seems to assume this),
so while FRAME_X_P and FRAME_MAC_P means that the output is to a
GUI, FRAME_W32_P only means that output is to a w32 system as such.

So my questions to the W32 experts are: 

* Does it make sense to build NTemacs without HAVE_NTGUI defined?
  Is it possible to do that?

* Does the W32 version support -nw?
  If so, is FRAME_W32_P still true when specified?


On MAC, I noticed two places in macterm.c where FRAME_X_P is tested
rather than FRAME_MAC_P, which basically means that the corresponding
code _IS_NOT_ executed:

 * In x_flush, the call to XFlush is conditioned by FRAME_X_P

 * In XTframe_up_to_date, the entire code is conditioned by FRAME_X_P.

Could a MAC expect please check these two strange-looking cases for
me, thanks!

Also on MAC, there are quite a number of TODO items, meaning that some
section of more-or-less unmodified X or W32 code is commented out with
#if 0...  #endif.  In the cases where the code is to be "consolidated",
I would prefer to leave a TODO note in the mac* file and simply delete
the code from the file.  Then in the consolidated code file, I will
put a #ifndef MAC_OS around the consolidated code to indicate that it
is not used on MAC_OS (yet).   Is that an acceptable approach?



Since I'm working on this right now, I would advise everyone to commit
any major changes that you may have in your local checkout, as
the structure of the code is going to change quite a lot due to
my work (the consolidated code is going to be moved out of the
mac* specific files).

Also, since I cannot really test the changes on MAC and W32, I do
expect that I'm going to make a few mistakes in the consolidation,
resulting in non-functional MAC and W32 versions on the CVS head,
which I hope some of you will assist me in solving (I hope/expect it
is just a matter of fixing some trivial compilation errors).


-- 
Kim F. Storm  http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> [...]
> 
> On MAC, I noticed two places in macterm.c where FRAME_X_P is tested
> rather than FRAME_MAC_P, which basically means that the corresponding
> code _IS_NOT_ executed:
> 
>  * In x_flush, the call to XFlush is conditioned by FRAME_X_P
> 
>  * In XTframe_up_to_date, the entire code is conditioned by FRAME_X_P.
> 
> Could a MAC expect please check these two strange-looking cases for
> me, thanks!

These are probably just cases I copied code from w32term.c or xterm.c
and then forgot to change them.  They will work fine if changed to
FRAME_MAC_P.

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Andrew Choi <akochoi@shaw.ca> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > [...]
> > 
> > On MAC, I noticed two places in macterm.c where FRAME_X_P is tested
> > rather than FRAME_MAC_P, which basically means that the corresponding
> > code _IS_NOT_ executed:
> > 
> >  * In x_flush, the call to XFlush is conditioned by FRAME_X_P
> > 
> >  * In XTframe_up_to_date, the entire code is conditioned by FRAME_X_P.
> > 
> > Could a MAC expect please check these two strange-looking cases for
> > me, thanks!
> 
> These are probably just cases I copied code from w32term.c or xterm.c
> and then forgot to change them.  They will work fine if changed to
> FRAME_MAC_P.

Ok.  I'll fix those.

I just wonder: Does it also run fine without changing them
(i.e. without executing the conditioned code?)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> Andrew Choi <akochoi@shaw.ca> writes:
> 
> > storm@cua.dk (Kim F. Storm) writes:
> > 
> > > [...]
> > > 
> > > On MAC, I noticed two places in macterm.c where FRAME_X_P is tested
> > > rather than FRAME_MAC_P, which basically means that the corresponding
> > > code _IS_NOT_ executed:
> > > 
> > >  * In x_flush, the call to XFlush is conditioned by FRAME_X_P
> > > 
> > >  * In XTframe_up_to_date, the entire code is conditioned by FRAME_X_P.
> > > 
> > > Could a MAC expect please check these two strange-looking cases for
> > > me, thanks!
> > 
> > These are probably just cases I copied code from w32term.c or xterm.c
> > and then forgot to change them.  They will work fine if changed to
> > FRAME_MAC_P.
> 
> Ok.  I'll fix those.
> 
> I just wonder: Does it also run fine without changing them
> (i.e. without executing the conditioned code?)

Yes, it does.

Re: Status of MAC/W32/X consolidation and some questions.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> [...]
> 
> Also on MAC, there are quite a number of TODO items, meaning that some
> section of more-or-less unmodified X or W32 code is commented out with
> #if 0...  #endif.  In the cases where the code is to be "consolidated",
> I would prefer to leave a TODO note in the mac* file and simply delete
> the code from the file.  Then in the consolidated code file, I will
> put a #ifndef MAC_OS around the consolidated code to indicate that it
> is not used on MAC_OS (yet).   Is that an acceptable approach?

Please consider using HAVE_CARBON instead of MAC_OS: some people compile
Emacs to run under X Window on Mac OS X.

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Andrew Choi <akochoi@shaw.ca> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > [...]
> > 
> > Also on MAC, there are quite a number of TODO items, meaning that some
> > section of more-or-less unmodified X or W32 code is commented out with
> > #if 0...  #endif.  In the cases where the code is to be "consolidated",
> > I would prefer to leave a TODO note in the mac* file and simply delete
> > the code from the file.  Then in the consolidated code file, I will
> > put a #ifndef MAC_OS around the consolidated code to indicate that it
> > is not used on MAC_OS (yet).   Is that an acceptable approach?
> 
> Please consider using HAVE_CARBON instead of MAC_OS: some people compile
> Emacs to run under X Window on Mac OS X.

I don't understand the difference between those names -- are you saying
that we should in general use HAVE_CARBON rather than MAC_OS throughout?

For the windows port, there are two defines:  WINDOWSNT and HAVE_NTGUI.
The first specifies that we are on a W32 platform, the other that we
are compiling for the W32 GUI.

Are MAC_OS and HAVE_CARBON the Mac equivalent of those?


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> Andrew Choi <akochoi@shaw.ca> writes:
> 
> > storm@cua.dk (Kim F. Storm) writes:
> > 
> > > [...]
> > > 
> > > Also on MAC, there are quite a number of TODO items, meaning that some
> > > section of more-or-less unmodified X or W32 code is commented out with
> > > #if 0...  #endif.  In the cases where the code is to be "consolidated",
> > > I would prefer to leave a TODO note in the mac* file and simply delete
> > > the code from the file.  Then in the consolidated code file, I will
> > > put a #ifndef MAC_OS around the consolidated code to indicate that it
> > > is not used on MAC_OS (yet).   Is that an acceptable approach?
> > 
> > Please consider using HAVE_CARBON instead of MAC_OS: some people compile
> > Emacs to run under X Window on Mac OS X.
> 
> I don't understand the difference between those names -- are you saying
> that we should in general use HAVE_CARBON rather than MAC_OS throughout?

Yes.

> For the windows port, there are two defines:  WINDOWSNT and HAVE_NTGUI.
> The first specifies that we are on a W32 platform, the other that we
> are compiling for the W32 GUI.
> 
> Are MAC_OS and HAVE_CARBON the Mac equivalent of those?

Yes, the two macros name the OS and GUI, respectively.  Some people may
be compiling Emacs with HAVE_CARBON and yet others may be compiling with
HAVE_X11 on the Mac OS X.

Re: Status of MAC/W32/X consolidation and some questions.

[Benjamin Riefenstahl]

Hi Kim,


storm@cua.dk (Kim F. Storm) writes:
> [There are a couple of MAC and W32 specific questions in this mail.
> Those who know these ports well, please take a look at them.]

I am not by any means an expert on the code, but I have some thoughts.

> Now, what I'm currently working on is to replace all mac* and w32*
> specific names with the corresponding x* names,
> e.g. FRAME_MAC_WINDOW is replaced by FRAME_X_WINDOW,

I guess this is probably the fastest path and would mostly leave alone
the X11 code.  But wouldn't it in theory better to rename those items
to a neutral FRAME_GUI_ANYTHING instead?  When I look at code I find
the tendency to re-use X11 names outside of X11 rather irritating.

> * Does the W32 version support -nw?

Yes. 

>   If so, is FRAME_W32_P still true when specified?

No.  If I read the code right, FRAME_W32_P is based on output_method,
and output_method can be interrogated as the result of the (framep)
function.  "(framep (selected-frame))" returns t (for "character-only
terminal") in this situation.

> Also, since I cannot really test the changes on MAC and W32, I do
> expect that I'm going to make a few mistakes in the consolidation,
> resulting in non-functional MAC and W32 versions on the CVS head,
> which I hope some of you will assist me in solving (I hope/expect it
> is just a matter of fixing some trivial compilation errors).

I can probably help with that. 


so long, benny

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> writes:

> > Now, what I'm currently working on is to replace all mac* and w32*
> > specific names with the corresponding x* names,
> > e.g. FRAME_MAC_WINDOW is replaced by FRAME_X_WINDOW,
> 
> I guess this is probably the fastest path and would mostly leave alone
> the X11 code.  But wouldn't it in theory better to rename those items
> to a neutral FRAME_GUI_ANYTHING instead?  When I look at code I find
> the tendency to re-use X11 names outside of X11 rather irritating.

I have been thinking about the same issue, and I am not very
satisfied with the overloading of the X names, either.

However, as the X version is really the "reference GUI platform", a
lot of the code which really isn't X-specific still use the x_ prefix
(or some other X-related name), so from that point of view, it does
make sense that the "reference gui" continues to use the X names, and
let the other (non-free) platforms overload the X-specific "reference
platform" functions.

> 
> > * Does the W32 version support -nw?
> 
> Yes. 
> 
> >   If so, is FRAME_W32_P still true when specified?
> 
> No.  If I read the code right, FRAME_W32_P is based on output_method,
> and output_method can be interrogated as the result of the (framep)
> function.  "(framep (selected-frame))" returns t (for "character-only
> terminal") in this situation.

Ok, thanks.  So output_method is output_termcap for -nw on windows.
Then some of the emacs code really make no sense to me.

What does 
        (assoc 'font (frame-parameters nil))
return when you try it on W32 emacs -nw ?

If your assumption is correct is will return (font . "tty"),
but the code seems to be supposed to return (font . "w32term").

Also, in xdisp.c, the following test is used to check whether
emacs is running as a GUI or TTY program on W32, X, or MAC:

  /* Don't do all this for graphical frames.  */
#ifdef HAVE_NTGUI
  if (!NILP (Vwindow_system))
    return;
#endif
#if defined (USE_X_TOOLKIT) || defined (USE_GTK)
  if (FRAME_X_P (f))
    return;
#endif
#ifdef MAC_OS
  if (FRAME_MAC_P (f))
    return;
#endif

If your assumption is correct, the first test could be
replaced by

  if (FRAME_W32_P (f))

    
> > which I hope some of you will assist me in solving (I hope/expect it
> > is just a matter of fixing some trivial compilation errors).
> 
> I can probably help with that. 

Thanks.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Juanma Barranquero]

On 11 Mar 2003 02:01:01 +0100, storm@cua.dk (Kim F. Storm) wrote:

> What does 
>         (assoc 'font (frame-parameters nil))
> return when you try it on W32 emacs -nw ?
> 
> If your assumption is correct is will return (font . "tty"),

Yes.


                                                           /L/e/k/t/u

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> On 11 Mar 2003 02:01:01 +0100, storm@cua.dk (Kim F. Storm) wrote:
> 
> > What does 
> >         (assoc 'font (frame-parameters nil))
> > return when you try it on W32 emacs -nw ?
> > 
> > If your assumption is correct is will return (font . "tty"),
> 
> Yes.
> 
Thank you.  That's proof enough for me :-)

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Jason Rumney]

storm@cua.dk (Kim F. Storm) writes:

> Ok, thanks.  So output_method is output_termcap for -nw on windows.
> Then some of the emacs code really make no sense to me.

I think the code that doesn't seem to make sense is there for
historical reasons and could be cleaned up.  IIRC, it used to be
necessary to choose GUI or console based output on W32 at compile
time, but Andrew Innes or Geoff Voelker fixed it so a single
executable could support both (and the -nw option) around 19.27 or so.
I guess some of the old code is still hanging around.

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > Ok, thanks.  So output_method is output_termcap for -nw on windows.
> > Then some of the emacs code really make no sense to me.
> 
> I think the code that doesn't seem to make sense is there for
> historical reasons and could be cleaned up.  IIRC, it used to be
> necessary to choose GUI or console based output on W32 at compile
> time, but Andrew Innes or Geoff Voelker fixed it so a single
> executable could support both (and the -nw option) around 19.27 or so.
> I guess some of the old code is still hanging around.
> 

Ok, I'll bear that in mind and try to clean some of it up along the way.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Miles Bader]

Jason Rumney <jasonr@gnu.org> writes:
> I think the code that doesn't seem to make sense is there for
> historical reasons and could be cleaned up.

Hmmm, that's a good description of at least 50% of the emacs source
code, I think!  :-) ... :-(

-Miles
-- 
P.S.  All information contained in the above letter is false,
      for reasons of military security.

Re: Status of MAC/W32/X consolidation and some questions.

[Jason Rumney]

storm@cua.dk (Kim F. Storm) writes:

> On W32, FRAME_W32_P seems to be true even when HAVE_NTGUI is not
> defined

It is only true when the frame's output is to a W32 GUI frame,
according to the definition in frame.h.

> (at lease Fframe_parameter in frame.c seems to assume this),

This seems to be a bug. But running emacs under the console in w32
does work, so it cannot be a serious as it looks.

> * Does it make sense to build NTemacs without HAVE_NTGUI defined?
>   Is it possible to do that?

Yes, it is possible, but probably not common.

> * Does the W32 version support -nw?

Yes.

>   If so, is FRAME_W32_P still true when specified?

No.

Re: Status of MAC/W32/X consolidation and some questions.

[Miles Bader]

storm@cua.dk (Kim F. Storm) writes:
> Since I'm working on this right now, I would advise everyone to commit
> any major changes that you may have in your local checkout, as
> the structure of the code is going to change quite a lot due to
> my work (the consolidated code is going to be moved out of the
> mac* specific files).

Ah well, I guess I'll have to toss out my face pixmaps patch (allow
images as face background) since there's no way it's going to survive
something like this, and there's no way I can commit it...

-Miles
-- 
"Whatever you do will be insignificant, but it is very important that
 you do it."  Mahatma Ghandi

Re: Status of MAC/W32/X consolidation and some questions.

[Stefan Monnier]

> Ah well, I guess I'll have to toss out my face pixmaps patch (allow
> images as face background) since there's no way it's going to survive
> something like this, and there's no way I can commit it...

How 'bout committing it to a branch at least, for posterity ?


	Stefan

Re: Status of MAC/W32/X consolidation and some questions.

[Miles Bader]

"Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> writes:
> > Ah well, I guess I'll have to toss out my face pixmaps patch (allow
> > images as face background) since there's no way it's going to survive
> > something like this, and there's no way I can commit it...
> 
> How 'bout committing it to a branch at least, for posterity ?

I think even that would require a fair bit of work (I haven't bothered
to keep it updated), but the main annoyance for me is that there are
_two_ versions, one of which works fairly well, but is lacking features,
and one which adds the features I want (which are necessary, I think, to
satisfy the `transparent background' crowd), but doesn't work (the two
patches even use different lisp-visible interfaces, which adds to the
fun).

All in all it's rather a mess; I should have maintained it as a branch
all along (which may have eased the pain of CVS enough so that I would
have actually finished it!), but I didn't really appreciate branches
until you clued me in (with the lex-binding stuff).

-Miles
-- 
自らを空にして、心を開く時、道は開かれる

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

Miles Bader <miles@lsi.nec.co.jp> writes:

> storm@cua.dk (Kim F. Storm) writes:
> > Since I'm working on this right now, I would advise everyone to commit
> > any major changes that you may have in your local checkout, as
> > the structure of the code is going to change quite a lot due to
> > my work (the consolidated code is going to be moved out of the
> > mac* specific files).
> 
> Ah well, I guess I'll have to toss out my face pixmaps patch (allow
> images as face background) since there's no way it's going to survive
> something like this, and there's no way I can commit it...


>From recent messages on this list, it seems that your changes survived
the big consolidation last year after all...  ??

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Miles Bader]

On Thu, Jul 22, 2004 at 05:11:42PM +0200, Kim F. Storm wrote:
> > Ah well, I guess I'll have to toss out my face pixmaps patch (allow
> > images as face background) since there's no way it's going to survive
> > something like this, and there's no way I can commit it...
> 
> From recent messages on this list, it seems that your changes survived
> the big consolidation last year after all...  ??

Yes, very few problems actually.

When you moved a lot of image functions to image.c, of course my patches to
those functions (in the original files) failed to apply, but then I just
applied the .rej file to image.c, and almost all of it applied fine... :-)

-Miles
-- 
Freedom's just another word, for nothing left to lose   --Janis Joplin

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> As you may have seen on emacs-devel, I'm in the process of
> consolidating the various ports of CVS emacs for X, W32, and MAC.

.. which turns out to be a bigger task than I first imagined.  Sigh!

> 
> Most of this work is to identify (and merge) code which basically only
> differs in "names" rather than "structure", e.g. by using FRAME_X_P in
> the X version, and FRAME_MAC_P in the MAC version, but otherwise is
> identical.
> 
> AFAICS, the best option for maximum code sharing is to extend the
> current approach in the MAC and W32 ports of using part of the X port
> name space, e.g. by defining a partial x_output structure to satisfy
> the needs of the basic code which expects an x_output structure to
> exist, or by defining some of the FRAME_X_* macros to point to the
> output_data.mac structure.

I'm getting less enthustiastic about this approach as a general way
to achieve the goal of reducing duplicate code (but it will still
be useful in many places).  

Although this method does allow sharing of more code (taken from the
xfns.c and xterm.c files), it makes a pretty arbitrary split between
the parts of xfns.c and xterm.c which are "shareable" and those parts
which are not.

Consequently, if I take out the parts of xfns.c and xterm.c which are
shareable and place them in, say, genfns.c and genterm.c, it really
makes a hotch-potch output of those four files, probably making the X
version harder to maintain -- simply because the code is broken apart
into what may seem pretty arbitrarty files (that's already the case
for some of the code, but it's probably getting even worse with this
approach).


A different approach - which I think I like better - is to try to
generalize some things rather than overloading names, e.g.

* move common parts of x_output, w32_output, and mac_output directly
  into the frame structure.  Then code doesn't have to go through the
  correct output_data macro, but can get to the data directly from the
  frame pointer.  This adds some overhead to the frame structure for
  non-windowing frames, but it's a VERY SMALL price to pay for
  increased portability.

* generalize code sections by extending the redisplay_interface so
  that common code can be moved out of e.g. xterm.c, but still have
  hooks into xterm.c to do the things that are really X specific.
  A good example is the handling of fringes.

* export more of the window system specific low-level
  functions like clipping and filling through the redisplay_interface.

* maybe generalize the glyph processing along the same lines.


The biggest advantage of this approach is that is can be done in
smaller steps, rather than the planned "change everything in one
big step" approach.  

So probably you should be prepared for a number of smaller, more
focused consolidation updates, rather than a single BIG one...


Stay tuned :-)
 
-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation and some questions.

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: 11 Mar 2003 15:19:27 +0100
> 
> * move common parts of x_output, w32_output, and mac_output directly
>   into the frame structure.  Then code doesn't have to go through the
>   correct output_data macro, but can get to the data directly from the
>   frame pointer.  This adds some overhead to the frame structure for
>   non-windowing frames, but it's a VERY SMALL price to pay for
>   increased portability.

Careful here.  IIUC, you are going to touch tty-specific code as well.
If that's true, please consider also the MSDOS code (which also
fiddles with the x_output and related things).  Ideally, it should
not be broken by whatever you do ;-)

TIA

Re: Status of MAC/W32/X consolidation and some questions.

[Kim F. Storm]

"Eli Zaretskii" <eliz@elta.co.il> writes:

> > From: storm@cua.dk (Kim F. Storm)
> > Date: 11 Mar 2003 15:19:27 +0100
> > 
> > * move common parts of x_output, w32_output, and mac_output directly
> >   into the frame structure.  Then code doesn't have to go through the
> >   correct output_data macro, but can get to the data directly from the
> >   frame pointer.  This adds some overhead to the frame structure for
> >   non-windowing frames, but it's a VERY SMALL price to pay for
> >   increased portability.
> 
> Careful here.  IIUC, you are going to touch tty-specific code as well.
> If that's true, please consider also the MSDOS code (which also
> fiddles with the x_output and related things).  Ideally, it should
> not be broken by whatever you do ;-)

Thanks for the reminder -- but I'm aware that the msdos port has a
"fake" x_output structure, that I need to handle correctly too.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Status of MAC/W32/X consolidation -- first major patch committed.

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> As you may have seen on emacs-devel, I'm in the process of
> consolidating the various ports of CVS emacs for X, W32, and MAC.

I have just committed a fairly big set of changes primarily to
xdisp.c, xterm.c, w32term.c, and macterm.c to consolidate the
gui-independent parts of the "glyph string" processing.

I have only been able to test this on X (under GNU/Linux), so there
may be some problems compiling (and running) the consolidated code on
W32 and MAC.  Please report any problems to me (unless you can fix
them yourself).

More consolidation patches will following in the coming days/weeks.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Jason Rumney]

As part of your changes, you introduced the following function:

  static void
  w32_get_glyph_overhangs (glyph, f, left, right)
       struct glyph *glyph;
       struct frame *f;
       int *left, *right;
  {
    HDC hdc = get_frame_dc (f);
    /* Convert to unicode! */
    x_get_glyph_overhangs (glyph, f, left, right);
    release_frame_dc (f, hdc);
  }

What does the comment "Convert to unicode!" mean?  Is there work
required to properly convert this to Windows? If that is the case, I
don't know when I would get time to look at it, and it will probably
get forgotten. It would be better to revert that part of the change.

As it is, the function is not needed. Since hdc is never used, there
is no point retrieving and releasing it. So we might as well use
x_get_glyph_overhangs directly.

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> As part of your changes, you introduced the following function:
> 
>   static void
>   w32_get_glyph_overhangs (glyph, f, left, right)
>        struct glyph *glyph;
>        struct frame *f;
>        int *left, *right;
>   {
>     HDC hdc = get_frame_dc (f);
>     /* Convert to unicode! */
>     x_get_glyph_overhangs (glyph, f, left, right);
>     release_frame_dc (f, hdc);
>   }
> 
> What does the comment "Convert to unicode!" mean?  Is there work
> required to properly convert this to Windows? If that is the case, I
> don't know when I would get time to look at it, and it will probably
> get forgotten. It would be better to revert that part of the change.

I didn't "invent" that comment... it was there already.

The above function used to be named "x_get_glyph_overhangs" calling
what used to be named "w32_get_glyph_overhangs".

I swapped the names of "w32_get_glyph_overhangs" and
"x_get_glyph_overhangs" to make the X and W32 code less different...


> 
> As it is, the function is not needed. Since hdc is never used, there
> is no point retrieving and releasing it. So we might as well use
> x_get_glyph_overhangs directly.
> 

I thought so myself, but it could seem like get_frame_dc had some
side-effects, so I didn't want to break anything.  If you think it is
safe to discard w32_get_glyph_overhangs, just do it!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Jason Rumney]

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. 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 *));

 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).

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> 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 <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Benjamin Riefenstahl]

Hi Kim,


no-spam@cua.dk (Kim F. Storm) writes:
> I have only been able to test this on X (under GNU/Linux), so there
> may be some problems compiling (and running) the consolidated code on
> W32 and MAC.  Please report any problems to me (unless you can fix
> them yourself).

On MacOSX I had to do this small patch:

<<<<
diff -c -p -r1.811 xdisp.c
*** src/xdisp.c	16 Mar 2003 20:45:46 -0000	1.811
--- src/xdisp.c	17 Mar 2003 14:31:51 -0000
*************** x_produce_glyphs (it)
*** 17475,17481 ****
  	      if (font == NULL)
  		{
  		  font = FRAME_FONT (it->f);
! 		  boff = it->f->output_data.x->baseline_offset;
  		  font_info = NULL;
  		}
  	      else
--- 17475,17481 ----
  	      if (font == NULL)
  		{
  		  font = FRAME_FONT (it->f);
! 		  boff = FRAME_BASELINE_OFFSET(it->f);
  		  font_info = NULL;
  		}
  	      else
>>>>

Note that FRAME_BASELINE_OFFSET() refers to the same variable "int
baseline_offset" in the different system-specific structs and the
variable is always accessed through the macro.  So it may properly
belong into the common data structure struct frame instead anyway.


so long, benny

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Benjamin Riefenstahl]

jasonr@f2s.com writes:
> Can you confirm that the latest CVS works on Mac?

I just recompiled from CVS.  Works fine now, thanks. 

so long, benny

Re: Status of MAC/W32/X consolidation -- first major patch committed.

[Juanma Barranquero]

On 17 Mar 2003 15:56:01 +0100, Benjamin Riefenstahl <Benjamin.Riefenstahl@epost.de> wrote:

> On MacOSX I had to do this small patch:

> ! 		  boff = it->f->output_data.x->baseline_offset;

> ! 		  boff = FRAME_BASELINE_OFFSET(it->f);

It's already fixed on the CVS.

                                                           /L/e/k/t/u

Status of MAC/W32/X consolidation -- second major patch committed.

[Kim F. Storm]

> storm@cua.dk (Kim F. Storm) writes:
> 
> > As you may have seen on emacs-devel, I'm in the process of
> > consolidating the various ports of CVS emacs for X, W32, and MAC.
> 
> More consolidation patches will following in the coming days/weeks.

As "promised", I have just committed another major patch to
consolidate the remaining duplicate code from xterm.c, w32term.c and
macterm.c into xdisp.c.  

So far, a total of 15000 lines of code from those three files have
been merged into just 5000 lines in xdisp.c.  There are still a few
minor thunks of duplicate code left, but IMO it's not worth the
efforts to squeeze those files any further.

My next step is to consolidate the duplicate code from xfns.c,
w32fns.c and macfns.c.

The ChangeLog has this blurb about the second major patch:

	The following changes consolidate code related to writing and
	inserting glyphs, exposing frame, the tool bar, the mouse face,
	the output cursor, and help echo from xterm.c, w32term.c and
	macterm.c into xdisp.c.  It also generalizes the use of the
	window_part enum instead of using numeric values throughout.

Again, I have only been able to test this on X (under GNU/Linux), so
there may be some problems compiling (and running) the consolidated
code on W32 and MAC.  The MSDOS port may also be affected by this
change.  

Please report any problems to me (unless you can fix them yourself).


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- second major patch committed.

[Juanma Barranquero]

On 21 Mar 2003 16:10:11 +0100, storm@cua.dk (Kim F. Storm) wrote:

> Again, I have only been able to test this on X (under GNU/Linux), so
> there may be some problems compiling (and running) the consolidated
> code on W32 and MAC.  The MSDOS port may also be affected by this
> change.  

There are at least two problems with the latest bunch of changes.

In xdisp.c, line 1582, there's an use of internal_border_width which
should be FRAME_INTERNAL_BORDER_WIDTH (or *_SAFE, I don't know which one).

More serious, w32_draw_window_cursor (in w32term.c) references
active_cursor, which is undefined in the file (it exists in xdisp.c).

That said, thanks *a lot* for the effort.


                                                           /L/e/k/t/u

Re: Status of MAC/W32/X consolidation -- second major patch committed.

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> On 21 Mar 2003 16:10:11 +0100, storm@cua.dk (Kim F. Storm) wrote:
> 
> > Again, I have only been able to test this on X (under GNU/Linux), so
> > there may be some problems compiling (and running) the consolidated
> > code on W32 and MAC.  The MSDOS port may also be affected by this
> > change.  
> 
> There are at least two problems with the latest bunch of changes.

Should be fixed now.  Thanks.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- second major patch committed.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> [...] Again, I have only been able to test this on X (under
> GNU/Linux), so there may be some problems compiling (and running) the
> consolidated code on W32 and MAC.  The MSDOS port may also be affected
> by this change.
> 
> Please report any problems to me (unless you can fix them yourself).

There were a few problems building on Mac OS X.  I have just fixed them
and checked them in.

Some Mac-specific code in xdisp.c has been removed and now the common
version is used.  Users of the Mac OS X build please report any problems
caused by this.

Status of MAC/W32/X consolidation -- third major patch committed.

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

I have now consolidated the common code related to frame-parameter
handling from xfns.c, w32fns.c, and macfns.c into frame.c.

Again, I have only been able to test this on X (under GNU/Linux), so
there may be some problems compiling (and running) the consolidated
code on W32 and MAC.  The MSDOS port may also be affected by this
change.  

Please report any problems to me (unless you can fix them yourself).


I do not plan to make any further consolidation of this magnitude, but
you may still expect some wide changes related to the introduction of
per-window parameters to supplement the current per-frame parameters,
and swapping of the fringe and display margin.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Status of MAC/W32/X consolidation -- third major patch committed.

[Andrew Choi]

storm@cua.dk (Kim F. Storm) writes:

> I have now consolidated the common code related to frame-parameter
> handling from xfns.c, w32fns.c, and macfns.c into frame.c.
> 
> Again, I have only been able to test this on X (under GNU/Linux), so
> there may be some problems compiling (and running) the consolidated
> code on W32 and MAC.  [...]

A few small changes were required to build on Mac OS X with Carbon.  I
have just checked these in.

Re: Status of MAC/W32/X consolidation -- third major patch committed.

[Kim F. Storm]

Andrew Choi <akochoi@shaw.ca> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > I have now consolidated the common code related to frame-parameter
> > handling from xfns.c, w32fns.c, and macfns.c into frame.c.
> > 
> > Again, I have only been able to test this on X (under GNU/Linux), so
> > there may be some problems compiling (and running) the consolidated
> > code on W32 and MAC.  [...]
> 
> A few small changes were required to build on Mac OS X with Carbon.  I
> have just checked these in.

Thank you.  And thanks to Juanma for fixing the W32 port.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk