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

Vertically centered table rows are not actually vertically centered #1964

Closed
rsynnest opened this issue Sep 5, 2021 · 6 comments
Closed
Milestone

Comments

@rsynnest
Copy link

rsynnest commented Sep 5, 2021

Using the default theme, the following Asciidoc generates the table seen in the screenshot below. Vertical-alignment seems to be off? I'm not a Ruby guy and can't figure this codebase out enough to fix, but I think this might be a good starting point? First added along with tests in this commit.

[cols=">.<,^.^,<.>",grid=all,frame=ends]  
|=========                                
| *Top*                                   
| *Center*                                
| *Bottom*                                
|=========                                

table

@mojavelinux
Copy link
Member

Getting text to vertically align center in Prawn is nearly impossible. That's because every font has a built-in line height, and how that information is reported varies from font to font. If we get it working for one font, then it's off for a different font. I have spent many, many hours trying to make it work consistently, and I just can't do it. So I regret to information you that this is just the way it is. It will never be perfect.

@mojavelinux
Copy link
Member

mojavelinux commented Sep 5, 2021

To clarify, I have already fixed the middle alignment in the main branch (see 285f857). However, I couldn't fix the bottom alignment, which ends up matching the middle alignment. But it's at least better than the screenshot you posted.

Unless someone can show me a better way to do it, I'm closing this issue as I have no plans to work on it any further.

@mojavelinux mojavelinux added this to the support milestone Sep 5, 2021
@rsynnest
Copy link
Author

rsynnest commented Sep 5, 2021

Thanks for the quick response! I don't know a lot about font rendering, but I took a stab at a fix in prawn-table, just by re-using their cheeky move-before-rendering alignment code. Here's the PR. I move up for Top, don't move at all for Center, and move down for Bottom. Do you see this breaking things for other fonts, given I'm using the same variables (font.line_gap and font_descender) for movement? If I knew what those variables are exactly I could maybe fine tune it.

@mojavelinux
Copy link
Member

That may work for the built-in fonts, but it's the TTF fonts where things get tricky (like NotoSerif). They are the ones that have a built-in line height. That's further complicated by the fact that Asciidoctor PDF itself supports a line height that has to be taken into consideration. The fix in Prawn itself will not be enough to fix the alignment universally in Asciidoctor PDF. You are welcome to try to make the fix in Asciidoctor PDF, and I will review it. But I won't be investing any time in it myself.

@rsynnest
Copy link
Author

rsynnest commented Sep 6, 2021

Thanks again for the time. I ran a test on 310 Windows TTF fonts, and my change seems to fix most TTF fonts. In cases where it doesn't help, things aren't any worse than the existing behavior, they just don't get much better. Hope to see this merged upstream so users can at least get some expected behavior for majority of cases, vs current behavior of being broken everywhere for even the builtin fonts.

If the Prawn fix doesn't help Asciidoctor, can I suggest adding a note in the PDF theming guide that vertical-alignment is broken? I spent a fair amount of time digging into this only to find it's a known issue but still being presented to users as a working feature.

@mojavelinux
Copy link
Member

You're welcome to submit a docs or code fix to Asciidoctor PDF, and I will review. As I said, I'm not going to initiate the effort to address this issue because I am already overstretched. But I will take the time to review an effort that is being driven by a community member.

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

No branches or pull requests

2 participants