GNU bug report logs - #51411
NS port cleanups

Previous Next

Full log

View this message in rfc822 format

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: bug#51411: NS port cleanups
Date: Thu, 28 Oct 2021 11:17:27 +0100

On Thu, Oct 28, 2021 at 09:09:33AM +0800, Po Lu wrote:
> Alan Third <alan <at> idiocy.org> writes:
> 
> > I understand what you did, but I think the better solution is for us
> > is to try to untangle ns_draw_glyph_string, even if that means saving
> > the context there directly on occasion.
> 
> > If you want me to have a look at it let me know, I think I know what
> > needs to be done, but I probably won't get round to it as soon as you
> > could.
> 
> Thanks.  I had a try at it, cleaning up much of what appeared obviously
> unnecessary.  (Though I did not dare change what seemed to be mysterious
> to me.)
> 
> Though, OTOH, I think calling saveGraphicsState inside the overhang draw
> process is not too much of a problem as it happens infrequently enough
> to not be relevant.

Thanks, I'm much happier with this now.

I have no problem with saveGraphicsState being called within functions
directly, I'd just rather avoid adding complexity to commonly used
functions like ns_focus, which I already think are doing too much.

I've got a couple of further notes.

> diff --git a/src/nsterm.m b/src/nsterm.m
> index 4c2a3f287c..051ee511ca 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -1078,11 +1078,20 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
>    /* clipping */
>    if (r)
>      {
> -      [[NSGraphicsContext currentContext] saveGraphicsState];
> +      NSGraphicsContext *ctx = [NSGraphicsContext currentContext];
> +      [ctx saveGraphicsState];
>        if (n == 2)
>          NSRectClipList (r, 2);
>        else
>          NSRectClip (*r);
> +#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
>        gsaved = YES;
>      }
>  }

NSRectClipList creates a union of the passed rectangles and then sets
the clipping rectangle to that, so it contains all of them. That's
useful because NSRectClip uses the intersection of the current
clipping rectangle and the new one.

Does DPSrectclip use the intersection? If so this may not work as
intended and it might be better to union the rectangles ourselves.

> @@ -4195,13 +4091,88 @@ overwriting cursor (usually when cursor on a tab) */
>  
>    /* Draw box if not done already.  */
>    if (!s->for_overlaps && !box_drawn_p && s->face->box != FACE_NO_BOX)
> +    ns_dumpglyphs_box_or_relief (s);
> +
> +  ns_unfocus (s->f);

You unfocus here...

> +
> +  /* Draw surrounding overhangs. */
> +  if (s->prev)
>      {
> -      n = ns_get_glyph_string_clip_rect (s, r);
> -      ns_focus (s->f, r, n);
> -      ns_dumpglyphs_box_or_relief (s);
> -      ns_unfocus (s->f);
> +      struct glyph_string *prev;
> +
> +      for (prev = s->prev; prev; prev = prev->prev)
> +	if (prev->hl != s->hl
> +	    && prev->x + prev->width + prev->right_overhang > s->x)
> +	  {
> +	    /* As prev was drawn while clipped to its own area, we
> +	       must draw the right_overhang part using s->hl now.  */
> +	    enum draw_glyphs_face save = prev->hl;
> +	    struct face *save_face = prev->face;
> +
> +	    prev->face = s->face;
> +	    NSRect r = NSMakeRect (s->x, s->y, s->width, s->height);
> +	    [[NSGraphicsContext currentContext] saveGraphicsState];

... then continue working on the frame here. You either need to focus
again or extend the original focus. Remember you can focus without
setting the clipping, then save and reset the graphics state as
required if you prefer.

-- 
Alan Third

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.