GNU bug report logs - #51473
[PATCH] Enable xwidget scrolling optimizations, and other xwidgets improvements

Previous Next

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

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

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: [PATCH] Enable xwidget scrolling optimizations, and other xwidgets
 improvements
Date: Fri, 29 Oct 2021 12:30:45 +0800

[Message part 1 (text/plain, inline)]

The attached changes fix flickering xwidgets by moving xwidget display
into a separate X window, and also enable the scrolling optimization for
xwidgets.

Someone with access to a Mac will have to do the changes necessary for
xwidget scrolling to work correctly on macOS, as they don't work on
GNUstep in the first place.

However, event passthrough doesn't work yet, and will require some
thought.  But that would be a fantastic opportunity to clean up the
current xwidget event handling mess (and perhaps even define a mechanism
for Lisp code to send events into xwidgets, which would make integrating
them into the existing Emacs event model much easier.)  Any thoughts?

Thanks.

[0001-Use-an-X-window-to-display-xwidgets-on-X11.patch (text/x-patch, attachment)]

[0002-Enable-scrolling-optimization-for-xwidgets.patch (text/x-patch, attachment)]

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

From: Po Lu <luangruo <at> yahoo.com>
To: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 29 Oct 2021 21:18:14 +0800

[Message part 1 (text/plain, inline)]

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

> The attached changes fix flickering xwidgets by moving xwidget display
> into a separate X window, and also enable the scrolling optimization for
> xwidgets.
>
> Someone with access to a Mac will have to do the changes necessary for
> xwidget scrolling to work correctly on macOS, as they don't work on
> GNUstep in the first place.
>
> However, event passthrough doesn't work yet, and will require some
> thought.  But that would be a fantastic opportunity to clean up the
> current xwidget event handling mess (and perhaps even define a mechanism
> for Lisp code to send events into xwidgets, which would make integrating
> them into the existing Emacs event model much easier.)  Any thoughts?
>
> Thanks.

And I missed something, you will also need the following patch for
everything to function correctly:

[0003-Destroy-xwidgets-when-destroying-frames.patch (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 00:11:58 +0100

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

> The attached changes fix flickering xwidgets by moving xwidget display
> into a separate X window, and also enable the scrolling optimization for
> xwidgets.

Sounds good...

> Someone with access to a Mac will have to do the changes necessary for
> xwidget scrolling to work correctly on macOS, as they don't work on
> GNUstep in the first place.

Does this mean that these patches will break the Macos build?

> However, event passthrough doesn't work yet, and will require some
> thought.  But that would be a fantastic opportunity to clean up the
> current xwidget event handling mess (and perhaps even define a mechanism
> for Lisp code to send events into xwidgets, which would make integrating
> them into the existing Emacs event model much easier.)  Any thoughts?

I'm not really that familiar with how xwidget works, so er does this
mean that the events work less with your patch than before?

I'm unable to test this at all, because:

larsi <at> elva:~/src/emacs/trunk$ emake;  ./src/emacs -geometry -0+0  
Overriding existing handler for signal 10. Set JSC_SIGNAL_FOR_GC if you want WebKit to use a different signal

** (emacs:2173922): ERROR **: 00:09:01.997: GApplication is required for xdg-desktop-portal access in the WebKit sandbox.
Fatal error 5: Trace/breakpoint trap


This is on Debian/bookwork.  (And without your patch, just building with
xwidget and `M-x xwidget-webkit-browse-url RET fsf.org RET', and Emacs
crashes.)  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: luangruo <at> yahoo.com, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations,
 and other xwidgets improvements
Date: Fri, 05 Nov 2021 09:20:32 +0200

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Fri, 05 Nov 2021 00:11:58 +0100
> Cc: 51473 <at> debbugs.gnu.org
> 
> (And without your patch, just building with
> xwidget and `M-x xwidget-webkit-browse-url RET fsf.org RET', and Emacs
> crashes.)  

Sounds like a serious problem.  Any idea why it crashes?  What does
the backtrace say?

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 15:29:58 +0800

Eli Zaretskii <eliz <at> gnu.org> writes:

> Sounds like a serious problem.  Any idea why it crashes?  What does
> the backtrace say?

Lars, are you using Fedora 34 by any chance?  The WebKitGTK 2 package
there has some kind of bug that also affects the GNOME Yelp viewer, not
just Emacs.

I realize that I've forgotten to update this issue with a few
improvements I've made though.  I'll send them soon, it would be nice if
someone took a look at them afer that.

Thanks.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 08:33:33 +0100

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

> Lars, are you using Fedora 34 by any chance?  The WebKitGTK 2 package
> there has some kind of bug that also affects the GNOME Yelp viewer, not
> just Emacs.

No, I'm using Debian/bookworm.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 15:34:23 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Sounds good...
>
>> Someone with access to a Mac will have to do the changes necessary for
>> xwidget scrolling to work correctly on macOS, as they don't work on
>> GNUstep in the first place.
>
> Does this mean that these patches will break the Macos build?

I tried my best to keep it working, but the scroll optimization will not
function there.

> I'm not really that familiar with how xwidget works, so er does this
> mean that the events work less with your patch than before?

Yes, but I fixed that, and defined a new facility for sending events to
xwidgets from Lisp code in a few other changes.  Should I squash them or
send the new commits as individual patches?  Thanks.

> I'm unable to test this at all, because:

> ** (emacs:2173922): ERROR **: 00:09:01.997: GApplication is required for xdg-desktop-portal access in the WebKit sandbox.
> Fatal error 5: Trace/breakpoint trap

> This is on Debian/bookwork.  (And without your patch, just building with
> xwidget and `M-x xwidget-webkit-browse-url RET fsf.org RET', and Emacs
> crashes.)  

This is probably the same WebKitGtk bug that also happens on Fedora.
Can you tell me the version of your WebKitGtk package and GLib?

If you can't upgrade/downgrade, continually retrying could help.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 15:47:43 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> No, I'm using Debian/bookworm.

Thanks, I tried to explain the situation better in my reply to your
message.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 21:10:13 +0800

[Message part 1 (text/plain, inline)]

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

> Yes, but I fixed that, and defined a new facility for sending events to
> xwidgets from Lisp code in a few other changes.  Should I squash them or
> send the new commits as individual patches?  Thanks.

Also, if you want to try out the enhancements, here they are:

[patches.tar.gz (application/gzip, attachment)]

[Message part 3 (text/plain, inline)]

I have tried to explain the user-facing changes in NEWS and in the
manual.

Thanks.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 14:28:23 +0100

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

> This is probably the same WebKitGtk bug that also happens on Fedora.
> Can you tell me the version of your WebKitGtk package and GLib?

Let's see...  it's these packages?

webkit2gtk-driver/testing 2.34.1-1 amd64
  WebKitGTK WebDriver support

libglib2.0-dev/testing,now 2.70.0-3 amd64 [installed,automatic]
  Development files for the GLib library

> If you can't upgrade/downgrade, continually retrying could help.

I've done an "apt update/upgrade" (and rebuilt Emacs), but it's still
failing in the same way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 14:32:58 +0100

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

> I have tried to explain the user-facing changes in NEWS and in the
> manual.

Thanks; I've only skimmed the patches, but the general gist makes sense
to me.  But I can't really test them, so somebody else will have to do that.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: luangruo <at> yahoo.com, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 14:35:10 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> Sounds like a serious problem.  Any idea why it crashes?  What does
> the backtrace say?

The start of it is:

Overriding existing handler for signal 10. Set JSC_SIGNAL_FOR_GC if you want WebKit to use a different signal

** (emacs:2296186): ERROR **: 14:33:31.815: GApplication is required for xdg-desktop-portal access in the WebKit sandbox.
Fatal error 5: Trace/breakpoint trap
Backtrace:

So I guess the Webkit code is doing something here to exit, so it didn't
seem worth trying to debug further on the Emacs side (for me).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 21:40:20 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Let's see...  it's these packages?
>
> webkit2gtk-driver/testing 2.34.1-1 amd64
>   WebKitGTK WebDriver support

That's not the right package.  You should be looking at the version of
`webkit2gtk', on Debian.

> libglib2.0-dev/testing,now 2.70.0-3 amd64 [installed,automatic]
>   Development files for the GLib library

Interesting, thanks.  Unfortunately, that puts an end to my theory that
this bug only happens with <2.69.3.

FWIW, the only systems I've seen to exhibit this bug have webkit2gtk3
2.32 with GLib 2.68.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Fri, 05 Nov 2021 14:48:03 +0100

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

> That's not the right package.  You should be looking at the version of
> `webkit2gtk', on Debian.

Uhm...  there's no package called exactly webkit2gtk here, but there's
these, perhaps?

libwebkit2gtk-4.0-dev/testing,now 2.34.1-1 amd64 [installed]
  Web content engine library for GTK - development files

webkit2gtk-driver/testing 2.34.1-1 amd64
  WebKitGTK WebDriver support

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 08:00:07 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Uhm...  there's no package called exactly webkit2gtk here, but there's
> these, perhaps?

> libwebkit2gtk-4.0-dev/testing,now 2.34.1-1 amd64 [installed]
>   Web content engine library for GTK - development files

That's the version of the library for GTK 4.x, not GTK 3, but the
versions should be the same.

Thanks for the help, I'll probably have a workaround soon.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 08:22:05 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> webkit2gtk-driver/testing 2.34.1-1 amd64
>   WebKitGTK WebDriver support

A temporary workaround is to disable sandboxing of the WebKit
sub-processes.  Would this be acceptable?  Also, would it be OK to adopt
such a change in the release branch?

Thanks.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 01:25:27 +0100

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

> A temporary workaround is to disable sandboxing of the WebKit
> sub-processes.  Would this be acceptable?

Hm...  I'd rather not, if possible.  But if that's the only non-invasive
fix possible...

> Also, would it be OK to adopt such a change in the release branch?

If Emacs 28 is segfaulting (and I assume it is, but I've only tried
Emacs 29) on a lot of newer systems, then I guess we should.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 10:06:06 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Po Lu <luangruo <at> yahoo.com> writes:
>
>> A temporary workaround is to disable sandboxing of the WebKit
>> sub-processes.  Would this be acceptable?
>
> Hm...  I'd rather not, if possible.  But if that's the only non-invasive
> fix possible...

I will try to find some other non-invasive fix.

>> Also, would it be OK to adopt such a change in the release branch?

> If Emacs 28 is segfaulting (and I assume it is, but I've only tried
> Emacs 29) on a lot of newer systems, then I guess we should.

It's not a segfault, but rather some race condition occurring in
WebKitGTK+, which causes it to invoke one of the GLib logging functions
incorrectly, which then raises SIGTRAP.

This crash also happens in Emacs 27, and also GNOME's Yelp viewer.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 13:39:34 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Hm...  I'd rather not, if possible.  But if that's the only non-invasive
> fix possible...

Another solution is to set the environment variables `SNAP', `SNAP_NAME'
and `SNAP_REVISION', so WebKitGTK thinks it is running in snap and
doesn't go down the faulty Bubblewrap code path.

It works, but it could potentially affect other programs running inside
Emacs as well, and that variable can't be temporarily set because it's
impossible to determine when WebKitGTK will actually launch a
subprocess.

So I think it should be documented in etc/PROBLEMS instead.  Something
along the lines of:

** Emacs crashes with SIGTRAP when trying to start an WebKit xwidget.

The version of WebKitGTK installed on your system is buggy, and errors
out trying to start a subprocess through bubblewrap.  You can prevent
the buggy code from being executed by setting the environment variables
`SNAP', `SNAP_NAME' and `SNAP_REVISION', which tricks WebKit into
thinking Emacs is running in snap and makes it use GLib to launch
subprocesses instead.

WDYT?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations,
 and other xwidgets improvements
Date: Sat, 06 Nov 2021 10:58:25 +0200

> Cc: 51473 <at> debbugs.gnu.org
> Date: Sat, 06 Nov 2021 13:39:34 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> So I think it should be documented in etc/PROBLEMS instead.  Something
> along the lines of:
> 
> ** Emacs crashes with SIGTRAP when trying to start an WebKit xwidget.
> 
> The version of WebKitGTK installed on your system is buggy, and errors
> out trying to start a subprocess through bubblewrap.  You can prevent
> the buggy code from being executed by setting the environment variables
> `SNAP', `SNAP_NAME' and `SNAP_REVISION', which tricks WebKit into
> thinking Emacs is running in snap and makes it use GLib to launch
> subprocesses instead.
> 
> WDYT?

Thanks, I added an entry in PROBLEMS along those lines.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 18:41:27 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> Thanks, I added an entry in PROBLEMS along those lines.

Thanks!  Lars, could you test the latest changes below with the
workaround now described in PROBLEMS?

Also, could someone with a Mac verify that this still builds on macOS,
and preferably implement the missing features on that platform as well?

I hope all goes well, because it would be really nice to see this
installed in the master branch soon.  Many people have been waiting for
xwidgets to be actually useful for a long time.

Thanks.

[patches.tar.gz (application/gzip, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sat, 06 Nov 2021 18:42:28 +0100

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

> Thanks!  Lars, could you test the latest changes below with the
> workaround now described in PROBLEMS?
>
> Also, could someone with a Mac verify that this still builds on macOS,
> and preferably implement the missing features on that platform as well?

I can test both, but it'd be easier to test if it was just one huge
patch.  (When applying for real, we can apply the patch series.)  So can
you post this series as one patch?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 08:41:15 +0800

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I can test both, but it'd be easier to test if it was just one huge
> patch.  (When applying for real, we can apply the patch series.)  So can
> you post this series as one patch?

Thanks, here you go.

[test.diff (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 01:55:37 +0100

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

> Thanks, here you go.

Thanks!  I've now tried on Debian/bookworm, and everything seems to be
working fine.  (But truth be told, I don't see much difference
before/after the patches with some very cursory browsing on
youtube.com and google.com.)

Now for Macos...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 02:04:01 +0100

On Macos, it fails with:

  CC       dispnew.o
In file included from dispnew.c:43:
./xwidget.h:182:12: error: unknown type name 'XEvent'
                                        const XEvent *);
                                              ^
1 error generated.
make[1]: *** [dispnew.o] Error 1


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 09:16:12 +0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Thanks!  I've now tried on Debian/bookworm, and everything seems to be
> working fine.  (But truth be told, I don't see much difference
> before/after the patches with some very cursory browsing on
> youtube.com and google.com.)

Less flickering, for one, and you can also press "e" and try typing.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 09:18:22 +0800

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>   CC       dispnew.o
> In file included from dispnew.c:43:
> ./xwidget.h:182:12: error: unknown type name 'XEvent'
>                                         const XEvent *);
>                                               ^

Does this version fix the problem?

[test.diff (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

Thanks.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 02:23:05 +0100

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

> Does this version fix the problem?

It gets further:

xwidget.c:1951:12: error: no member named 'find_text' in 'struct xwidget'
  if (!xw->find_text)
       ~~  ^
xwidget.c:1983:12: error: no member named 'find_text' in 'struct xwidget'
  if (!xw->find_text)
       ~~  ^
xwidget.c:2021:12: error: no member named 'find_text' in 'struct xwidget'
  if (!xw->find_text)
       ~~  ^

Oh, and I didn't notice this error the first time around, apparently:

print.c:1529:23: error: no member named 'widget_osr' in 'struct xwidget'
                           XXWIDGET (obj)->widget_osr);
                           ~~~~~~~~~~~~~~  ^


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 09:26:41 +0800

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> It gets further:
>
> xwidget.c:1951:12: error: no member named 'find_text' in 'struct xwidget'
>   if (!xw->find_text)
>        ~~  ^
> xwidget.c:1983:12: error: no member named 'find_text' in 'struct xwidget'
>   if (!xw->find_text)
>        ~~  ^
> xwidget.c:2021:12: error: no member named 'find_text' in 'struct xwidget'
>   if (!xw->find_text)
>        ~~  ^
>
> Oh, and I didn't notice this error the first time around, apparently:
>
> print.c:1529:23: error: no member named 'widget_osr' in 'struct xwidget'
>                            XXWIDGET (obj)->widget_osr);
>                            ~~~~~~~~~~~~~~  ^

Thanks, does this work instead?

[test.diff (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 02:29:34 +0100

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

> Thanks, does this work instead?

Now it's:

In file included from window.c:41:
./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
  gchar *find_text;
  ^~~~~
  char
In file included from xdisp.c:465:
./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
  gchar *find_text;
  ^~~~~
  char
In file included from dispnew.c:43:
./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
  gchar *find_text;

repeated a whole bunch more times, but it's just that error, I think.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 09:34:23 +0800

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Now it's:
>
> In file included from window.c:41:
> ./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
>   gchar *find_text;
>   ^~~~~
>   char
> In file included from xdisp.c:465:
> ./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
>   gchar *find_text;
>   ^~~~~
>   char
> In file included from dispnew.c:43:
> ./xwidget.h:65:3: error: unknown type name 'gchar'; did you mean 'char'?
>   gchar *find_text;
>
> repeated a whole bunch more times, but it's just that error, I think.

Good catch, thanks!  Does this work?

[test.diff (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 02:39:26 +0100

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

> Good catch, thanks!  Does this work?

It builds!  ✨

And it seems to work fine, too after doing some cursory browsing to a
handful of sites.  So if you send me a new tar ball of patches to apply,
I'll get them pushed.

(They're to be applied in the sequence they're numbered?)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 09:55:20 +0800

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> It builds!  ✨
>
> And it seems to work fine, too after doing some cursory browsing to a
> handful of sites.  So if you send me a new tar ball of patches to apply,
> I'll get them pushed.

Thanks :)

> (They're to be applied in the sequence they're numbered?)

Yes, I created them with git format-patch.

[patches.tar.gz (application/gzip, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 03:04:05 +0100

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

>> (They're to be applied in the sequence they're numbered?)
>
> Yes, I created them with git format-patch.

Great!  Now applied and pushed to trunk.  😀

(So I'm closing this bug report.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 12:09:00 +0200

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  51473 <at> debbugs.gnu.org
> Date: Sun, 07 Nov 2021 08:41:15 +0800
> 
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> 
> > I can test both, but it'd be easier to test if it was just one huge
> > patch.  (When applying for real, we can apply the patch series.)  So can
> > you post this series as one patch?
> 
> Thanks, here you go.

> --- a/src/print.c
> +++ b/src/print.c
> @@ -1521,8 +1521,20 @@ print_vectorlike (Lisp_Object obj, Lisp_Object printcharfun, bool escapeflag,
>        printchar ('>', printcharfun);
>        break;
>  
> -    case PVEC_XWIDGET: case PVEC_XWIDGET_VIEW:
> -      print_c_string ("#<xwidget ", printcharfun);
> +    case PVEC_XWIDGET:
> +#ifdef HAVE_XWIDGETS
> +      {
> +	int len = sprintf (buf, "#<xwidget %u %p>",
> +			   XXWIDGET (obj)->xwidget_id,
> +			   XXWIDGET (obj)->widget_osr);
> +	strout (buf, len, len, printcharfun);
> +	break;
> +      }

The printed representation of xwidget objects should be documented in
the ELisp manual, under the "Editing Types" section.  In fact, we lack
a subsection there for xwidgets; it should be added.

Thanks.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 20:03:04 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> The printed representation of xwidget objects should be documented in
> the ELisp manual, under the "Editing Types" section.  In fact, we lack
> a subsection there for xwidgets; it should be added.
>
> Thanks.

Will this work?

[0001-Document-the-xwidget-type.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 14:33:21 +0200

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: larsi <at> gnus.org,  51473 <at> debbugs.gnu.org
> Date: Sun, 07 Nov 2021 20:03:04 +0800
> 
> > The printed representation of xwidget objects should be documented in
> > the ELisp manual, under the "Editing Types" section.  In fact, we lack
> > a subsection there for xwidgets; it should be added.
> >
> > Thanks.
> 
> Will this work?

With some minor changes, yes.

> +@node Xwidget Type
> +@subsection Xwidget Type

It is generally a good idea to have a @cindex entry with the name of
the section (but with lower-case letters), so that the section could
be easily found via Info-index.

> +                                                Each window such an
> +xwidget is be displayed in

This style is better avoided: it produces long sentences that are hard
to grasp for non-native speakers, because the important part ("in") is
at the end.  It is better to say it simply:

  Each window that displays an xwidget will also have a @dfn{xwidget
  view}, ...

And add a @cindex for "xwidget-view", again generally a good practice
of indexing each place that introduces new terminology (indicated by
@dfn).

>                                                           , which on
> +X-Windows corresponds to a single X window used to display the widget.

What about NS?  And what about Wayland?

> +Neither of these objects are readable; their print syntaxes look like
> +@samp{#<xwidget>} and @samp{#<xwidget-view>}, respectively.  Lisp
> +programmers should not be interested in the functionality of xwidget
> +views.

I'd drop the second of these sentences.  It doesn't add anything to
the description.

Thanks.

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

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 20:40:49 +0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> It is generally a good idea to have a @cindex entry with the name of
> the section (but with lower-case letters), so that the section could
> be easily found via Info-index.

Thanks, done.

> This style is better avoided: it produces long sentences that are hard
> to grasp for non-native speakers, because the important part ("in") is
> at the end.  It is better to say it simply:
>
>   Each window that displays an xwidget will also have a @dfn{xwidget
>   view}, ...
>
> And add a @cindex for "xwidget-view", again generally a good practice
> of indexing each place that introduces new terminology (indicated by
> @dfn).

Likewise.

> What about NS?  And what about Wayland?

I don't know exactly how it is handled on NS.  On Wayland, it's still an
X window that runs through Wayland's compatibility layer for X
applications.  (Like the rest of Emacs, at present.)

> I'd drop the second of these sentences.  It doesn't add anything to
> the description.

Done as well, please check the attached patch.

[0001-Document-the-xwidget-type.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: larsi <at> gnus.org, 51473 <at> debbugs.gnu.org
Subject: Re: bug#51473: [PATCH] Enable xwidget scrolling optimizations, and
 other xwidgets improvements
Date: Sun, 07 Nov 2021 14:52:43 +0200

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: larsi <at> gnus.org,  51473 <at> debbugs.gnu.org
> Date: Sun, 07 Nov 2021 20:40:49 +0800
> 
> Done as well, please check the attached patch.

Thanks, installed.

Previous Next

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