Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance parallel package with overflow frame alignment, footnote handling and improved documentation #2216

Draft
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

no-vici
Copy link

@no-vici no-vici commented Jan 15, 2025

Summary

This pull request introduces significant enhancements to the parallel package, including improved footnote handling, reasonable alignment logic, and expanded documentation.

Key Features and Changes

  1. Footnote Handling:

    • Added parallel_footnote and related commands for managing footnotes in parallel frames.
    • Implemented generateFootnoteId and getFootnoteHeight functions to uniquely identify and measure footnotes, supporting efficient layout and splitting.
    • Introduced the typesetFootnotes function to process footnotes, handle overflow, and manage their layout dynamically across frames.
  2. Frame Alignment:

    • Improved sync command to include parskip handling and modularized balancing logic.
    • Enhanced parallelPagebreak function to handle overflow content, dynamic frame balancing, and proper synchronization of frames and footnotes.
  3. Documentation:

    • Expanded the package.documentation section with detailed explanations, usage notes, and examples.
    • Highlighted challenges and best practices for maintaining alignment, managing footnotes, and working with SILE’s parallel typesetting capabilities.

Why This Change is Important

These updates improve the usability and flexibility of the parallel package by addressing key limitations and providing tools for managing complex layouts. The enhanced documentation also helps users understand and utilize the package more effectively with what SILE can offer.

Testing and Compatibility

  • The updates have been tested with standard SILE setups using the Gentium Plus font family at a 12pt font size.
  • Known limitations include unsupported custom document.parskip, document.lineskip and potential alignment challenges with varying font sizes or custom spacing settings.

Next Steps

  • Additional testing with more complex layouts and document classes.
  • Gathering user feedback to refine and extend the package’s functionality.

no-vici and others added 24 commits January 15, 2025 10:35
Added two parallel frames specifically designed for typesetting footnotes. These frames are positioned below the main parallel frames to improve layout organisation and ensure footnotes are clearly separated from the main content.
Registered two additional frames using the parallel package for accommodate footnotes. These frames are set up to align with the main parallel frames and maintain a consistent layout for footnotes (fixed height).
@no-vici no-vici requested a review from alerque as a code owner January 15, 2025 15:40
@Omikhleia Omikhleia marked this pull request as draft January 18, 2025 09:49

-- If lineHeight could not be calculated, fall back to baselineSkip and lineSkip of the document
if not lineHeight then
local baselineSkip = SILE.settings:get("document.baselineskip").height or SILE.types.length({ length = 0 })
Copy link
Member

@Omikhleia Omikhleia Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SILE.types.length({ length = 0 }) -> you can just write SILE.types.length() (zero by default)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your tip. I will use that in the future. I can't find a manual for SILE's internals, and find it hard to get things right. I should come back to farming rather than diving into this crazy field ;)

-- If lineHeight could not be calculated, fall back to baselineSkip and lineSkip of the document
if not lineHeight then
local baselineSkip = SILE.settings:get("document.baselineskip").height or SILE.types.length({ length = 0 })
local lineSkip = SILE.settings:get("document.lineskip").height or SILE.types.length({ length = 0 })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that depending on the linespacing algorithm (the default one is more or less the TeX one), the lineskip is used conditionally (when boxes are greater than the baselineskip and would collide vertically), not always. In most cases (i.e. regular text with decent settings for the baselineskip), the lineskip isn't applied.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the default one doesn't work as expected and eliminating the lineskip makes the frame alignment even worse. With the same thought, I eliminated it in the calculation of the fallback, it messes the alignment up too. Here is a reasonable visual example with lineskip accounted for. Without lineskip, the outcome would look like this:

image

SILE.settings:set("font.size", SILE.settings:get("font.size") * 0.80)

-- Measure the marker width
local markerWidth = measureStringWidth(markerText)
Copy link
Member

@Omikhleia Omikhleia Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are computing the width by shaping the marker text. This will be shaped again below when you typeset it (SILE.typesetter:typeset(markerText)). I'd suggest rather using a box: you'd get the width and can just re-use it as-is afterwards, without re-computing costs under the hood.

