Re: NS port cleanups

[Alan Third]

On Wed, Oct 20, 2021 at 08:44:48PM +0800, Po Lu wrote:
> Po Lu <luangruo@yahoo.com> writes:
> 
> > Here is an updated patch that fixes this, and also fixes a few crashes
> > with the context menu.  (Again, totally untested on macOS)
> 
> And here's a new patch with some other problems fixed (such as broken
> mouse face display, tab bar grab reporting and some crashes in nsmenu).
> And hopefully a working though ugly solution for fixing the tool bar
> height on GNUstep.  Thanks.

Looks better, thanks!

I think I would prefer if you split this along the lines you outlined
in the previous email. It's quite a large patch at the moment doing a
number of apparently unrelated things.


> @@ -1074,15 +1074,25 @@ static NSRect constrain_frame_rect(NSRect frameRect, 
> bool isFullscreen)
>        [view lockFocus];
>      }
>  
> +  NSGraphicsContext *ctx = [NSGraphicsContext currentContext];
> +  [ctx saveGraphicsState];
> +  gsaved++;
> +

Can I ask why you're saving the context every time ns_focus is called?
It shouldn't be necessary unless we're making a change, like calling
NSRectClip. Any deeper functions that make changes save and restore
the context locally.

>    /* clipping */
>    if (r)
>      {
> -      [[NSGraphicsContext currentContext] saveGraphicsState];
>        if (n == 2)
>          NSRectClipList (r, 2);
>        else
>          NSRectClip (*r);
> -      gsaved = YES;
> +#ifdef NS_IMPL_GNUSTEP
> +      DPSrectclip (ctx, NSMinX (*r), NSMinY (*r),
> +                NSWidth (*r), NSHeight (*r));
> +
> +      if (n == 2)
> +     DPSrectclip (ctx, NSMinX (r[1]), NSMinY (r[1]),
> +                  NSWidth (r[1]), NSHeight (r[1]));
> +#endif
>      }
>  }

Is this DPS clipping for font drawing?

> -  /* We draw the cursor (with NSRectFill), then draw the glyph on top
> -     (other terminals do it the other way round).  We must set
> -     w->phys_cursor_width to the cursor width.  For bar cursors, that
> -     is CURSOR_WIDTH; for box cursors, it is the glyph width.  */
>    get_phys_cursor_geometry (w, glyph_row, phys_cursor_glyph, &fx, &fy, &h);

I didn't actually mean for you to get rid of the whole comment as
apart from the first sentence it refers to the action that immediately
follows. Or do you think it's redundant?
>  
> @@ -9867,7 +9873,11 @@ Convert an X font name (XLFD) to an NS font name.
>  \n\
>  Each SYMBOL is `control', `meta', `alt', `super', `hyper' or `none'.\n\
>  If `none', the key is ignored by Emacs and retains its standard meaning.");
> +#ifdef NS_IMPL_GNUSTEP
> +  ns_alternate_modifier = Qalt;
> +#else
>    ns_alternate_modifier = Qmeta;
> +#endif
>  
>    DEFVAR_LISP ("ns-right-alternate-modifier", ns_right_alternate_modifier,
>                 "This variable describes the behavior of the right alternate 
> or option key.\n\
> @@ -9888,7 +9898,11 @@ Convert an X font name (XLFD) to an NS font name.
>  \n\
>  Each SYMBOL is `control', `meta', `alt', `super', `hyper' or `none'.\n\
>  If `none', the key is ignored by Emacs and retains its standard meaning.");
> +#ifdef NS_IMPL_GNUSTEP
> +  ns_command_modifier = Qmeta;
> +#else
>    ns_command_modifier = Qsuper;
> +#endif

I don't think these changes are right:

    By default, GNUstep uses Control_L (left Ctrl) and Control_R (right
    Ctrl) as CONTROL, Alt_L (left alt) as COMMAND, and Alt_R (right alt,
    sometimes called AltGr) as ALTERNATE. As a special exception, if Alt_R
    is not bound to any key on your keyboard, GNUstep will try to use
    Mode_switch for ALTERNATE instead. Some X server map AltGr onto
    ISO_Level3_Shift. To allow for this the second code ALTERNATE may be
    bound to this key.
        
http://www.gnustep.org/resources/documentation/User/Gui/KeyboardSetup.html

We have command bound to super so the default Openstep shortcuts work,
like cmd-q to quit, and alt bound to meta, because that's pretty
standard in Emacs.

Now, it seems to me that right alt does NOT work as alt under
GNUstep, but instead still acts as normal altGr, but I don't think
that's anything to do with how we bind it in Emacs.

And weirdly the windows key is bound to Hyper by default... I kind of
suspect the GNUstep standards for keyboards are quite out of date, but
we should probably try to stick to them.

-- 
Alan Third