TODO progress-bars

[jdtsmith]

Here's a feature for consideration: progress bars for TODO completion stats. When configured, this supplants the org-modern-progress char, and instead implements a boxed progress bar "behind" the text of the stats. It works by styling the [ and ] using blank 'display strings of the right (mix of) background face(s), and also applying the right faces across the xxx/yyy chars.

The granularity of the bar is one character. In practice it's good enough with an 11 char bar width (the default). The granularity is not easy to overcome without introducing specified spaces, which would introduce gaps in the text.

Things to consider:

1. Faces: When using complex inheritance, I had issues with the two bar faces (normal and "done") having slightly different character widths. This led to bar size instability as the fraction changes. I solved that with a fairly explicit pair of faces, which people might like to configure anyway.
2. Alignment: It was quite easy to offer custom alignment of the bar in the same function, so I did this. If this looks good, probably we should possibly also offer the ability to right-align tags.

Glamour shot:

[minad]

Thanks! This looks nice at first glance. After trying it, there are multiple issues:

1. I am not fond of alignment. I feel this makes editing worse due to jumping cursor. Therefore I've also not added this for tags. In org-modern I try to stick to "lightweight" styling which doesn't interfere with editing. But maybe alignment is acceptable in practice.
2. The granularity doesn't feel fine enough. Can it made finer by using a smaller face for the percentage numbers?
3. Display issues on folded headings.
4. The progress bar looks too thick, since it scales with the heading.

Are these issues fixable? 1. is trivially fixable if we decide to not add alignment, but the other issues seem harder or impossible to fix?

(I've considered adding such progress bars in my initial design but didn't go with it for various reasons, some of the above.)

[minad]

Btw, I currently use the Nerd Fonts and use the following setting in my config as alternative:

(setq org-modern-progress "󰝦󰪞󰪟󰪠󰪡󰪢󰪣󰪤󰪥") ;; Nerd fonts

You'll probably only see Tofus. With Nerd Fonts there should be pies with finer granularity.

[minad]

As an alternative we could also create a bitmap progress bar with a thin border, like this =======__ 90%? So far I've avoided dynamically creating images, but it may be okay with a cache.

[jdtsmith]

Thanks for the feedback. I think alignment is OK here since you don't really hand-edit the todo stats, and the matcher doesn't match until the data are populated. Did you try with org-modern-progress-bar-align-pos=nil?

I had another idea for pixel-level granularity: use the [/] as flanking specified spaces of the correct size, and add the full space padding on the "data". Let me see how that goes.

For the height of the bar, we could use the trick I use with mlscroll: reverse video+box. E.g. try something like this (disabling font-lock 1st):

(let ((box '(:line-width (0 . -3))))
  (insert
   "\nNow looks like: "
   (propertize (concat (propertize "    1/" 'display '(raise .2)
                                   'face `( :height 0.7 :foreground "gray80"
                                            :reverse-video t
                                            :box ,box))
                       (propertize "2     " 'display '(raise .2)
                                   'face `( :height 0.7 :foreground "gray50"
                                            :reverse-video t :box ,box))))))

Would need two different colors which contrast with BG color. I don't use larger height headlines, so let me know how that looks for you. Oh I see, if headlines grow larger in height, the box offset has to increase.

For ... styling, we can probably include a final "regular" space to defeat that, if there's nothing past it on the line. I don't know why rear-nonsticky doesn't work.

And as usual when doing stuff like this, I found a display bug (luckily with a workaround).

[minad]

Now this looks really nice. I am happy to add this. Please without the alignment for now. Alignment can be added in a separate PR, maybe with a general option org-modern-right-align which can apply to both tags, progress bar and maybe other things.

For the height of the bar, we could use the trick I use with mlscroll: reverse video+box. E.g. try something like this (disabling font-lock 1st):

This is the same trick I use for the TODO keywords. The problem is only that we lose the border, but it looks even better without the border. More modern ;)

For ... styling, we can probably include a final "regular" space to defeat that, if there's nothing past it on the line. I don't know why rear-nonsticky doesn't work.

It would be good to handle this if possible.

[minad]

Would need two different colors which contrast with BG color. I don't use larger height headlines, so let me know how that looks for you. Oh I see, if headlines grow larger in height, the box offset has to increase.

You can try Prot's Modus or Ef-themes with a headline configuration. I suggest anyway to test with those since they are very popular and widely installed. They are also great of course. :)

[jdtsmith]

Will give Prot's theme's a try. I suppose working in terminal is not a goal, right? mlscroll works in the terminal, but there the quantum of space is 1 char.

[minad]

I suppose working in terminal is not a goal, right? mlscroll works in the terminal, but there the quantum of space is 1 char.

I am not sure about most org-modern features on terminal. We shouldn't make the situation worse however.

[jdtsmith]

OK, turned out pretty well.

• Removed the alignment code.
• Provided two different fonts and a separate -height for configuring bar height.
• Used a negative vertical :box to get skinnier bars, with consistent height independent of line font height.
• Use a divided specified spaces on the [ and ] to achieve pixel-precise "completed" bar length.

It was a challenge to find a loophole for nesting 'display properties. Also, I ~middle-align the text via display raise, and I think my algorithm should be generally applicable, but it's not spelled out anywhere explicitly. I took a look with modus themes, but noted that recently they do not actually resize heading faces. I tried the first suggestion which does so and it seemed good:

In terms of the folded line ... artifacts, the right solution I think is to do something like:

(setq org-ellipsis "↴")
(set-face-attribute 'org-ellipsis nil :inherit 'default :box nil)

The first is generally useful anyway now, with the folding triangles. The 2nd ensures that the ellipsis character never takes on the styling of the headline text (which org explicitly does otherwise). I note this was a problem even with the old progress tags. One wrinkle is updates to these require updating the display table, which org does early. So maybe a "recommendation for the config" thing; have a look.

[minad]

I took a look with modus themes, but noted that recently they do not actually resize heading faces.

Yes, but it is easily configurable in Modus. It is a commonly used configuration.

It was a challenge to find a loophole for nesting 'display properties. Also, I ~middle-align the text via display raise, and I think my algorithm should be generally applicable, but it's not spelled out anywhere explicitly.

Now that you've implemented this improved middle alignment, I wonder if we should also use it for todo, tags, priorities and dates (everything inheriting from org-label). The mechanism used for org-labels is much more primitive and doesn't work too well. It may be less costly though I believe. In any case, it doesn't feel right to have two ways to achieve such box alignment. Can we reuse/factor out your alignment algorithm? Is there optimization potential, to avoid repeated font lookups? What do you suggest?

[jdtsmith]

It did occur to me that the TODO labels could also be "smaller". Note I need to know the text height of the progress bar to calculate raise. People may want a different height for TODO vs the bar. But yes the same approach could be used.

As for optimization, we could look up and cache font info in advance for the org headings, it's just fragile to alternate configs and would not cover e.g. timestamps, which can appear many places. Font lookup is actually quite fast. The even more robust approach is line-pixel-height, but that seems quite variable in speed.

BTW, did you test it on your system? Want to make sure the display propertize display raise works.

[minad]

It did occur to me that the TODO labels could also be "smaller". Note I need to know the text height of the progress bar to calculate raise. People may want a different height for TODO vs the bar. But yes the same approach could be used.

Okay, I think we should try this then.

As for optimization, we could look up and cache font info in advance for the org headings, it's just fragile to alternate configs and would not cover e.g. timestamps, which can appear many places. Font lookup is actually quite fast. The even more robust approach is line-pixel-height, but that seems quite variable in speed.

Maybe we can first factor out the generic code such that we can reuse it for the different syntax elements. Then we can look into optimization possibilities. See also org-modern--pre-redisplay where we could load certain data only once. Or we use some other caching technique and get rid of org-modern--pre-redisplay altogether.

BTW, did you test it on your system? Want to make sure the display propertize display raise works.

No, I haven't but the principle should work. Using rais has been discussed before also for TODO labels.

[jdtsmith]

OK, I'd feel better if you tested. The raise itself isn't the issue as much as displaying a string with it's own display properties (which usually doesn't work, but does with raise and height). It works with both NS and Mac builds for me, so probably no issue, but better to be careful.

For pre-redisplay, Is the idea that that runs once then a bunch of font-locking happens?

I'll do some factoring and we'll see. It will definitely be somewhat slower than just applying the label face, but maybe not too bad.

[jdtsmith]

In the meantime, here's a piece of code you can use to test the raise algorithm with your font and setup. Just turn off font-lock-mode then evaluate:

(cl-loop for sh from 0.35 to 1.0 by 0.15
         with dh = (frame-char-height)
         with descent = (aref (font-info (font-at (point))) 9)
         do
         (cl-loop for h from 1 to 1.85 by 0.15
          for lh = (floor (* h dh))
          for bt = (floor (/ (- lh (* sh dh)) 2))
          for raise = (max 0 (/ (- bt (/ (float (or descent 0)) 2)) lh))
          do
          (insert (propertize (format "\nlh=%d h=%5.2f sh=%5.2f bt=%d raise=%0.2f: ⎹" lh h sh bt raise) 'face `(:height ,h))
              (propertize "  THIS is a TEST  " 'display `((raise ,raise) (height ,sh))
                      'face `( :box (:line-width (0 . ,(- bt)))
                           :width condensed :weight semi-bold
                           :inverse-video t))
              (propertize "⎸" 'face `(:height ,h)))))

For me it looks like:

[jdtsmith]

OK,I refactored and it actually sped up a bit as a result. You can now box + vertically center a region or string providing the desired fractional height and position to probe for font info; see org-modern--valign-box-props.

In terms of speed, in my benchmarking, progress-bar is running in about 300-400µs, whereas progress (which just applies a face) is just a few µs. Font info is quite quick to gather; I think it's perhaps more the string manipulation, haven't been able to get a reliable performance breakdown. Unless you have hundreds of stats tokens I suspect this isn't really significant.

[minad]

OK, I'd feel better if you tested.

Sure, I'll do as soon as I find time.

For pre-redisplay, Is the idea that that runs once then a bunch of font-locking happens?

Yes. But it is not a great idea. We could also remove it and use some memoization directly in the font locking

I'll do some factoring and we'll see.

Thanks.

In the meantime, here's a piece of code you can use to test the raise algorithm with your font and setup. Just turn off font-lock-mode then evaluate:

Done. The result looks good, similar to your screenshot.

In terms of speed, in my benchmarking, progress-bar is running in about 300-400µs, whereas progress (which just applies a face) is just a few µs.

Factor 100 doesn't sound great, but it may still be okay. Maybe we can do better with caching.

Font info is quite quick to gather; I think it's perhaps more the string manipulation, haven't been able to get a reliable performance breakdown. Unless you have hundreds of stats tokens I suspect this isn't really significant.

I doubt that string manipulation is the costly aspect. But even then we could cache tags and their styling.

[minad]

I found a problem. Your snippet does not work if I set line-spacing to a large value. Can we also support line-spacing?

[jdtsmith]

Well, that took a deep dive. Here's a new snippet which respects line-spacing. Note that :box is symmetric w.r.t. to the entire line height, including the line-spacing (which falls below the line).
This leads to the box being offset somewhat below the baseline, if line-spacing is very large.

BTW, I'm not certain this algorithm should be used for TODO's, since those come first and seem to be visually "part of a headline". It might be weird if they don't take on the height of that text.

Update: I now measure the font properties of the (smaller) progress bar face, rather than guessing them by scaling.

(cl-loop
 with f = 0.15
 with ld = (frame-char-height)
 for i upfrom 1
 for sh from 0.5 to 1.1 by 0.1
 with s = (cond ((floatp line-spacing) (* line-spacing ld)) (line-spacing) (t 0))
 for (ls . ds) =
 (save-window-excursion
   (with-temp-buffer
     (insert (propertize "0/1" 'face `(org-modern-progress-bar :height ,sh)))
     (set-window-dedicated-p nil nil)
     (set-window-buffer nil (current-buffer))
     (let ((fi (font-info (font-at (point-min)))))
       (cons (aref fi 3) (aref fi 9))))) 
 do
 (cl-loop
  for j upfrom 1
  for th from 1.0 to 2.25 by .25 do
  (insert "\n" (propertize "TALL" 'face `(:height ,th)))
  (beginning-of-line)
  (let* ((fi (font-info (font-at (point))))
     (d (aref fi 9))
     (lt (aref fi 3))
     (eps (max 1 (* f (- ls ds))))
     (b (round (/ (float (+ lt s (- ds ls) (* -2 eps))) 2)))
     (r (/ (float (- (+ b eps) s d)) ld)))
    (end-of-line)
    (insert (propertize "   12/44   " 'display `((raise ,r) (height ,sh))
            'face `(org-modern-progress-bar :box (0 . ,(- b))))
        (format
         "\t\t[%2d,%2d th=%4.2f sh=%4.2f lt=%d d=%d ls=%4.2f ds=%4.2f eps=%4.2f b=%4.2f raise=%4.2f]"
         i j th sh lt d ls ds eps b r)))))

[jdtsmith]

One way to cache would be to examine the face (e.g. org-level-1) and assume that a given face maps to a prior calculated box setup. That's only about 10-15µs worth though, so probably need to look elsewhere. Maybe all the make-string could be avoided by caching blank strings of given lengths.

[jdtsmith]

Another fly in the ointment: text scale changes. At a given text scale, the algorithm is just fine, but once progress bars are drawn, changes to text scale don't magically recompute their :box and raise setting needed to center the text in the box.

I have a way I think will work to address this that will use the above mentioned dynamic face creation strategy, one per heading-level, as a side effect. In the end this will mean drawing the progress bar won't be much different than the current process: apply a display string with a certain face. I'll probably make a new "middle-centered, inverse, label" face for headlines and default text, and inherit from that; other tags can then do the same if they want.

[jdtsmith]

OK this is going to work out I think. But one question I need input on: should heading labels/bars be constant pixel height (ala SVG buttons), or an adaptive fraction of the headline height?

Right now org-modern simply sets :height 0.8 for the label face. A face :height attribute scales w.r.t. the "underlying face", i.e. org-level-1/2/3/. In a setup with different heights for different headings, the labels are therefore not constant height. In contrast, a display (height ... property sets a height relative to the default character height, disregarding the "underlying face". Also a hard-coded :height (in 1/10th points) does the same, but that's kind of inflexible.

[minad]

We have to try both options I guess. But most likely adaptive height will look better with larger headlines. Could it be configurable?

[jdtsmith]

The last commit puts this in a non-working state, but I'd like to get your eyeballs on it before going further. Main differences:

1. org-modern--update-heading-label-faces: uses a temp buffer to measure actual font info and, in case of text-scale, apply remappings. I'll need to expand this to measure the box label font for both fixed and adaptive, for as many distinct heading faces as there are. Not a big deal.
2. To allow the user to select either fixed or adaptive height for various entities, there's a new custom var org-modern-fixed-height-entities. Based on its value, faces have :height applied to them dynamically; that is no longer part of the face definition. If you want fixed height, you can't have :height but must use display ((height ....
3. The helper routine org-modern--heading-label-box can be used to apply the appropriate face/display to box and center in-buffer text or a string, either fixed or adaptive. The idea is all the font-lock callbacks that wanted box centering (including future ones) would use this. This doesn't yet do box-centering for any other entities.

This is looking pretty involved so I wanted to check in and get your thoughts. Would this ever be needed outside of headings? No rush.

[jdtsmith]

OK, I found a bit more time and moved this forward. The current setup is:

• A set of label types which can be aligned and boxed:

  todo-done progress-bar priority tag timestamp-active timestamp-inactive

  These have corresponding faces, which is where any font-altering attributes should go (if desired). Note that so far I've only "turned on" box-labeling for progress-bar.

• org-modern-fixed-height-types is a list of the above types for labels intended to have fixed absolute height, specified as a fraction of the default frame char height, via org-modern-fixed-height.
• On startup and text-scaling, the fonts for default text and each org level are measured and used to set or remap the appropriate :box-carrying faces.

One thing that is important if you want the fixed height labels to actually be fixed height is to prevent other settings from increasing line height beyond the font itself. For example, org-checkbox by default adds an (outer) box which increases line height on list items. Switching it to (:line-width (2 . -2) :style pressed-button) and changing checkbox glyphs to:

(org-modern-checkbox
   '((?X . "✔")
    (?- . "┅")
    (?\s . " ")))

really works out well:

In terms of performance for the progress bar font-lock function, it still measures somewhat slow, about 700µs. I'm suspicious of my measurements though, because most of that time is not parsing the stats string, calculating the bar size, selecting the appropriate faces based on heading level, creating and concatenating the bar strings, or applying the properties to that replacement string. Instead, most of the time (>95%) comes exclusively from adding display and face text properties to the buffer itself. So maybe redisplay is getting included in my benchmark-run measurement.

Normal labels (e.g. TODO) with just a face and raise applied should be pretty fast; haven't yet tested.

[minad]

One thing that is important if you want the fixed height labels to actually be fixed height is to prevent other settings from increasing line height beyond the font itself. For example, org-checkbox by default adds an (outer) box which increases line height on list items.

I assume this is also a problem of your face. With Iosevka the check boxes should not increase in size, even with the composed characters.

In terms of performance for the progress bar font-lock function, it still measures somewhat slow, about 700µs...So maybe redisplay is getting included in my benchmark-run measurement.

"somewhat slow" ;) Can we get a reliable benchmark? Also we should make sure that redisplay time does not go up significantly. It can very well happen that there are around 100 or more tags in the visible region. If time is in the order of 1ms, font locking will become very noticeable.

[jdtsmith]

One thing that is important if you want the fixed height labels to actually be fixed height is to prevent other settings from increasing line height beyond the font itself. For example, org-checkbox by default adds an (outer) box which increases line height on list items.

I assume this is also a problem of your face. With Iosevka the check boxes should not increase in size, even with the composed characters.

It's the org face itself which adds a positive width box attribute by default, nothing to do with the font; my changes were just for the appearance. But yes if you get glyphs from other faces this can also happen. In practice it means users need to exercise some care to get uniform label heights. But now the situation is non-uniform anyway so maybe not so bad. A documentation note may suffice.

In terms of performance for the progress bar font-lock function, it still measures somewhat slow, about 700µs...So maybe redisplay is getting included in my benchmark-run measurement.

"somewhat slow" ;) Can we get a reliable benchmark? Also we should make sure that redisplay time does not go up significantly. It can very well happen that there are around 100 or more tags in the visible region. If time is in the order of 1ms, font locking will become very noticeable.

Maybe we can generate a stress test file and time full font locking of it. What's weird is the "expensive" part seems the same as what org-modern already does plenty of: apply some faces and display props.

[minad]

It's the org face itself which adds a positive width box attribute by default, nothing to do with the font; my changes were just for the appearance.

By default? Maybe I have deactivated that in my setup. I don't recall.

Maybe we can generate a stress test file and time full font locking of it. What's weird is the "expensive" part seems the same as what org-modern already does plenty of: apply some faces and display props.

Yes, this would be ideal.

[jdtsmith]

By default? Maybe I have deactivated that in my setup. I don't recall.

You are right, my (old) theme had done this; it was easy to work around.

I've cleaned out the old progress code now so you can get a clearer look. So far labeling is only applied to the progress-bars.

For other types of labels, like TODO, getting them box-labeled takes a little bit of work, since nested display doesn't (for the most part) work, and you use display for space-padding. You could easily put a replacing display property over the entire thing with the appropriate raise and height, just as it is for the progress-bar text.

The other even simpler idea would be to eliminate the space-padding, and use a positive horizontal (left/right) "width" of the box on the todo box-label faces. Would just need an option for that width (in pixels or say fraction of a character width). Then to style TODO(/DONE) you'd only need to apply the collection of faces and raise (+height if TODO is configured to be a fixed-height style).

The live face dispatch for todo (org-modern-todo-faces) is a bit of a problem, since we're aiming to have all font-affecting face attributes be applied at the level of org-modern-todo face, so we can reliably measure their effects.

[minad]

You could easily put a replacing display property over the entire thing with the appropriate raise and height, just as it is for the progress-bar text.

No, we cannot do this. The idea is to keep the TODO label editable. If we replace it at once, we will lose that. It is okay for the progress bar, since there the display is further away from the underlying actual text.

[jdtsmith]

You could easily put a replacing display property over the entire thing with the appropriate raise and height, just as it is for the progress-bar text.

No, we cannot do this. The idea is to keep the TODO label editable. If we replace it at once, we will lose that. It is okay for the progress bar, since there the display is further away from the underlying actual text.

Ah, good point. The wide :box idea sounds doable then. Also, that gives me an idea how I can keep progress stats editable and avoid using a replacing string altogether.... give me some time to work that idea up.

[jdtsmith]

OK, quite a bit more progress. I radically simplified the progress-bar code (as usual: by taking pencil to paper and working through the constraint problem). Now drawing the bar uses only specified space + face on the [ and ] and face+display on the x/y text itself — no more blank string creation, no more replacing text. This also allows you to edit the stats if you so choose, or (more practically) move point through naturally, copy them, etc. The only new constraint required to achieve this is that the bar must break at the boundary of the text, or inside it (except for the special case of full/empty bars).

I also made progress on the performance issue. At first as I was dismayed because my much tighter bar code appeared to run slower. It turns out all I have been exercising with my former tests is Emacs' quite poor GC performance. Since the progress bar creates custom lisp lists, continuously replacing the properties over the same region of text is an extreme anti-pattern. If I made those lists fixed in any way, an immediate ~50-100x speed improvement appeared: suspicious. Your original progress function didn't have that problem in my tests, because it uses a cache and therefore creates no new lisp objects, so doesn't trigger GC. Man, we need a newer/smarter GC.

In any case, for a more appropriate test, I tried the below (feel free to give it a try on your end). On my machine this results in Progress bar: old=4.16 new=11.15 µs on a file with 5000 random bars. Not too bad! I think a file with 5000 progress bars visible is pretty special.

At this point it's worth some testing on your end to be sure you're happy with this direction. As you mention this is a reasonably complex addition so it's worth taking time to digest. If so, we can look at implementing fixed box labels for TODO/etc. We could also consider a cache for the various display/face lists, e.g. by setting the granularity resolution to 0.1 char-widths. But with such an approach you'd eventually accumulate many hundreds of lists anyway, and suffer the overhead of checking the cache for them, many cache misses, etc. so I doubt it's much of a win.

(require 'lorem-ipsum)
(let ((re (rx ?\s (group ?\[)
          (group (or (and (group (+ num)) ?%)
             (and (group (+ num)) ?/ (group (+ num)))))
          (group ?\])))
      (re2 " \\(\\(\\[\\)\\(?:\\([0-9]+\\)%\\|\\([0-9]+\\)/\\([0-9]+\\)\\)\\(\\]\\)\\)")
      (n 5000)
      (mx 72) op np)
  (with-current-buffer (get-buffer-create "*OM-PBT*")
    (erase-buffer)
    (dotimes (_ n)
      (lorem-ipsum-insert-sentences)
      (let* ((cnt (max 2 (random mx)))
         (cmp (random cnt)))
    (insert (format "[%d/%d]\n" cmp cnt))))
    (pop-to-buffer (current-buffer))
    (goto-char (point-min))
    (garbage-collect)
    (setq np
      (/ (car
          (benchmark-run 1
        (while (re-search-forward re nil t)
          (org-modern--progress-bar))))
         n 1e-6))
    (goto-char (point-min))
    (remove-text-properties (point-min) (point-max) '(display nil face nil))
    (garbage-collect)
    (setq op
      (/ (car
          (benchmark-run 1
        (while (re-search-forward re2 nil t)
          (org-modern--progress))))
         n 1e-6))
    (message "Progress bar: old=%0.2f new=%0.2f µs" op np)))

[minad]

The fixed height progress bars look nice, but I still observe a few display issues when scaling the text size. It looks as if the bar is not recomputed properly. But maybe the reason is the constraint you mentioned, about the breaking of the bar at the text. So we won't get centering?

Regarding the fixed height formatting - I have difficulties evaluating the benefit of this change, since I cannot compare it to the status quo for simple syntax elements like tags or todo labels. I'd like to see the visual improvement if we go through the difficulty with the raise display property, the font computations and so on.

If we won't go with fixed-height labels in general, we could also use non-fixed height for the progress bars, right? The alignment wouldn't be as good, but the code would be much simpler. But maybe it will look prohibitively ugly in contrast to your tuned formatting. However pulling in the fixed-height only for the progress bars seems a bit over the top for me. For me it looks like an all or nothing decision. What do you think?

You mentioned memory pressure and caching. I took a bit of care to avoid excessive object creation. So I am very much in favor of keeping this property, given the current GC situation (at least there is hope now). Elements which are used often (tags, todos) should better be cached. For the progress bar, dynamic computations may be okay, since these objects likely don't appear as often.

[jdtsmith]

The fixed height progress bars look nice, but I still observe a few display issues when scaling the text size. It looks as if the bar is not recomputed properly. But maybe the reason is the constraint you mentioned, about the breaking of the bar at the text. So we won't get centering?

Vertical centering works perfectly for me and is definitely designed to preserve centering as font size changes. Do you mean changing the text size using text-scale-mode? That should recompute the centering and apply face remaps. Check face-remapping-alist to see. You can't reapply raise obviously, but it's relative so no need. If you change the line spacing or default font, you need to re-init. Or do you mean horizontal centering? That is optimized given the mentioned "break on or in" constraint, but text scale has nothing to do with that.

Regarding the fixed height formatting - I have difficulties evaluating the benefit of this change, since I cannot compare it to the status quo for simple syntax elements like tags or todo labels. I'd like to see the visual improvement if we go through the difficulty with the raise display property, the font computations and so on.

The status quo was my first entry at the top, to which you objected as not scaling well with changes in line height (e.g. modus themes large headings). You can see this well here when comparing the (uncentered/full height box) TODO and tags/priority with the progress bar (modus-vivendi, using the first mentioned heading style example):

Are there other problem cases for labels, or is it mostly for tall fonts with extra line spacing?

If we won't go with fixed-height labels in general, we could also use non-fixed height for the progress bars, right? The alignment wouldn't be as good, but the code would be much simpler. But maybe it will look prohibitively ugly in contrast to your tuned formatting. However pulling in the fixed-height only for the progress bars seems a bit over the top for me. For me it looks like an all or nothing decision. What do you think?

Yes we could certainly let the bars be whatever height a fixed amount of negative box gets you without much trouble, but this height will vary based on line height. I'd personally be OK with that, since that's the treatment TODO/tags/etc. already get anyway. But then again I don't use varying line heights. If I did, I bet it would bug me (see above screenshot).

Is your main concern the complexity of the code to support raise + :box based vertical centering? There is one other idea which is kind of lame: use underline as a (super-skinny) "bar" under the text. But I'd probably rather have varying-height bars than that. It's too bad :box doesn't allow floating point fractions (of char width/height), then these problems would be much simpler.

You mentioned memory pressure and caching. I took a bit of care to avoid excessive object creation. So I am very much in favor of keeping this property, given the current GC situation (at least there is hope now). Elements which are used often (tags, todos) should better be cached. For the progress bar, dynamic computations may be okay, since these objects likely don't appear as often.

I noticed that, and I've learned the wisdom of it through experiment. For other simple non-dynamic elements, caching is much more straightforward, and I agree, should be prioritized.

[minad]

Yes, I wonder if the added complexity is worth it, considering potential reliability issues and performance. For example the ATTACH/IMPORTANT tags in your screenshot look okay, despite not having gotten any special treatment, if I've understood correctly? The progress bars of course look better, but maybe it is possible to get an equally good result via the current approach, adjusting only the box line-width in org-modern--update-label-face. I mean sticking to the current approach for vertical alignment.

[minad]

Vertical centering works perfectly for me and is definitely designed to preserve centering as font size changes. Do you mean changing the text size using text-scale-mode? That should recompute the centering and apply face remaps. Check face-remapping-alist to see. You can't reapply raise obviously, but it's relative so no need. If you change the line spacing or default font, you need to re-init. Or do you mean horizontal centering? That is optimized given the mentioned "break on or in" constraint, but text scale has nothing to do with that.

I think what confused me about the progress bar is that the width stays constant even when scaling the text. Why is that? Therefore the horizontal centering changes when scaling.

Regarding the complexity argument - when I put :tags: next to the [100%] progress, I don't see much of a difference in vertical alignment, when I use your code, even when I scale the text. Therefore I wonder about why we need all this. The difference becomes apparent when I set a large value of line-spacing. Then the :tags: will quickly look messed up, while your progress bar stays aligned better. However the progress bar will vertically shift down relative to the baseline of the text (and the text will also vertically lose alignment, but only slightly), such that the result won't be perfect anyway. This means large values of line spacing won't work with either approach?

[jdtsmith]

Yeah the only thing getting centering treatment here is the progress bar. I think performance is a non-issue; in particular the memory load from progress-bars is independent of constant-height stuff. The only cost of that (aside from the code) is 10ms of computing fonts once per session (or text-scale).

I think what confused me about the progress bar is that the width stays constant even when scaling the text. Why is that? Therefore the horizontal centering changes when scaling.

Wow, I never noticed that, you are right. ~And IMO you've identified... a bug in the display code for specified space!~ Update: this is wrong, just need to specify as (num . width).

The brackets should stay lined up as you text-scale, but they do not. In fact all specified space seems to be w.r.t. the base character size. I'll open a report.

The difference becomes apparent when I set a large value of line-spacing.

Try a large headline face with a small label size; that's also where the centering matters most. It's easy to preview a non-fixed height bar: I just switch the box face to '(:box :line-width (0 . -1)) (so no fixed height) and it looks like:

For large line spacing, given that :box is symmetric about the top and bottom line boundaries, and line spacing is explicitly included in that (which seems weird to me, but anyway), you can get some drop below the baseline for some sizes. Making labels shorter helps.

[jdtsmith]

I fixed the text scale bar width issue; give that a look. BTW, the pre-redisplay and update-label stuff could I think be eliminated with the text-scale-mode hook the progress-bars currently employ.

[jdtsmith]

And here it is with line-spacing of 0.5. I'm OK if we want to forgo all the constant height/centering for now. Fairly separable from the progress bar code itself.

No raise/vertical centering:

vs.

With vertical centering

More uniform, but slight label drop. I don't know what kind of line-spacing amount people actually use. I think it's pretty rare.

[minad]

Thanks. The problem with your "no raise/vertical centering" screenshot is, that it is not configured properly. One can set org-modern-label-border to a different value to achieve a similar look as the one below (the progress bar could potentially even use its own border value). So the comparison is unfair. For example the label drop for the progress bar inside the text is pretty significant, and visually not appealing. We have to separate the two issues:

1. Progress bar formatting
2. Label raise

I am not really convinced by the label raising technique, given the gains and complexity. You threw a lot of stuff at the problem, face remapping, separate buffer computations, computation per face and so on.

I would like to evaluate the techniques separately. For example we could compare label alignment in the simple case for tags and then evaluate the benefits. If it turns out that the new alignment is significantly better, use it (but right now it doesn't look like it). Alternatively use the current technique for the progress bar as you had in the beginning, but tune it slightly better. The result will be that the progress bar scales with heading and gets more easily off-track with line-spacing>0 but we won't need the complex mechanisms.

[jdtsmith]

Some intermediate posture along the complexity-visual gain axes seems worthwhile to pursue. I'm just not sure of the approach. The label raise is key to small and/or fixed height :box formats, otherwise there will non-centering of the text. Do you have a screenshot of what a tuned setup looks like for a modus varying height headline style like the above (ignoring progress bars). I guess I don't understand how :box can adapt to different line heights in the current setup.

I see the order of potential improvement (and complexity) as:

1. Tuning :box to be "about right". This is already done in pre-redisplay (though ideally would be done in a text-scale-mode-hook).
2. Adding some calculated amount of display raise for basic text centering. One and done.
3. Adding display height for fixed height labels.
4. Allowing each label type to have different font and height styling (fixed or not).
5. Doing this separately for all unique headlines (and keeping track), to adapt to their native font heights.
6. Updating :box via face remapping for all text-scaling amounts.

This PR currently does all of the above.

One modest complexity reduction would be insisting all labels are fixed in text height (user decides multiple of default window char). That would save all the -type variations of box faces and associated raise but still would need to adapt per headline. It's not much difference though.

[minad]

The label raise is key to small and/or fixed height :box formats, otherwise there will non-centering of the text.

Why? I don't raise the text currently in tags and todo labels and it still looks okay. Of course the alignment will get off for larger line-spacing. (EDIT: The point is that we currently don't have fixed-height labels.)

I guess I don't understand how :box can adapt to different line heights in the current setup.

It doesn't have to adapt to different line heights. The only requirement is that the label box is smaller than the surrounding text, such that we get the desired label effect (or progress bar, smaller in height). Scaling is not a requirement. Unfortunately I wasn't clear about that in the beginning.

This is already done in pre-redisplay (though ideally would be done in a text-scale-mode-hook).

I don't necessarily agree that the text-scale-mode-hook is better. It hooks into another independent subsystem, and it is not a catch all mechanism. There could also be other mechanisms at play, which change sizes, and we would be missing those.

Another idea could be to display the progress bars like this 50%===_ where the progress bar parts === and are scaled in size. Then we would avoid the alignment problem relative to the progress bar parts.

What do you suggest on how to proceed? How difficult is it to adapt the tag display to use your raise code? Then we could side by side compare the current display against the new implementation, but only for the label display. I cannot do this right now, since I don't have a progress bar display right now.

Another line of development could target Emacs directly. I think Emacs needs a facility to specify vertical text centering for line-spacing > 0. This would solve a lot of the issues we are facing right now.

[minad]

A simple alternative proposal: https://github.com/minad/org-modern/commit/6c192145b16cfcfbdb1eb923325d03e8fefd805d

What do you think?

[jdtsmith]

Why? I don't raise the text currently in tags and todo labels and it still looks okay. Of course the alignment will get off for larger line-spacing. (EDIT: The point is that we currently don't have fixed-height labels.)

Just for centering. Diagram of the whole gory details (don't judge, it was too painful without a clear drawing):

But yeah there are probably some approximate settings which are "pretty good" for a reasonable range of surrounding font, label font, and line spacing, especially if we let the user tweak that heuristic a bit.

It doesn't have to adapt to different line heights. The only requirement is that the label box is smaller than the surrounding text, such that we get the desired label effect (or progress bar, smaller in height). Scaling is not a requirement. Unfortunately I wasn't clear about that in the beginning.

I think compared to other labels, for the progress bar a fixed height seems much cleaner. But since I don't vary line height I get that "for free", so totally OK with skipping the measuring of fonts, and just using a "pretty good" heuristic for height (with some user tweak ability). So basically close to where we started (except I now have a much better draw algorithm for the progress bar itself).

What do you suggest on how to proceed?

How about I try to rip out all the text-scale and font-measurement, and we fall back on a single "one :box heuristic for all labels everywhere" method to see how we like it, maybe we do some tuning on that heuristic for various setups and see what knobs can be introduced. In that way we'd avoid measuring faces, the extra buffer, the face-remapping on text-scale, etc.

Another line of development could target Emacs directly. I think Emacs needs a facility to specify vertical text centering for line-spacing > 0. This would solve a lot of the issues we are facing right now.

Yes. Fractional box line-width values would be a nice simple addition and save all the "recalculating :box". And a 4 way box (left right top bottom) with fractional or pixel values would be ideal. It's very weird to me that underline and box treat line-spacing like it's just "really tall font descent". I don't think anybody would want that, unless they demand that their boxed lines adjoin.

A simple alternative proposal: https://github.com/minad/org-modern/commit/6c192145b16cfcfbdb1eb923325d03e8fefd805d What do you think?

I thought about fractional box chars (they come in eights) and was even playing with them this morning (with overfine and underline is nice). But many fonts including mine don't include all the fractional sizes sadly. And this shade character is also missing:

I also really prefer the stats to be inside the bar, just seems far more natural and "modern".

[minad]

Just for centering. Diagram of the whole gory details (don't judge, it was too painful without a clear drawing):

Yes, but then I want to see it demonstrated for TODO and tags. Then I can see if I find the additional complexity justified.

But yeah there are probably some approximate settings which are "pretty good" for a reasonable range of surrounding font, label font, and line spacing, especially if we let the user tweak that heuristic a bit.

Exactly.

How about I try to rip out all the text-scale and font-measurement, and we fall back on a single "one :box heuristic for all labels everywhere" method to see how we like it, maybe we do some tuning on that heuristic for various setups and see what knobs can be introduced. In that way we'd avoid measuring faces, the extra buffer, the face-remapping on text-scale, etc.

Sounds okay. But again - I want to see how it works for TODO and tags. Then I can compare side by side.

Yes. Fractional box line-width values would be a nice simple addition and save all the "recalculating :box". And a 4 way box (left right top bottom) with fractional or pixel values would be ideal.

Yes. Also rounded corners if we are writing a wish list ;)

I thought about fractional box chars (they come in eights) and was even playing with them this morning (with overfine and underline is nice). But many fonts including mine don't include all the fractional sizes sadly. And this shade character is also missing:

Take a look at this one: https://github.com/minad/org-modern/commit/b777fce2b7aec3136e3aa29e7758aa4f69ef60f9

I only use the full block character now. I must say, your font is really incomplete. The shade character is a standard character used for box drawing.

I may agree with you that a full height progress bar looks more modern. However if we compare it with your first proposal, there we also had a border around the progress bar. In any case, what I want to say - if we can get a half decent result with a minimal amount of complexity, we should prefer that. I'd find it okay to accept more added complexity, if the result reaches perfection. But this is not the case with your code, which still becomes misaligned for larger values for the line spacing. So this is clearly a case of diminishing returns. We are throwing a lot of work at the problem but don't get significantly better in contrast to simpler heuristics.

[jdtsmith]

It is a classic font, but just haven't found one easier on the eyes. You can use that font sight to see whether a character is widely supported.

Take a look at this one: https://github.com/minad/org-modern/commit/b777fce2b7aec3136e3aa29e7758aa4f69ef60f9

That works, though still the stats are "to the side". And of course it has the 1-char granularity problem you were concerned about above. There are full/quarter/eighth blocks designed especially for this, btw (but support for those is also not uniform: the perils of relying on unicode).

But again - I want to see how it works for TODO and tags. Then I can compare side by side.

We've miscommunicated before, so I need to make sure I understand you. Do you mean continue the current approach, with the full vertical current alignment in place for all 6 "box-label" types mentioned in org-modern--label-types? That will involve some re-tooling, such as the wide-box issue I mentioned above somewhere. And obviously if the progress bar is already not "perfect enough" it wouldn't be worth doing this.

I'd find it okay to accept more added complexity, if the result reaches perfection. But this is not the case with your code, which still becomes misaligned for larger values for the line spacing.

By misaligned you mean the entire label shifts below the baseline? It's perfectly aligned with the center of the line (as you can see with highlight-line-mode), it's just that Emacs has a strange idea of how to arrange that line. We should decide on what is a reasonable range of line-spacing to accommodate. I never use that. I used 50% to show an extreme value; I doubt anyone uses that much in practice. And even with that, if you make your bar smaller, it will be closer to the baseline.

One further idea which could help with line-spacing: if you are willing to accommodate additional space above the line, we could do the following:

• leave the stats text [x/y] entirely unraised, sitting happily on the baseline.
• to center the inverse box on them, use raise on a blank character, to pad space on the top of the line (to offset the line spacing). This could achieve pixel perfect baseline text alignment, at the cost of increasing the line height on affected lines.

But I also think a tunable heuristic might be good enough for most people. I propose we define some fixed cases that need to look reasonable:

1. Constant height lines (easy, done).
2. Line-spacing up to values XXX.
3. Varying line heights (e.g. modus style) by YYY% (25?).
4. Line spacing and varying headline height.

[minad]

By misaligned you mean the entire label shifts below the baseline? It's perfectly aligned with the center of the line (as you can see with highlight-line-mode), it's just that Emacs has a strange idea of how to arrange that line.

Yes, this is what I mean. In the end, Emacs line alignment itself should be improved.

We should decide on what is a reasonable range of line-spacing to accommodate. I never use that. I used 50% to show an extreme value; I doubt anyone uses that much in practice. And even with that, if you make your bar smaller, it will be closer to the baseline.

Yes, right now, org-modern also works for only a small value of line-spacing. This limitation can stay.

This could achieve pixel perfect baseline text alignment, at the cost of increasing the line height on affected lines.

Changing the line height does not seem like an acceptable option to me.

But I also think a tunable heuristic might be good enough for most people. I propose we define some fixed cases that need to look reasonable:

Agree with those points. This is also the case right now.

We've miscommunicated before, so I need to make sure I understand you. Do you mean continue the current approach, with the full vertical current alignment in place for all 6 "box-label" types mentioned in org-modern--label-types? That will involve some re-tooling, such as the wide-box issue I mentioned above somewhere. And obviously if the progress bar is already not "perfect enough" it wouldn't be worth doing this.

The main source of complexity in your patch is due to the fixed-height label formatting/alignment, right? My point is that if we are going to add such an improved label formatting, it should apply to all labels consistently. Furthermore the complexity should stay in check and the addition should somehow pull its weight. The best way to evaluate the formatting, is to check TODO or tag labels side by side (the new vs the old code). As far as I understood, your improved label formatting is a necessary prerequisite for the fancy progress bar formatting?

[jdtsmith]

As far as I understood, your improved label formatting is a necessary prerequisite for the fancy progress bar formatting?

No, the specified space method will work fine with a single :box heuristic. I.e. it can easily be used with varying height, similar to the current labels. Some people might even like "big important heading" = "bigger progress bar".

Note that if we could do :box (:line-width (0 . -0.2)) or so the heuristic could be much better. I agree the added complexity of precise center alignment may not be worth the gains. Tonight I'll try stripping all the complexity out and see where we stand.

[jdtsmith]

OK I ripped out all the fancy font measuring/remapping/text-scaling stuff — quite a bit of stuff. I basically just fell back onto your old heuristic with slightly changed :box widths. It seems pretty OK, if I altered the label-border to suit (as the user would do). All modus vivendi.

All headlines same height, no line-spacing, label-border=auto

All headlines same height, 0.25 line space, label-border=1

Headline height varies (1.5x max), no line-spacing, label-border=auto

Headline height varies (1.5x max), 0.25 line space, label-border=3

[jdtsmith]

The only thing that might be a small improvement is custom box per type. I.e. instead of setting :box on org-modern-label using the heuristic, set it on each of the 6 "main type faces" (progress-bar, tag, todo, priority, timestamp-inactive, timestamp-active), after checking their :height. That way you could have different sizes for the elements and box would be tunable. Then make org-modern-label-border (optionally) an alist keyed by those types.

[minad]

Thanks! I spent the last few hours experimenting with your code.

One thing bothers me about the display - the progress bar always breaks at the numbers, and this prevents the numbers from being centered. Based on your code I made this experiment: https://github.com/minad/org-modern/tree/simple-progress-2

I am almost happy with the display. However my experiment has another unfortunate consequence - it prevents the numbers from being editable.

I also tried to apply on parts of the bar to parts of the underlying text, to reenable editing. However then we get white thin lines between the bars due to the box :line-width. Note that in your PR, you modified the :line-width to (cons 0 v), which is not supported anymore by Emacs 29. I don't agree with this Emacs upstream change, since there is no technical reason to disallow a line width of zero. But that's what it is.

Any better ideas? It seems we either lose the editability or we compromise on the formatting.