positron-solutions / dslide

Present anything Emacs can do with programmable, extensible, configurable slides made from org mode headings
GNU General Public License v3.0
92 stars 2 forks source link

Inconsistent size of title header #3

Closed terlar closed 3 months ago

terlar commented 3 months ago

The size of the title header keeps changing depending on which subheading is displayed (I'm using relative font sizes, so perhaps that is why). Kooha-2024-06-24-15-53-30

psionic-k commented 3 months ago

Not your main issue, but you will want to customize dslide-breadcrumb-face, which by default just inherits org-level-8. Maybe you don't have an org-level-8 defined.

The issue still shouldn't happen to begin with, but let me know how that goes.

If it doesn't work immediately, you can look inside dslide--overlays there is :before-string property which will show you the state of the header, with all of the text relevant properties.

terlar commented 3 months ago

I tried customizing dslide-breadcrumb-face and it didn't seem to make any changes. I saw that it inherited org-level-8 by default and I do have that defined (:height 1.1 :weight medium :slant italic).

I noticed dslide-breadcrumb-face not showing up in the faces, I guess because it is not defined via defface. Also it's type face complains when you try to set it to the default value, as (:inherit default) is not a type face. I guess it is meant to refer to other faces currently.

Every time I inspect dslide--overlays it is nil. Not sure what is going on. I guess it has something to do with my relative sizes for my org-level-* headers that somehow interferes?

But hard to debug, since I am not sure how to investigate what faces apply.

terlar commented 3 months ago

I tried to get any effect and it seems that dslide-breadcrumb-face is only applied to the breadcrumbs (of course). But the issue is with the whole header. Seems they inherit face options depending on what header is displayed. Because size keeps changing and I also noticed for smaller headers that the slanting is changing as well and that is for the whole header.

image

psionic-k commented 3 months ago

I meant dslide--header-overlay to see the :before-string, via overlay-properties

(overlay-properties dslide--header-overlay)
;; (before-string
;;  #("
;; Domain Specific sLIDEs
;; 2024-06-26  Positron  <contact@positron.solutions>  
;; Start 🢒 Controls
;; " 0 1
;;    (line-height 0.5 face org-document-info)
;;    1 23
;;    (face org-document-title)
;;    23 24
;;    (line-height 0.5 face org-document-info)
;;    24 77
;;    (face org-document-info)
;;    77 82
;;    (wrap-prefix "  " face
;;     ((:inherit org-level-8)
;;      org-level-1)
;;     fontified t)
;;    82 85
;;    (wrap-prefix "  " face
;;     ((:inherit org-level-8)
;;      org-level-1)
;;     fontified t)
;;    85 93
;;    (wrap-prefix "   " face
;;     ((:inherit org-level-8)
;;      org-level-2)
;;     jinx--pending t fontified t)
;;    93 94
;;    (line-height 1.5 face org-document-info)))

You can see the face property of the last breadcrumb, from 85 to 93, is a list, ((:inherit org-level-8) org-level-2). Properties of org-level-8 will shadow org-level-2.

I'm pretty sure I saw this during development, and the issue was fixed by setting the breadcrumb face, but I have org level 8 defined in my theme:

(org-level-8 :inherit 'variable-pitch :foreground violet :weight 'bold :height 1.00)

Before I set this, I saw something similar as you. Now I want to make it work out of the box for everyone. I'll look into this more tomorrow. I definitely want a better solution.

psionic-k commented 3 months ago

it is not defined via defface

Correct, you can use either a face name or just a face property list. I think it can be a defface, but the key was to ensure it shadows the height of the other faces if you want consistent breadcrumb height. This shouldn't affect the title at all. That's my curiosity.

terlar commented 3 months ago

I have basically the same overlay as you:

(overlay-properties dslide--header-overlay)
(before-string #("
Domain Specific sLIDEs
2024-07-02  Positron  <contact@positron.solutions>  
Start 🢒 Controls
"
         0 1 (line-height 0.5 face org-document-info) 1 23 (face org-document-title) 23 24
         (line-height 0.5 face org-document-info) 24 77 (face org-document-info) 77 82
         (wrap-prefix #("  " 0 2 (face org-level-1)) face ((:inherit org-level-8) org-level-1)
                  fontified t)
         82 85
         (wrap-prefix #("  " 0 2 (face org-level-1)) face ((:inherit org-level-8) org-level-1)
                  fontified t)
         85 93
         (wrap-prefix #("   " 0 3 (face org-level-2)) face ((:inherit org-level-8) org-level-2)
                  fontified t)
         93 94 (line-height 1.5 face org-document-info)))

I notice that I have 0 2 (face org-level-1)) which you didn't have inside the wrap-prefixes.

I do have all faces defined, also use relative font sizes for org-document-title, org-level-1, org-level-2, org-level-8, etc.

org-document-info  ()
org-document-title (:height 1.8 :weight medium)
org-level-1        (:height 1.6 :weight medium)
org-level-2        (:height 1.4 :weight medium)
org-level-8        (:height 1.1 :weight medium :slant italic)

As mentioned changing what is inside dslide-breadcrum-face doesn't change anything really, at least on the scale/size issue.

terlar commented 3 months ago

The size/scale seems to go down with org-level, so seems it is inherited for the whole thing somehow? Also I can see it start slanting when reaching some levels, while the top level org-levels don't have this.

terlar commented 3 months ago

