GNU bug report logs - #51411
NS port cleanups

Previous Next

Package: emacs;

Reported by: Po Lu <luangruo <at> yahoo.com>

Date: Tue, 26 Oct 2021 11:43:01 UTC

Severity: normal

Tags: patch

Done: Alan Third <alan <at> idiocy.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51411 in the body.
You can then email your comments to 51411 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Tue, 26 Oct 2021 11:43:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Po Lu <luangruo <at> yahoo.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 26 Oct 2021 11:43:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: bug-gnu-emacs <at> gnu.org
Subject: NS port cleanups
Date: Tue, 26 Oct 2021 19:41:31 +0800
[Message part 1 (text/plain, inline)]
I did not notice that the thread previously titled "NS port
improvements" was moved off bug#51251 onto emacs-devel, but now that
there are concrete changes I think it would be a good opportunity to
post them on bug-gnu-emacs, per the CONTRIBUTE document.

The changes, with the generic improvements split from the changes to
font rendering are attached.

Alan raised several questions, which I answered.  I've reproduced that
part of the discussion below:

> Is there any reason to nest ns_focus? There are (according to Apple)
> performance reasons to not save the context unless you really need to.

Right now it's used in the code that clips to the exact bounds of a
string, if a string's overhangs are already drawn.

I couldn't find a cleaner way to do this, and that situation is rare, so
I think the performance problems will usually be avoided.

> I mean alt as defined by GNUstep in the quote I sent you in my last
> email. I don't think it's our job to say that GNUstep's choice of
> defaults is wrong and therefore do something that would be unexpected
> for a GNUstep user.

Well, on every other platform Meta is on the alt on the users' keyboard.
Emacs in general doesn't conform to the platform expectations WRT to key
bindings, so I think being consistent with Emacs on other platforms is
more important here.

But in the end, it's your call.  Thanks.

[0002-General-improvements-to-NS-port.patch (text/x-patch, attachment)]
[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Tue, 26 Oct 2021 12:40:01 GMT) Full text and rfc822 format available.

Message #8 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Tue, 26 Oct 2021 13:39:15 +0100
On Tue, Oct 26, 2021 at 07:41:31PM +0800, Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> > Is there any reason to nest ns_focus? There are (according to Apple)
> > performance reasons to not save the context unless you really need to.
> 
> Right now it's used in the code that clips to the exact bounds of a
> string, if a string's overhangs are already drawn.
> 
> I couldn't find a cleaner way to do this, and that situation is rare, so
> I think the performance problems will usually be avoided.

Is this the code in ns_draw_glyph_string?

> > I mean alt as defined by GNUstep in the quote I sent you in my last
> > email. I don't think it's our job to say that GNUstep's choice of
> > defaults is wrong and therefore do something that would be unexpected
> > for a GNUstep user.
> 
> Well, on every other platform Meta is on the alt on the users' keyboard.
> Emacs in general doesn't conform to the platform expectations WRT to key
> bindings, so I think being consistent with Emacs on other platforms is
> more important here.

I think we normally accept what the system tells us is Alt, or the
nearest equivalent. In this case GNUstep tells us what it's
understanding of the nearest equivalent (option) is, and I don't see
any reason to override that.

So please remove the changes to modifier keys.
-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Tue, 26 Oct 2021 12:52:01 GMT) Full text and rfc822 format available.

Message #11 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Tue, 26 Oct 2021 20:50:52 +0800
[Message part 1 (text/plain, inline)]
Alan Third <alan <at> idiocy.org> writes:

> Is this the code in ns_draw_glyph_string?

Yes, it is.

> I think we normally accept what the system tells us is Alt, or the
> nearest equivalent. In this case GNUstep tells us what it's
> understanding of the nearest equivalent (option) is, and I don't see
> any reason to override that.

> So please remove the changes to modifier keys.

Fair enough, thanks for the feedback.  Done.

[0001-General-improvements-to-NS-port.patch (text/x-patch, attachment)]

Added tag(s) patch. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Tue, 26 Oct 2021 22:41:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Wed, 27 Oct 2021 17:21:01 GMT) Full text and rfc822 format available.

