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

Discarded glues at the top of a frame shall not account in glue adjustement #1752

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

Omikhleia
Copy link
Member

Closes #1647

It turned out to be a bit trickier than I thought and needs scrutiny...

Regarding impacted tests (besides a dedicated test for that case)

  • bibtex.sil (using parskip) was actually revealing the same issue.

    image

    The extra space is no longer showing and the page is maximally stretched.

  • bug-252 is impacted on page 3, the small space below now disappears. Seems better.

    image

  • bug-301 is much harder to judge. It doesn't seem worse, but it's hard to tell.

Looking closely, this test expection had an extra parskip at the
bottom of the first page, exactly the same symptom as sile-typesetter#1647.
It's harder to say how it relates to sile-typesetter#1647, but checking with
-d all, this test showed a small vertical space at the bottom of
page 3, meaning the content frame was not stretched as much as
expected. It seems to be some lineskip. Now the text seems fully
stretched within the content frame.
The changes from sile-typesetter#1647 impact this test on page 3 (last line is
slightly lower), this doesn't seem worse though the test is hard
to check.
@alerque
Copy link
Member

alerque commented Mar 29, 2023

I tried this on one of my book project where I was having some troubles and it seems to be a solid improvement. I don't like the way settings and variables are scoped, but at least this code makes it explicit what is being tracked where and what subsystems need to interface with others where. I think this will make a settings rescope involving typesetters and frames easier in the future.

Is there any reason we can't bump this up to v0.14.9?

I'll look into the bug-301 test now too.

@alerque
Copy link
Member

alerque commented Mar 29, 2023

301 has always been a hard bug to replicate. It looks like the change in it is for the better in line with the other affected tests you described. It is still showing something that looks like the same bug, but it actually is different. The unfilled frames are the way they are because of the non-stretchable skips with specific metrics used to trigger the bug.

@Omikhleia Omikhleia added this to the v0.14.9 milestone Mar 29, 2023
@Omikhleia
Copy link
Member Author

Is there any reason we can't bump this up to v0.14.9?

Nope, just added it. It was 2AM or so when I completed it, I must have been too tired to notice I had the milestone on the issue but not on the related PR ^^

@Omikhleia Omikhleia added the bug Software bug issue label Mar 29, 2023
@alerque alerque merged commit dca0a15 into sile-typesetter:master Mar 29, 2023
@Omikhleia Omikhleia deleted the 1647-top-glue-issue branch April 10, 2023 11:10
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Software bug issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

\par creates space in the wrong location
2 participants