The dslide-breadcum-face only applies to the breadcrumb itself, how to style the rest of the header?

Setting it as red shows it clearly: Kooha-2024-07-02-00-40-17

psionic-k commented 3 months ago

so seems it is inherited for the whole thing somehow

I concur. Something about the current header is affecting the :before-string that holds the actual header contents.

https://github.com/positron-solutions/dslide/blob/master/dslide.el#L2230-L2278

This might be an Emacs display bug. I'll try to fix / workaround and also mention this in the mailing list. What's your Emacs version?

Thanks for pulling your :before-string contents. I think I can fix this once the coffee kicks in.

terlar commented 3 months ago

I am using Emacs from main branch, specifically this commit (8198a144376cfea3490ea5628392fb3a49fec2d6)

If I can assist in any other way, let me know. I guess it could be a bug after all, because it is very confusing.

skissue commented 3 months ago

I ran into this issue as well, so I decided to do a little digging.

I am also using relative sizes for my header and title faces (org-level-1, org-document-title, etc.) and saw the same issue. However, when setting these faces to an absolute size (e.g. :height 140), the issue went away, which lead me to the same conclusion that the current slide's header's face was being inherited.

I'm not sure if this is correct, but my hunch is that overlays inherit the face properties of the text "underneath" them. Since the first text in the buffer is the current headline, that means that the overlay text inherits the properties of the current headline's face. Thus, if org-document-title has a relative size (such as :height 1.5), the height would be calculated relative to the current headline's face, which would change from slide to slide, resulting in changes to the title size as well. This seems like a reasonable assumption, since I changed dslide--make-header to not apply any fontification, and the title retained the appearance of the active headline.

I was able to workaround this by changing the text in the overlay to also inherit from the default face. For dslide--make-header:

         (overlay-put
          dslide--header-overlay 'before-string
          (concat (dslide--margin-lines dslide-margin-title-above)
-                 (propertize title 'face 'org-document-title)
+                 (propertize title 'face '(org-document-title default))
                  (dslide--margin-lines dslide-margin-title-below)
                  (when (and  dslide-header-date date)

For dslide--info-face:

 (defun dslide--info-face (s)
-  (propertize s 'face 'org-document-info))
+  (propertize s 'face '(org-document-info default)))

This is all assumptions, probably needs a bit more scrutiny :sweat_smile:.

terlar commented 3 months ago

Perhaps adding a new face so users can control it. I am not sure I would like to use the same face as the org-document-title. Which in my case is quite big.

skissue commented 3 months ago

Perhaps adding a new face so users can control it. I am not sure I would like to use the same face as the org-document-title. Which in my case is quite big.

Perhaps it'd be worthwhile to mirror all the faces; e.g dslide-header-title, dslide-header-info, etc. These faces can, by default, inherit from their respective Org face as well as default, which should fix this and would allow for further user customization.

psionic-k commented 3 months ago

I agree with the mechanism. The first headline face is definitely coupling with the display of the header contents.

https://github.com/positron-solutions/dslide/commit/5d1c50be3dc903001f7839e43694ee49bac043f8

I tried using the identical :before-string. My faces didn't react, but this technique might be helpful.

(overlay-put
 dslide--header-overlay 'before-string
 #("
Domain Specific sLIDEs
2024-07-02  Positron  <contact@positron.solutions>  
Start 🢒 Controls
"
   0 1 (line-height 0.5 face org-document-info) 1 23 (face org-document-title) 23 24
   (line-height 0.5 face org-document-info) 24 77 (face org-document-info) 77 82
   (wrap-prefix #("  " 0 2 (face org-level-1)) face ((:inherit org-level-8) org-level-1)
                fontified t)
   82 85
   (wrap-prefix #("  " 0 2 (face org-level-1)) face ((:inherit org-level-8) org-level-1)
                fontified t)
   85 93
   (wrap-prefix #("   " 0 3 (face org-level-2)) face ((:inherit org-level-8) org-level-2)
                fontified t)
   93 94 (line-height 1.5 face org-document-info)))

I can't reproduce the display by just manually setting the same overlay string. This points to my org heading faces or other config settings.

I removed this hunk from the unstable branch because it was dead. It may have been alive at one point. I recall having a variation of the problem in this issue, but that I never saw it again after setting the face on breadcrumbs.

;; TODO what is this t?
(defface dslide-header-overlay-face '((t :inherit default))
  "Face for `dslide--header-overlay'.")

Let me know how the branch works out and we can go from there.

psionic-k commented 3 months ago

I added a new option to completely override the header function. This can be better for some people than figuring out all the customize variables.

Here's a minimal example:

(defun my-custom-header (cleanup &optional breadcrumbs)
  (if dslide--header-overlay
      (delete-overlay dslide--header-overlay))
  (unless cleanup
    (setq dslide--header-overlay
          (make-overlay (point-min) (1+ (point-min))))
    (overlay-put dslide--header-overlay
                 'before-string
                 (propertize "TADA!\n" 'face '(org-document-title default)))))

Fortunately, this quick test, without setting the faces, revealed that indeed the current heading face will apply to the 'before-string. It's unclear if 'default is necessary in many cases, but this could vary depending on the users other faces, so perhaps it's a good idea here. I would expect many people to trip on this issue without a safe default.

skissue commented 3 months ago

Can confirm that this seems to fix the issue for me :tada:

psionic-k commented 3 months ago

Boom. Free to reopen if not everyone has success. Thanks for bringing up the issue.