Message #16 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Wed, 27 Oct 2021 18:20:34 +0100
On Tue, Oct 26, 2021 at 08:50:52PM +0800, Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Alan Third <alan <at> idiocy.org> writes:
> 
> > Is this the code in ns_draw_glyph_string?
> 
> Yes, it is.

That function is a real mess of calls to ns_focus and ns_unfocus. I
think there's no good reason for them to be called as often as they
are, and some related functions (ns_dumpglyphs_stretch) don't need to
call them themselves either.

I think we need to have a rethink of how clipping is handled here. We
don't need to use ns_focus to clip and we repeatedly call ns_focus on
the same rectangle. For a lot of those calls we could replace them
with a single clipping rectangle and adjust it as required for
overhangs or whatever.

I also think you misunderstood what I was saying about the performance
problems with calling saveGraphicsState. Calling it when not required
is frowned upon by Apple as they say it causes performance problems.
Your code is now calling it in ns_focus whether it's required or not.
Ideally we only call it when modifying the clipping rectangle, because
there are times ns_focus is called where we don't modify the clipping
rectangle and therefore we don't need to save the graphics context.

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.
-- 
Alan Third




Severity set to 'wishlist' from 'normal' Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Wed, 27 Oct 2021 21:29:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Thu, 28 Oct 2021 01:10:02 GMT) Full text and rfc822 format available.

Message #21 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Thu, 28 Oct 2021 09:09:33 +0800
[Message part 1 (text/plain, inline)]
Alan Third <alan <at> idiocy.org> writes:

> That function is a real mess of calls to ns_focus and ns_unfocus. I
> think there's no good reason for them to be called as often as they
> are, and some related functions (ns_dumpglyphs_stretch) don't need to
> call them themselves either.

> I think we need to have a rethink of how clipping is handled here. We
> don't need to use ns_focus to clip and we repeatedly call ns_focus on
> the same rectangle. For a lot of those calls we could replace them
> with a single clipping rectangle and adjust it as required for
> overhangs or whatever.

> I also think you misunderstood what I was saying about the performance
> problems with calling saveGraphicsState. Calling it when not required
> is frowned upon by Apple as they say it causes performance problems.
> Your code is now calling it in ns_focus whether it's required or not.
> Ideally we only call it when modifying the clipping rectangle, because
> there are times ns_focus is called where we don't modify the clipping
> rectangle and therefore we don't need to save the graphics context.

I understand what you mean now, thanks.

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

[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
I think these changes are appropriate enough for the font display patch,
as they affect cursor display and overhang drawing.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Thu, 28 Oct 2021 10:18:03 GMT) Full text and rfc822 format available.

Message #24 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Thu, 28 Oct 2021 11:26:02 GMT) Full text and rfc822 format available.

Message #27 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Thu, 28 Oct 2021 19:25:23 +0800
[Message part 1 (text/plain, inline)]
Alan Third <alan <at> idiocy.org> writes:

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

It does intersect the rectangle with the current clipping.  I determined
that from a cursory examination of the source code, as the function
appeared in the documentation as documented, but no documentation was
actually written for it.

>> @@ -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, 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.

Hmm.  Will this work better?  Thanks.

[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 02:39:02 GMT) Full text and rfc822 format available.

Message #30 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Alan Third <alan <at> idiocy.org>, 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 10:38:05 +0800
Stefan Kangas <stefan <at> marxist.se> writes:

> severity 51411 wishlist
> quit

Is it really fair to tag this issue as wishlist?  It fixes two concrete
issues, bad font display on GNUstep being one, and crashes with the
context menu from `context-menu-mode' being another.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 02:57:02 GMT) Full text and rfc822 format available.

Message #33 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Alan Third <alan <at> idiocy.org>, 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sat, 30 Oct 2021 19:56:37 -0700
tags 51411 normal
thanks

Po Lu <luangruo <at> yahoo.com> writes:

> Stefan Kangas <stefan <at> marxist.se> writes:
>
>> severity 51411 wishlist
>> quit
>
> Is it really fair to tag this issue as wishlist?  It fixes two concrete
> issues, bad font display on GNUstep being one, and crashes with the
> context menu from `context-menu-mode' being another.

Sorry, I had missed that part.  It sounds to me that this should better
be tagged "normal", so I'm adjusting the tag back to that with this
message.

Thanks for paying attention to the severity tagging.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 03:14:02 GMT) Full text and rfc822 format available.

Message #36 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Alan Third <alan <at> idiocy.org>, 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 11:13:25 +0800
Stefan Kangas <stefan <at> marxist.se> writes:

> Sorry, I had missed that part.  It sounds to me that this should better
> be tagged "normal", so I'm adjusting the tag back to that with this
> message.
>
> Thanks for paying attention to the severity tagging.

Thanks, but you forgot to reply to the control server.
(Partly my fault for removing it from Cc)




Severity set to 'normal' from 'wishlist' Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Sun, 31 Oct 2021 03:30:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 04:00:02 GMT) Full text and rfc822 format available.

Message #41 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Alan Third <alan <at> idiocy.org>, 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sat, 30 Oct 2021 20:59:24 -0700
Po Lu <luangruo <at> yahoo.com> writes:

> Thanks, but you forgot to reply to the control server.
> (Partly my fault for removing it from Cc)

I actually had it in Bcc, which is best practice, but I said "tags 51411
normal" instead of "severity 51411 normal" so it didn't take.

Should be fixed now.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 10:23:02 GMT) Full text and rfc822 format available.

Message #44 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 10:22:50 +0000
On Thu, Oct 28, 2021 at 07:25:23PM +0800, Po Lu wrote:
> Alan Third <alan <at> idiocy.org> writes:
> 
> > 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.
> 
> It does intersect the rectangle with the current clipping.  I determined
> that from a cursory examination of the source code, as the function
> appeared in the documentation as documented, but no documentation was
> actually written for it.

I think what you'll need to do is union the two rectangles and then
clip to that, rather than clipping them both separately. That will
then provide the same clipping as the NSClipRect code does.

> >> @@ -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, 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.
> 
> Hmm.  Will this work better?  Thanks.

I must be failing to communicate well, we keep seeming to
misunderstand each other.

You still need to focus, however you don't have to clip when you
focus.

For example you could do something like:

    ns_focus (f, nil, 0);
    [[NSGraphicsContext currentContext] saveGraphicsState];
    NSClipRect (r1);
    // do something
    [[NSGraphicsContext currentContext] restoreGraphicsState]

    [[NSGraphicsContext currentContext] saveGraphicsState];
    NSClipRect (r2);
    // do something else
    [[NSGraphicsContext currentContext] restoreGraphicsState]
    ns_unfocus (f);

The only time you don't need to focus is when you can guarantee the
calling function (or its caller, etc.) has already focused the view.
-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 10:35:01 GMT) Full text and rfc822 format available.

Message #47 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 18:34:15 +0800
Alan Third <alan <at> idiocy.org> writes:

> I think what you'll need to do is union the two rectangles and then
> clip to that, rather than clipping them both separately. That will
> then provide the same clipping as the NSClipRect code does.

I meant to say that DPSrectclip intersects (IOW, behaves just as
NSClipRect does).  Unless that's incorrect, I think what I'm doing right
now should work fine.

> I must be failing to communicate well, we keep seeming to
> misunderstand each other.

It could be my problem as well: my reading comprehension is nowhere near
as good as I would rather it be.  Thanks a lot for tolerating it.

> You still need to focus, however you don't have to clip when you
> focus.

After I unfocus here:

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

I make sure to focus again if an overhang might be drawn again (inside
if (s->prev) and if (s->next)), like so:

  /* Draw surrounding overhangs. */
  if (s->prev)
    {
->    ns_focus (s->f, NULL, 0);
      struct glyph_string *prev;

There is, of course, a matching unfocus.  Is that not adequate, and if
so, could you please explain how?

Thanks for the feedback, I really appreciate it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 10:55:01 GMT) Full text and rfc822 format available.

Message #50 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 18:54:34 +0800
[Message part 1 (text/plain, inline)]
Po Lu <luangruo <at> yahoo.com> writes:

> I meant to say that DPSrectclip intersects (IOW, behaves just as
> NSClipRect does).  Unless that's incorrect, I think what I'm doing right
> now should work fine.

Please disregard what I said here, as I just found out my understanding
of NSRectClip has been incorrect.