local markerBox = SILE:typesetter:makeHbox({ markerText })
local markerWidth = markerBox.width
...
SILE.typesetter:pushHbox(markerBox) -- don't re-typeset, just push our box

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd seen it in some of your books and wanted to ask you to the right approach though. Looked into your resilent.footnotes but did not find it, so just did a dirty hack for a quick try.

Thank you so much for pointing to the right direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked into your resilent.footnotes but did not find it,

Hehe.... The logic is entirely abstracted and buried deep into the style:apply:number logic (in the resilient.styles package), 'cause every styling decision is customizable in re·sil·ient, and the footnote package doesn't have to care, it just applies a style where needed.


-- Set hanging indentation
local hangIndent = SILE.types.length("14.4pt"):absolute()
SILE.settings:set("current.hangAfter", 1) -- Indent subsequent lines
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mere remark: note that hangAfter/hangIndent is touchy (on when it resets etc.) and overkill here.
For a 1-line indent, it can most of the time be avoided:

  • set the whole left skip to your amount (document.lskip)
  • set the current paragraph indent to the negative of that amount ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't agree more. You are absolutely right, that computation was an overkill.

SILE.settings:set("current.hangIndent", hangIndent)

-- Calculate the gap after the marker
local markerGap = hangIndent - markerWidth
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tricky: what to do if the markerGap is negative here (i.e. footnotes reach such a number that it's larger that your note indent (if I got it right, the hard-coded 14.4pt above... Why this peculiar value by the way?)... Or even close to 0?
This is something I addressed in resilient.footnotes, basically the idea is the following (simplified)

  • decentSpacing is some space >= interword space (usual greater by a small amount)
  • If markerGap is > decentSpacing then do you usual stuff:
    • push marker,
    • add kern,
    • process content
  • otherwise, we are too close or past the available size:
    • push marker,
    • push glue with size interwordSpace
    • process content

BTW interwordSpace = SILE.shaper:measureSpace(SILE.font.loadDefaults({})) (respecting other adequate settings, and it has stretch and shrink)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B. Unless I misunderstood what you were trying here, regarding this type of footnotes, and my above discussion, I didn't invent it. It's (one of) Jan Tschichold's recommendation(s).
As long as the marker fits, it's in the available space:
image
Once the marker no longer fits, the first line shall flow as a regular line:
image

Tschichold provides other indications for different ways of formatting things (footnotes, etc.) and he is often quite opinionated on many topics. But what to expect from a great typographer, designer and writer? ;)
Probably the best things I've read while working on my modules. Robert Bringhurst comes a bit after, his book is dull or imprecise at times, though still commendable ;) -- it doesn't say anything on that matter, anyhow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out that makerGap could be negative after a certain state. 14.4pt is equivalent to 1.2em in the context of the document.fontsize (12pt, Gentinnum Plus) scaled down to 80%. SILE always converts my hangIndent to pt.

Bellow could be a safer approach:

   self:registerCommand("parallel_footnote:constructor", function(options, content)
      local markerText = options.marker or "?" -- Default marker if none provided
      local hangIndent = SILE.types.length("1.5em"):absolute()

      SILE.settings:temporarily(function()
         -- Set font footnotes, 80% of current font size
         SILE.settings:set("font.size", SILE.settings:get("font.size") * 0.80)

         -- Put markerText in a hbox
         local markerBox = SILE.typesetter:makeHbox({ markerText })

         -- Measure the marker width
         local markerWidth = markerBox.width

         -- Set left skip for the whole paragraph
         SILE.settings:set("document.lskip", hangIndent)
         -- Set a negative indent for the first line
         SILE.settings:set("document.parindent", -hangIndent)

         -- Calculate the gap after the marker
         local markerGap = hangIndent - markerWidth

         if markerGap > SILE.types.length("2.5pt"):absolute() then
            -- Push the markerBox
            SILE.typesetter:pushHbox(markerBox)
            -- Add spacing after the marker for alignment
            SILE.call("kern", { width = markerGap })
         else
            local newLeftSkip = hangIndent + 2 * math.abs(markerGap)
            -- Set the left skip to a new value
            SILE.settings:set("document.lskip", newLeftSkip)
            -- Set a negative indent for the first line
            SILE.settings:set("document.parindent", -newLeftSkip)

            -- Push the markerBox
            SILE.typesetter:pushHbox(markerBox)
            -- Recalculate the gap after the marker
            markerGap = newLeftSkip - markerWidth
            -- Add new spacing after the marker for alignment
            SILE.call("kern", { width = markerGap })
         end

         -- Process the footnote content
         SILE.process(content)

         -- End the paragraph
         SILE.call("par")
      end)
   end)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: ... SILE.types.length("2.5pt"):absolute()

