unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#47895: 28.0.50; Emacs should only animate images that are visible
@ 2021-04-19 18:19 Lars Ingebrigtsen
  2021-04-19 18:28 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-04-19 18:19 UTC (permalink / raw)
  To: 47895


Emacs is generally slow about calculating animated images, and that
should be fixed (there's a separate bug report for that), but there's
one thing Emacs could be doing:

Not animating images that are off screen.

I think most web browsers do this -- to avoid using too much CPU, which
is the same problem we have in Emacs: Opening a web page with many large
animated GIFs usually makes Emacs very slow.


In GNU Emacs 28.0.50 (build 78, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-04-11 built on xo
Repository revision: 751e801f90339480ea43fc2237fc45c8eb39bd6f
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid


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






^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-19 18:19 bug#47895: 28.0.50; Emacs should only animate images that are visible Lars Ingebrigtsen
@ 2021-04-19 18:28 ` Eli Zaretskii
  2021-04-19 20:49   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2021-04-19 18:28 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47895

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 19 Apr 2021 20:19:19 +0200
> 
> Emacs is generally slow about calculating animated images, and that
> should be fixed (there's a separate bug report for that), but there's
> one thing Emacs could be doing:
> 
> Not animating images that are off screen.
> 
> I think most web browsers do this -- to avoid using too much CPU, which
> is the same problem we have in Emacs: Opening a web page with many large
> animated GIFs usually makes Emacs very slow.

I'd be interested to see a backtrace for such animation of an image
that is not on display.  In general, since animation is triggered (I
think) when the image is being prepared for redisplay, what you
describe shouldn't be happening.  If it dies indeed happen, I have a
couple of guesses how that could be triggered, but it would be nice to
see evidence.

Thanks.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-19 18:28 ` Eli Zaretskii
@ 2021-04-19 20:49   ` Lars Ingebrigtsen
  2021-04-20  2:24     ` Eli Zaretskii
  2021-04-20 13:51     ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-04-19 20:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47895

Eli Zaretskii <eliz@gnu.org> writes:

> I'd be interested to see a backtrace for such animation of an image
> that is not on display.  In general, since animation is triggered (I
> think) when the image is being prepared for redisplay, what you
> describe shouldn't be happening.  If it dies indeed happen, I have a
> couple of guesses how that could be triggered, but it would be nice to
> see evidence.

I unfortunately don't have the time to debug right now, but it's easy
enough to reproduce:

(progn
  (eww "https://lars.ingebrigtsen.no/wp-content/uploads/2018/03/candid.gif")
  (bury-buffer))

This will leave you with an Emacs that uses at least 99% CPU, even if
the eww buffer isn't even displayed.  (At least on this Debian/bullseye
system, starting from -Q.)

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-19 20:49   ` Lars Ingebrigtsen
@ 2021-04-20  2:24     ` Eli Zaretskii
  2021-04-20 13:51     ` Eli Zaretskii
  1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2021-04-20  2:24 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47895

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 47895@debbugs.gnu.org
> Date: Mon, 19 Apr 2021 22:49:55 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'd be interested to see a backtrace for such animation of an image
> > that is not on display.  In general, since animation is triggered (I
> > think) when the image is being prepared for redisplay, what you
> > describe shouldn't be happening.  If it dies indeed happen, I have a
> > couple of guesses how that could be triggered, but it would be nice to
> > see evidence.
> 
> I unfortunately don't have the time to debug right now, but it's easy
> enough to reproduce:
> 
> (progn
>   (eww "https://lars.ingebrigtsen.no/wp-content/uploads/2018/03/candid.gif")
>   (bury-buffer))
> 
> This will leave you with an Emacs that uses at least 99% CPU, even if
> the eww buffer isn't even displayed.  (At least on this Debian/bullseye
> system, starting from -Q.)

Thanks, I will take a look when I have time.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-19 20:49   ` Lars Ingebrigtsen
  2021-04-20  2:24     ` Eli Zaretskii
@ 2021-04-20 13:51     ` Eli Zaretskii
  2021-04-25 18:48       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2021-04-20 13:51 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47895

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 47895@debbugs.gnu.org
> Date: Mon, 19 Apr 2021 22:49:55 +0200
> 
> (progn
>   (eww "https://lars.ingebrigtsen.no/wp-content/uploads/2018/03/candid.gif")
>   (bury-buffer))

The timer set up by image.el keeps "displaying" the animated GIF.

In this simple case, we could use

 (get-buffer-window (plist-get (cdr image) :animate-buffer) 'visible)

in image-animate-timeout to see if the buffer is displayed in any
window.  The harder questions are:

  . if the buffer is not displayed, what to do with the timer?
    continue running it? if so, how to interpret the LIMIT arg?

  . what if the window _is_ displayed, but the image is not visible?
    I think we'd need to record the image's buffer position in its
    plist, so that we could use pos-visible-in-window-p to find out
    whether the image is visible





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-20 13:51     ` Eli Zaretskii
@ 2021-04-25 18:48       ` Lars Ingebrigtsen
  2021-04-25 19:01         ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-04-25 18:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47895

Eli Zaretskii <eliz@gnu.org> writes:

> The timer set up by image.el keeps "displaying" the animated GIF.

Yup.  But if the image isn't displayed, why does this take any time?
That is, the image.el code increases the :index in the image spec, and
then calls force-window-update, and it's presumably this that takes
time?  Even if the image isn't displayed?  I find that part rather
unexpected.

Hm...  No, even without the force-update, Emacs uses 100% CPU.  I've
done some more testing, and even if image-animate-timeout just does:

  (plist-put (cdr image) :animate-tardiness
             (+ (* (plist-get (cdr image) :animate-tardiness) 0.9)
                (float-time (time-since target-time))))

and then re-runs itself, it'll use 100% CPU.  This seems to indicate
that any alteration of the image plist leads to Emacs re-computing the
image -- even if it isn't displayed?  Both of these things seem
unexpected: 1) Altering a plist item that's not relevant for the display of
the image shouldn't lead to an image recomputation, and 2) if the image
isn't displayed, it shouldn't be recomputed anyway.

I guess 1) is because the redisplay code can't find the image in the
image cache -- because it has no concept of "this is an image-relevant
plist item" -- it just computes a hash of all the properties.

> In this simple case, we could use
>
>  (get-buffer-window (plist-get (cdr image) :animate-buffer) 'visible)
>
> in image-animate-timeout to see if the buffer is displayed in any
> window.  The harder questions are:
>
>   . if the buffer is not displayed, what to do with the timer?
>     continue running it? if so, how to interpret the LIMIT arg?

I'd keep interpreting that the same -- that is, count down, even if the
image isn't displayed.

>   . what if the window _is_ displayed, but the image is not visible?
>     I think we'd need to record the image's buffer position in its
>     plist, so that we could use pos-visible-in-window-p to find out
>     whether the image is visible

Or just compute the position on each iteration -- the image may change
its position if more text is inserted, for instance.

But I'm still wondering about why this doesn't just work
"automatically" -- if we could handle this in the redisplay code, that
would be more natural.

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-25 18:48       ` Lars Ingebrigtsen
@ 2021-04-25 19:01         ` Eli Zaretskii
  2021-04-25 19:07           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2021-04-25 19:01 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47895

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 47895@debbugs.gnu.org
> Date: Sun, 25 Apr 2021 20:48:47 +0200
> 
> Hm...  No, even without the force-update, Emacs uses 100% CPU.

Beware: changing that might make the timer run more frequently, so you
might see CPU usage soar even though Emacs does almost nothing.

>   (plist-put (cdr image) :animate-tardiness
>              (+ (* (plist-get (cdr image) :animate-tardiness) 0.9)
>                 (float-time (time-since target-time))))
> 
> and then re-runs itself, it'll use 100% CPU.  This seems to indicate
> that any alteration of the image plist leads to Emacs re-computing the
> image -- even if it isn't displayed?  Both of these things seem
> unexpected: 1) Altering a plist item that's not relevant for the display of
> the image shouldn't lead to an image recomputation, and 2) if the image
> isn't displayed, it shouldn't be recomputed anyway.

We access a different frame of the GIF image, so that would mean
regenerating the pixmap for the image, no?

> > In this simple case, we could use
> >
> >  (get-buffer-window (plist-get (cdr image) :animate-buffer) 'visible)
> >
> > in image-animate-timeout to see if the buffer is displayed in any
> > window.  The harder questions are:
> >
> >   . if the buffer is not displayed, what to do with the timer?
> >     continue running it? if so, how to interpret the LIMIT arg?
> 
> I'd keep interpreting that the same -- that is, count down, even if the
> image isn't displayed.

But then if and when the image becomes visible, it won't show the
animation, because it already reached the LIMIT.  Right?

> >   . what if the window _is_ displayed, but the image is not visible?
> >     I think we'd need to record the image's buffer position in its
> >     plist, so that we could use pos-visible-in-window-p to find out
> >     whether the image is visible
> 
> Or just compute the position on each iteration -- the image may change
> its position if more text is inserted, for instance.

Sure, but even if the position doesn't change we currently cannot tell
if the image is visible.  We have pos-visible-in-window-p, but that
needs a buffer position -- which is why I suggest to record that
position in the image.

> But I'm still wondering about why this doesn't just work
> "automatically" -- if we could handle this in the redisplay code, that
> would be more natural.

Animation doesn't work in redisplay, it works in this code I pointed
to.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-25 19:01         ` Eli Zaretskii
@ 2021-04-25 19:07           ` Lars Ingebrigtsen
       [not found]             ` <YIciJ+1fSjJGcu+P@faroe.holly.idiocy.org>
  2021-05-02  9:35             ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-04-25 19:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47895

Eli Zaretskii <eliz@gnu.org> writes:

>> Hm...  No, even without the force-update, Emacs uses 100% CPU.
>
> Beware: changing that might make the timer run more frequently, so you
> might see CPU usage soar even though Emacs does almost nothing.

It's still updating at the same rate (25 times a second for the image in
question).

> We access a different frame of the GIF image, so that would mean
> regenerating the pixmap for the image, no?

It does, but why does that happen before redisplay has decided to
display the image?

>> I'd keep interpreting that the same -- that is, count down, even if the
>> image isn't displayed.
>
> But then if and when the image becomes visible, it won't show the
> animation, because it already reached the LIMIT.  Right?

Yes.  I think that's fine -- you've asked for X repetitions, and you get
X repetitions, whether it's shown or not.

>> Or just compute the position on each iteration -- the image may change
>> its position if more text is inserted, for instance.
>
> Sure, but even if the position doesn't change we currently cannot tell
> if the image is visible.  We have pos-visible-in-window-p, but that
> needs a buffer position -- which is why I suggest to record that
> position in the image.

Sure.  Could make it a marker, I guess.

>> But I'm still wondering about why this doesn't just work
>> "automatically" -- if we could handle this in the redisplay code, that
>> would be more natural.
>
> Animation doesn't work in redisplay, it works in this code I pointed
> to.

The code just alters some elements in the image plist.  It's unexpected
that this should lead to Emacs doing a lot of work -- unless it's
actually displaying the image.

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
       [not found]             ` <YIciJ+1fSjJGcu+P@faroe.holly.idiocy.org>
@ 2021-04-27 15:51               ` Alan Third
  2021-04-27 23:23                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Third @ 2021-04-27 15:51 UTC (permalink / raw)
  To: Lars Ingebrigtsen, Eli Zaretskii, 47895

(I don't think this sent originally, apologies if it shows up twice.)

On Sun, Apr 25, 2021 at 09:07:48PM +0200, Lars Ingebrigtsen wrote:
> 
> The code just alters some elements in the image plist.  It's unexpected
> that this should lead to Emacs doing a lot of work -- unless it's
> actually displaying the image.

Since the image is being loaded whether it's displayed or not it may
be worth checking if it's also being flushed from the cache every time
through the animation. If so the high CPU usage is presumably due to
Emacs having to reload the frame every time.

-- 
Alan Third





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-27 15:51               ` Alan Third
@ 2021-04-27 23:23                 ` Lars Ingebrigtsen
  0 siblings, 0 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-04-27 23:23 UTC (permalink / raw)
  To: Alan Third; +Cc: 47895

Alan Third <alan@idiocy.org> writes:

> Since the image is being loaded whether it's displayed or not it may
> be worth checking if it's also being flushed from the cache every time
> through the animation. If so the high CPU usage is presumably due to
> Emacs having to reload the frame every time.

Yup; should also be checked.  (I introduced a separate cache for this in
the ImageMagick case many years ago, but the non-ImageMagick one doesn't
have an animation cache, if I remember correctly.)

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-04-25 19:07           ` Lars Ingebrigtsen
       [not found]             ` <YIciJ+1fSjJGcu+P@faroe.holly.idiocy.org>
@ 2021-05-02  9:35             ` Eli Zaretskii
  2021-05-03  9:52               ` Lars Ingebrigtsen
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2021-05-02  9:35 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 47895

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 47895@debbugs.gnu.org
> Date: Sun, 25 Apr 2021 21:07:48 +0200
> 
> > We access a different frame of the GIF image, so that would mean
> > regenerating the pixmap for the image, no?
> 
> It does, but why does that happen before redisplay has decided to
> display the image?

Because image-animate-timeout calls image-metadata (via
image-multi-frame-p), which calls lookup_image, which regenerates the
pixmap.

> >> I'd keep interpreting that the same -- that is, count down, even if the
> >> image isn't displayed.
> >
> > But then if and when the image becomes visible, it won't show the
> > animation, because it already reached the LIMIT.  Right?
> 
> Yes.  I think that's fine -- you've asked for X repetitions, and you get
> X repetitions, whether it's shown or not.

But no repetitions were actually displayed yet, so won't this be
confusing?  Shouldn't we start counting only when the image is
actually visible?

> > Animation doesn't work in redisplay, it works in this code I pointed
> > to.
> 
> The code just alters some elements in the image plist.  It's unexpected
> that this should lead to Emacs doing a lot of work -- unless it's
> actually displaying the image.

It's computing the image, see above.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#47895: 28.0.50; Emacs should only animate images that are visible
  2021-05-02  9:35             ` Eli Zaretskii
@ 2021-05-03  9:52               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 12+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-03  9:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 47895

Eli Zaretskii <eliz@gnu.org> writes:

> Because image-animate-timeout calls image-metadata (via
> image-multi-frame-p), which calls lookup_image, which regenerates the
> pixmap.

Aah!  Thanks; image.el doesn't have to keep calling that function -- it
can just call it once and then stash the data in the image plist.

I've now done that change on the trunk, and my test case went from using
100% CPU to using 7% CPU, which is an improvement.  :-)

Virtually all of the remaining CPU usage comes from the call to
`force-window-update' -- and I guess that shouldn't be called if the
buffer isn't displayed in a window.  Let's see...

Yup, with that change, the CPU usage went down to 2%.

So I think that this problem is now fixed, and I'm closing this bug report.

> But no repetitions were actually displayed yet, so won't this be
> confusing?  Shouldn't we start counting only when the image is
> actually visible?

I don't really have an opinion here -- but the image animation code
hasn't taken this into consideration before, so that would be a change
in behaviour.  

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





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-05-03  9:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 18:19 bug#47895: 28.0.50; Emacs should only animate images that are visible Lars Ingebrigtsen
2021-04-19 18:28 ` Eli Zaretskii
2021-04-19 20:49   ` Lars Ingebrigtsen
2021-04-20  2:24     ` Eli Zaretskii
2021-04-20 13:51     ` Eli Zaretskii
2021-04-25 18:48       ` Lars Ingebrigtsen
2021-04-25 19:01         ` Eli Zaretskii
2021-04-25 19:07           ` Lars Ingebrigtsen
     [not found]             ` <YIciJ+1fSjJGcu+P@faroe.holly.idiocy.org>
2021-04-27 15:51               ` Alan Third
2021-04-27 23:23                 ` Lars Ingebrigtsen
2021-05-02  9:35             ` Eli Zaretskii
2021-05-03  9:52               ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).