OTOH, I found a function that should better suit the situation.
GSRectClipList sets both the DPS and NS clipping, so I changed the
function to just use that instead.

Thanks.
[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 11:00:02 GMT) Full text and rfc822 format available.

Message #53 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 10:59:40 +0000
On Sun, Oct 31, 2021 at 06:34:15PM +0800, Po Lu wrote:
> Alan Third <alan <at> idiocy.org> writes:
> 
> > I think what you'll need to do is union the two rectangles and then
> > clip to that, rather than clipping them both separately. That will
> > then provide the same clipping as the NSClipRect code does.
> 
> I meant to say that DPSrectclip intersects (IOW, behaves just as
> NSClipRect does).  Unless that's incorrect, I think what I'm doing right
> now should work fine.

When there are two rectangles we use NSRectClipList, which behaves a
little differently. It uses the union of the two rectangles to
intersect with the existing clipping. So you'll have to do something
like

    u = NSUnionRect (r[0], r[1]);
    DPSrectclip (ctx, NSMinX (u), NSMinY (u),
		   NSWidth (u), NSHeight (u));

> > I must be failing to communicate well, we keep seeming to
> > misunderstand each other.
> 
> It could be my problem as well: my reading comprehension is nowhere near
> as good as I would rather it be.  Thanks a lot for tolerating it.

It turns out in this case I'm getting confused over what's changed in
each patch! Entirely my fault.

> > You still need to focus, however you don't have to clip when you
> > focus.
> 
> After I unfocus here:
> 
>   /* 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);
> 
> I make sure to focus again if an overhang might be drawn again (inside
> if (s->prev) and if (s->next)), like so:
> 
>   /* Draw surrounding overhangs. */
>   if (s->prev)
>     {
> ->    ns_focus (s->f, NULL, 0);
>       struct glyph_string *prev;
> 
> There is, of course, a matching unfocus.  Is that not adequate, and if
> so, could you please explain how?

Sorry, I've got confused. What you're doing here now is good.

I noticed that you removed all calls to ns_focus in
ns_draw_window_cursor and thought you'd removed all calls to ns_focus,
but now I see that's an older change.

Is that a good idea? I think ns_draw_window_cursor is sometimes called
without matching ns_update_begin/end calls, so it needs to focus, or am I
misunderstanding the flow?

-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 11:21:01 GMT) Full text and rfc822 format available.

Message #56 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 19:20:38 +0800
Alan Third <alan <at> idiocy.org> writes:

> When there are two rectangles we use NSRectClipList, which behaves a
> little differently. It uses the union of the two rectangles to
> intersect with the existing clipping. So you'll have to do something
> like
>
>     u = NSUnionRect (r[0], r[1]);
>     DPSrectclip (ctx, NSMinX (u), NSMinY (u),
> 		   NSWidth (u), NSHeight (u));

Yes, thanks, please see my other mail.  Thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 11:27:02 GMT) Full text and rfc822 format available.

Message #59 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 19:26:05 +0800
Alan Third <alan <at> idiocy.org> writes:

> I noticed that you removed all calls to ns_focus in
> ns_draw_window_cursor and thought you'd removed all calls to ns_focus,
> but now I see that's an older change.
>
> Is that a good idea? I think ns_draw_window_cursor is sometimes called
> without matching ns_update_begin/end calls, so it needs to focus, or am I
> misunderstanding the flow?

I don't know if ns_draw_window_cursor is called outside
ns_update_begin/end, but if that's the case then I'm not sure how, for
instance, double buffering would work on the X port.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 12:56:02 GMT) Full text and rfc822 format available.

Message #62 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 12:55:11 +0000
On Sun, Oct 31, 2021 at 07:26:05PM +0800, Po Lu wrote:
> Alan Third <alan <at> idiocy.org> writes:
> 
> > I noticed that you removed all calls to ns_focus in
> > ns_draw_window_cursor and thought you'd removed all calls to ns_focus,
> > but now I see that's an older change.
> >
> > Is that a good idea? I think ns_draw_window_cursor is sometimes called
> > without matching ns_update_begin/end calls, so it needs to focus, or am I
> > misunderstanding the flow?
> 
> I don't know if ns_draw_window_cursor is called outside
> ns_update_begin/end, but if that's the case then I'm not sure how, for
> instance, double buffering would work on the X port.

Some drawing functions are definitely used outside
ns_update_begin/end, at least in the NS port.

Could you please put ns_focus calls back into ns_draw_window_cursor,
just in case they are needed, and we can get these changes put into
master and then maybe look at whether we need all these ns_focus calls
as a separate task (it would certainly be nice if we could get rid of
a few of them!).
-- 
Alan Third




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sun, 31 Oct 2021 13:13:02 GMT) Full text and rfc822 format available.

Message #65 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sun, 31 Oct 2021 21:12:03 +0800
[Message part 1 (text/plain, inline)]
Alan Third <alan <at> idiocy.org> writes:

> Some drawing functions are definitely used outside
> ns_update_begin/end, at least in the NS port.
>
> Could you please put ns_focus calls back into ns_draw_window_cursor,
> just in case they are needed, and we can get these changes put into
> master and then maybe look at whether we need all these ns_focus calls
> as a separate task (it would certainly be nice if we could get rid of
> a few of them!).

Thanks, here's the updated patch (along with the other patch for the
improvements not related to text display).  I will be sure to take a
look at the rest of the ns_focus calls later, if I get the time.

[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]
[0002-General-improvements-to-NS-port.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Fri, 05 Nov 2021 07:46:02 GMT) Full text and rfc822 format available.

Message #68 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Fri, 05 Nov 2021 15:44:48 +0800
[Message part 1 (text/plain, inline)]
Po Lu <luangruo <at> yahoo.com> writes:

> Thanks, here's the updated patch (along with the other patch for the
> improvements not related to text display).  I will be sure to take a
> look at the rest of the ns_focus calls later, if I get the time.

As of 48af19c1 those patches don't apply anymore.  Here's a version that
does.  (I kept my version of the overhang computation code instead of
Daniel's, because the xterm code computes overhangs correctly, and
bug#51105 doesn't apply to it AFAIU.)

[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]
[0002-General-improvements-to-NS-port.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#51411; Package emacs. (Sat, 06 Nov 2021 00:22:02 GMT) Full text and rfc822 format available.

Message #71 received at 51411 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 51411 <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sat, 06 Nov 2021 08:20:49 +0800
[Message part 1 (text/plain, inline)]
Po Lu <luangruo <at> yahoo.com> writes:

> As of 48af19c1 those patches don't apply anymore.  Here's a version that
> does.  (I kept my version of the overhang computation code instead of
> Daniel's, because the xterm code computes overhangs correctly, and
> bug#51105 doesn't apply to it AFAIU.)

And it doesn't apply, again.  This should:

[0001-Improve-font-display-on-NS-port.patch (text/x-patch, attachment)]
[0002-General-improvements-to-NS-port.patch (text/x-patch, attachment)]
[Message part 4 (text/plain, inline)]
Thanks.

Reply sent to Alan Third <alan <at> idiocy.org>:
You have taken responsibility. (Sat, 06 Nov 2021 13:03:02 GMT) Full text and rfc822 format available.

Notification sent to Po Lu <luangruo <at> yahoo.com>:
bug acknowledged by developer. (Sat, 06 Nov 2021 13:03:02 GMT) Full text and rfc822 format available.

Message #76 received at 51411-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51411-done <at> debbugs.gnu.org
Subject: Re: bug#51411: NS port cleanups
Date: Sat, 6 Nov 2021 13:02:03 +0000
On Sat, Nov 06, 2021 at 08:20:49AM +0800, Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Po Lu <luangruo <at> yahoo.com> writes:
> 
> > As of 48af19c1 those patches don't apply anymore.  Here's a version that
> > does.  (I kept my version of the overhang computation code instead of
> > Daniel's, because the xterm code computes overhangs correctly, and
> > bug#51105 doesn't apply to it AFAIU.)
> 
> And it doesn't apply, again.  This should:

Sorry, I've not been very attentive to Emacs stuff recently. I've
pushed these changes to master.

Thank you!
-- 
Alan Third




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 05 Dec 2021 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 113 days ago.

Previous Next


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