Absolutizing converts any relative unit (e.g. em) to points. Not needed here, thus, you are already in pt in your length (but why an hard-coded 2.5, one wonders).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re:

SILE always converts my hangIndent to pt.

'Cause it's being "absolutized" (see previous comment) at some point. (The question of SILE absolutizing things too soon or too late is for another day...).

SILE.types.length("1em"):absolute() is the current font size in pt. Knowing this, you shouldn't need an hard-coded 14.4 if it's supposed to be depended on the current font size. Either use the font.size (always converted to points as a number), or express things in 1em and absolutize() them to points.

-- Handle parskip (spacing between successive frames).
local parskip = SILE.settings:get("document.parskip")

if not parskip.length then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does it occur? document.parskip is always defined no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!
It occurs when the document.parskip is not set by commenting out: \set[parameter=document.parskip,value=0pt]
I may be wrong there, but somehow it works for me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be wrong there, but somehow it works for me.

Hm... not parsikip is still false when parskip is set to 0pt. It's always false ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be wrong there, but somehow it works for me.

Hm... not parsikip is still false when parskip is set to 0pt. It's always false ;)

I find this strange too. Knowing that only nil or false is evaluated as false, but setting document.parskip to 0pt still works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turned out that parskip.length is always nil regardless of whatever the document.parskip is set to.

Quick fix:

      -- Retrieve the parskip setting
      local parskip = SILE.settings:get("document.parskip")

      -- Check if parskip is effectively zero or unset
      if not parskip or parskip.height:tonumber() == 0 then
      
         -- Insert flexible glue if parskip is nil or zero
         addParskipToFrames(SILE.types.length("1em"):absolute())
         
      else
         -- Use the user-defined parskip value
         addParskipToFrames(parskip)
      end

SILE.settings:temporarily(function()
local currentSize = SILE.settings:get("font.size")
SILE.settings:set("font.size", currentSize * 0.75) -- Scale down to 75%
SILE.settings:set("font.weight", 800)
Copy link
Member

@Omikhleia Omikhleia Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "0.75em" is what you probably want without needing to perform the computation yourself (EDIT = calling the font command then. Hmm. Might not be that simpler, just higher-level...)
  • Weight 800 is very heavy...
  • Of course textsubsuper.sile would allow not bothering here. But your are in the core distribution. Bah...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My "Bah!" above = never mind. People like reinventing the wheel ever and ever ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "0.75em" is what you probably want without needing to perform the computation yourself (EDIT = calling the font command then. Hmm. Might not be that simpler, just higher-level...)

Oh, that works. Thank you!

  • Weight 800 is very heavy...

I'm an old man, so a bold text would allow me to spot it more easily ;) Will adjust it afterward.

  • Of course textsubsuper.sile would allow not bothering here. But your are in the core distribution. Bah...

Honestly, I don't what you meant here.
Many people did not know that the wheel they were trying to invent already existed ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm an old man, so a bold text would allow me to spot it more easily ;) Will adjust it afterward.

textsubsuper.sile adopts weight 600 (medium) for fake footnotes (raised+scaled text), following Bringhurst in that matter (".. smaller numbers in semibold ...")

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

textsubsuper.sile adopts weight 600 (medium) for fake footnotes (raised+scaled text), following Bringhurst in that matter (".. smaller numbers in semibold ...")

I have set it to *600. I don't know how to typeset superscript, so I went with raiser and smaller. I guess, SILE must supports that straight out of the box.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, SILE must supports that straight out of the box.

There's a 3rd-party package for that, which does have a bunch of options:

  • using natural superscripts (some fonts have very nice ones, e.g. Brill)
  • using fake superscripts (raised, smaller) = more or less like you do here, but with extra capabilities (of course fiddling with the weight, but also performing some re-scaling)

