From: Karthik Chikmagalur <karthikchikmagalur@gmail.com>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: stardiviner <numbchild@gmail.com>, Org mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH v3] Inline image display as part of a new org-link-preview system
Date: Sun, 15 Sep 2024 13:50:18 -0700 [thread overview]
Message-ID: <875xqw63qt.fsf@gmail.com> (raw)
In-Reply-To: <87h6ah72ui.fsf@localhost>
>>> Image cache is cleared _only_ with REFRESH argument.
>>> I think that makes sense, right?
>>
>> Yes. But `org-link-preview-region' is always called with the REFRESH
>> argument set to `t' though.
>
> Sure. What's a problem with that?
Why flush Emacs' entire image cache to preview images in one Org buffer?
This has implications for other Emacs buffers with lots of images, like
Org buffers with LaTeX previews. I'm not sure about how much it matters
though.
> In theory, we might not need to clear image cache as long as we call
> `image-flush' in `org-link-preview-clear'.
image-flush is called unconditionally in `org-link-preview-file', so
it should be fine to not set REFRESH unconditionally. I'll keep the
REFRESH argument around but remove the Emacs-wide (clear-image-cache).
>> Moved (require 'image) to inside the `org-link-preview-file'. I know it
>> doesn't affect performance in any reasonable way, but I'm usually
>> hesitant to do this in functions that run inside loops.
>
> There are pros and cons.
> Ideally, we should have these previews in a separate library, so that
> loading Org mode does not need to require so much staff. Loading time is
> not great already.
>
> My hesitance about top-level require is that ol.el is loaded by default
> and that the preview functionality may or may not be used by the
> users. So, loading image.el ought to be optional.
Okay.
>> ...
>> + ;; Run preview asynchronously in batches:
>> + ;; preview-queue is a list of preview-batch, which is a list of preview-spec
>> + (when (car preview-queue)
>> + (dolist (preview-batch (nreverse preview-queue))
>> + (run-with-idle-timer
>> + org-link-preview-delay nil
>
> It means that you are scheduling every single preview batch to fire at
> the same time. I think that the timers here should run sequentially -
> (1) fire the first batch without delay; (2) wait org-link-preview-delay
> and fire the next batch; (3) ...
You're right, I'll redo this. Scheduling many idle timers at once is
tricky, the manual even warns against making idle timers setting up
the next one with the same idle time:
--8<---------------cut here---------------start------------->8---
Similarly, do not write an idle timer function that sets up another idle
timer (including the same idle timer) with SECS argument less than or
equal to the current idleness time. Such a timer will run almost
immediately, and continue running again and again, instead of waiting
for the next time Emacs becomes idle. The correct approach is to
reschedule with an appropriate increment of the current value of the
idleness time, as described below.
--8<---------------cut here---------------end--------------->8---
So I'll have to do it carefully.
>> + (lambda (previews)
>> + ;; (message "queue: %d" preview-queue-size)
>> + (cl-decf preview-queue-size)
>> + (dolist (preview-spec (nreverse previews)) ;spec is (preview-func overlay path link)
>> + (when-let* ((ov (cadr preview-spec))
>> + (buf (overlay-buffer ov)))
>
> ov or its buffer may not exit at the time the timer is fired.
> Because, for example, the buffer is killed, or because user edited the
> underlying link before it got displayed.
This is handled by the (when-let* ((buf (overlay-buffer ov)))) part.
(overlay-buffer ov) is nil if the overlay is gone, or if the buffer is
gone.
Karthik
next prev parent reply other threads:[~2024-09-15 20:51 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-15 3:28 [PATCH] add a function to only refresh inline images under current headline instead of global buffer Christopher M. Miles
2023-05-15 11:08 ` Ihor Radchenko
2023-05-15 13:01 ` Christopher M. Miles
2023-05-15 14:00 ` [PATCH v2] " Christopher M. Miles
2023-05-16 9:17 ` Ihor Radchenko
2023-05-16 12:18 ` Christopher M. Miles
2023-07-24 11:25 ` Ihor Radchenko
2023-08-01 4:40 ` [PATCH v3] " Christopher M. Miles
2023-08-01 8:04 ` Ihor Radchenko
2023-08-01 12:17 ` [PATCH v3.1] " Christopher M. Miles
2023-08-01 14:09 ` Ihor Radchenko
2023-08-01 15:22 ` Christopher M. Miles
2023-08-01 15:46 ` Christopher M. Miles
2023-08-02 16:08 ` Ihor Radchenko
2023-08-04 6:30 ` Christopher M. Miles
2023-08-02 7:26 ` Ihor Radchenko
2023-08-02 15:44 ` Christopher M. Miles
2023-08-04 8:20 ` Ihor Radchenko
2023-08-05 5:28 ` [PATCH v3.2] " Christopher M. Miles
2024-07-22 10:46 ` Ihor Radchenko
2024-08-01 22:58 ` [PATCH v4.0] " stardiviner
[not found] ` <66a8b73b.170a0220.383476.996e@mx.google.com>
2024-08-12 10:18 ` Ihor Radchenko
2024-08-14 2:04 ` stardiviner
2024-08-18 10:27 ` Ihor Radchenko
2024-08-20 2:02 ` Karthik Chikmagalur
2024-08-20 15:43 ` Karthik Chikmagalur
2024-08-20 18:19 ` Ihor Radchenko
2024-08-20 19:46 ` Karthik Chikmagalur
2024-08-22 13:19 ` Ihor Radchenko
2024-08-23 6:04 ` Karthik Chikmagalur
2024-08-23 23:36 ` [PATCH v1] Inline image display as part of a new org-link-preview system Karthik Chikmagalur
2024-08-24 1:00 ` Karthik Chikmagalur
2024-08-31 14:22 ` Ihor Radchenko
2024-08-31 16:41 ` Karthik Chikmagalur
2024-08-31 16:53 ` Ihor Radchenko
2024-08-31 22:37 ` [PATCH v2] " Karthik Chikmagalur
2024-09-01 13:06 ` Ihor Radchenko
2024-09-02 20:13 ` [PATCH v3] " Karthik Chikmagalur
2024-09-08 7:43 ` Ihor Radchenko
2024-09-09 3:21 ` Karthik Chikmagalur
2024-09-09 6:06 ` Ihor Radchenko
2024-09-09 6:30 ` Karthik Chikmagalur
2024-09-09 16:47 ` Ihor Radchenko
2024-09-09 19:14 ` Karthik Chikmagalur
2024-09-10 16:57 ` Ihor Radchenko
2024-09-10 19:53 ` Karthik Chikmagalur
2024-09-15 7:51 ` Ihor Radchenko
2024-09-09 21:45 ` Karthik Chikmagalur
2024-09-10 16:58 ` Ihor Radchenko
2024-09-10 17:38 ` Karthik Chikmagalur
2024-09-10 18:34 ` Ihor Radchenko
2024-09-10 19:43 ` Karthik Chikmagalur
2024-09-15 8:12 ` Ihor Radchenko
2024-09-15 20:50 ` Karthik Chikmagalur [this message]
2024-09-15 21:57 ` [PATCH v4] " Karthik Chikmagalur
2024-09-17 18:16 ` Ihor Radchenko
2024-09-18 1:44 ` [PATCH v5] " Karthik Chikmagalur
2024-09-21 10:11 ` Ihor Radchenko
2024-09-22 22:00 ` [PATCH v6] " Karthik Chikmagalur
2024-10-03 17:03 ` Ihor Radchenko
2024-10-08 0:07 ` [PATCH v7] " Karthik Chikmagalur
2024-10-10 17:15 ` Ihor Radchenko
2024-10-10 21:23 ` Karthik Chikmagalur
2024-10-12 8:15 ` Ihor Radchenko
2024-10-28 6:13 ` [PATCH v8] " Karthik Chikmagalur
2024-10-28 18:16 ` Ihor Radchenko
2024-10-28 21:53 ` [PATCH v9] " Karthik Chikmagalur
2024-10-29 18:46 ` Ihor Radchenko
2024-10-29 20:45 ` [PATCH v10] " Karthik Chikmagalur
2024-10-30 17:32 ` Ihor Radchenko
2024-10-30 19:50 ` Karthik Chikmagalur
2024-10-31 18:16 ` Ihor Radchenko
2024-11-14 5:43 ` Karthik Chikmagalur
2024-11-14 17:14 ` Ihor Radchenko
2024-11-14 22:10 ` Karthik Chikmagalur
2024-11-21 3:46 ` Karthik Chikmagalur
2024-11-23 16:04 ` Ihor Radchenko
2024-11-25 3:17 ` [PATCH v11] " Karthik Chikmagalur
2024-11-25 18:11 ` Ihor Radchenko
2024-11-26 1:08 ` Karthik Chikmagalur
2024-11-26 17:18 ` Ihor Radchenko
2024-11-26 17:46 ` Ihor Radchenko
2024-11-26 18:45 ` [PATCH v12] " Karthik Chikmagalur
2024-11-27 18:47 ` Ihor Radchenko
2024-11-27 21:53 ` Karthik Chikmagalur
2024-11-26 2:08 ` Karthik Chikmagalur
2024-11-23 16:11 ` [PATCH v10] " Ihor Radchenko
2024-08-18 10:34 ` [FR] Automatically display images in resutls of evaluation (was: [PATCH v4.0] Re: [PATCH] add a function to only refresh inline images under current headline instead of global buffer) Ihor Radchenko
[not found] ` <66c54dfc.a70a0220.3c823a.2899@mx.google.com>
2024-08-22 13:06 ` [FR] Automatically display images in resutls of evaluation Ihor Radchenko
[not found] ` <66c89411.170a0220.3255c1.0cd5@mx.google.com>
[not found] ` <87zfor7b04.fsf@localhost>
[not found] ` <CAL1eYuLOsaS43ahueN4uWiCn+Ykp=p_-t9dzAypKdy1en_53BQ@mail.gmail.com>
2024-09-15 13:33 ` [PATCH v3] " Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875xqw63qt.fsf@gmail.com \
--to=karthikchikmagalur@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=numbchild@gmail.com \
--cc=yantar92@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).