I am not sure whether bringing it to the core ("out of the box") distribution is the right time... I am not opposed to it -- I don't care, actually -- but there was sort of a meta-discussion long ago on what was expected in the core distribution, and the conclusions are dubious at best.... This being said, I do tend to think that a diglot class and a parallel package would be best addressed outside of the core distribution. But heh, it's not my call. 🐰

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether bringing it to the core ("out of the box") distribution is the right time... I am not opposed to it -- I don't care, actually -- but there was sort of a meta-discussion long ago on what was expected in the core distribution, and the conclusions are dubious at best.... This being said, I do tend to think that a diglot class and a parallel package would be best addressed outside of the core distribution. But heh, it's not my call. 🐰

I meant superscript text might have been supported by SILE's core, so users don't have to reinvent the wheel like I did ;).

Bringing the parallel package into the core is not a wise idea, and the demand for it is not worth the effort of doing so.

SU.warn("Offset is larger than the number of lines available; no dummy content will be generated.")
return
end

-- Fill the frame with dummy content using white-colored text to avoid visible output.
-- Add dummy content to fill the frame
SILE.call("color", { color = "white" }, function()
Copy link
Member

@Omikhleia Omikhleia Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand what you are trying with this dummy content, but "white-colored" text is a dubious solution.

  • If you really want N blank lines, one way would be to use zerohbox nodes instead of some dummy text (i.e. IIRC, SILE.typesetter:pushHorizontal(SILE.types.node.zerohbox()). It should work, normally... But if failing report a bug and in the meantime, typeset a non-breaking space in that case (luautf8.char(0x00A0) or one of his friends). Still a dummy text/space, but no need for coloring it ;)
  • Theoretically, the better way would likely be to push a SILE.types.node.vkern() with the appropriate height, directly to the output queue. These nodes exists in the code base, but don't have a decent API and are only used in a fairly bad package (linespacing). I had some notes about them and vglues in general (there's more that meets the eyes here)... I'll look for them and open an issue... Another option in the meantime might, if the vkern does not work well, woulf be to hack a vglue so that its "discardable" and "explicit" properties are both false. That what a vkern should be, with some extra considerations...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying for fun without much knowledge about how SILE handle things behind the scene. Dummy white coloured text was the only solution I could think of, but the dirty way works quite ok. I knew it would not be a proper way to handle overflow.

image

I did not know that SILE.types.node.vkern() can do the job reasonably with some manual adjustment. I will do some test with it and the zerobox.

Very grateful for these tricks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Theoretically, the better way would likely be to push a SILE.types.node.vkern() with the appropriate height, directly to the output queue. These nodes exists in the code base, but don't have a decent API and are only used in a fairly bad package (linespacing). I had some notes about them and vglues in general (there's more that meets the eyes here)... I'll look for them and open an issue... Another option in the meantime might, if the vkern does not work well, woulf be to hack a vglue so that its "discardable" and "explicit" properties are both false. That what a vkern should be, with some extra considerations...

Would you mind having a play with my Parallel Typesetting settings?

I've found that using real dummy text or hard glue works best for me. I’ve tried all of your suggestions, and they work quite well with some manual adjustments in my document, such as adding \skip or phantom text for overflow frames. However, the hard glue patch seems to require the least intervention overall.

My understanding of SILE is still a work in progress, so sometimes I don't know how or why something works when I mess around with it. You flagged it as dubious… 😉 For me, it was just a stroke of luck!

- Introduced `FootnoteManager` class to encapsulate footnote management logic.
- Modularized footnote typesetting with new methods:
  - `preprocessFrame`
  - `processSingleNote`
  - `handleOverflowingNote`
  - `finalizeFrame`
- Replaced repetitive loops with `processNotes` for footnote iteration.
- Improved frame balancing logic with better height calculation and dummy content.
- Enhanced logging and debug capabilities for easier issue tracking.
- Updated `\sync` command to ensure proper processing order of main text and footnotes.
- Added clear separation of concerns for glue and alignment logic.
- Documentation updated to reflect the refactored structure.
…Pagebreak to avoid overflow footnote content being stuck in the same frame.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants