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

Align corner brackets to the top and bottom when scaling #4200

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bluebear94
Copy link
Contributor

See #4188

This broke scaling of the corner brackets, U+231C – U+231F. Hopefully this didn’t happen to be needed? Right?

@PgBiel
Copy link
Contributor

PgBiel commented May 21, 2024

This broke scaling of the corner brackets, U+231C – U+231F.

Could you provide some images comparing the old and the new behavior? Should make it easier to tell if that's a problem or not (e.g. if the previous behavior was unintended).

@Enter-tainer
Copy link
Contributor

I think there are images in #4188 but i wonder if it's the correct place to apply "fix". And i'm also surprised to see non of other tests fail

@Enivex
Copy link
Collaborator

Enivex commented May 21, 2024

I think there are images in #4188 but i wonder if it's the correct place to apply "fix". And i'm also surprised to see non of other tests fail

Presumably because these are some of the few delimiters that aren't already vertically centered in most fonts. In fact, looking at http://mirrors.ctan.org/macros/unicodetex/latex/unicode-math/unimath-symbols.pdf they seem to be the only ones.

I don't know the motivation behind centering delimiters, but I guess the safest option would be to only disable centering for these specific ones @bluebear94 ?

@Enivex
Copy link
Collaborator

Enivex commented May 21, 2024

#1756

This was done purposely, so the issue here is that these specific corner delimiters have to be handled in a different way.

Perhaps check how LuaTeX/XeTeX deals with this? The same issue presumably appears there.

@bluebear94
Copy link
Contributor Author

Both LuaTeX and XeTeX have incorrect results with \left and \right (though less so with LuaTeX):

\usepackage{unicode-math}

\[\ulcorner c \urcorner \ulcorner \frac{a}{b} \urcorner\]

\[\left\ulcorner c \right\urcorner \left\ulcorner \frac{a}{b} \right\urcorner\]

gives the following in LuaLaTeX:

image

and the following in XeLaTeX:

image

@Enivex
Copy link
Collaborator

Enivex commented May 22, 2024

Okay, so I guess we'll have to come up with a solution ourselves.

LuaTeX at least doesn't seem to center it even with \left and \right, even if they're not scaled at all @bluebear94. Unlike XeTeX, which seems to suffer from the same problem as typst.

@Enivex
Copy link
Collaborator

Enivex commented May 22, 2024

Can these specific delimiters be top and bottom aligned?

@bluebear94 bluebear94 changed the title Don’t center delimiters on axis when scaling Align corner brackets to the top and bottom when scaling May 22, 2024
This broke scaling of the corner brackets, U+231C – U+231F.
Hopefully this didn’t happen to be needed? Right?
@@ -62,3 +62,7 @@ $ 1/(2 y (x) (2(3)) $
// Test ignoring weak spacing immediately after the opening
// and immediately before the closing.
$ [#h(1em, weak: true)A(dif x, f(x) dif x)sum#h(1em, weak: true)] $

--- math-lr-corner-brackets ---
// Test positioning of U+231C to U+231F
Copy link
Member

Choose a reason for hiding this comment

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

could you add a comment here that they are supposed to look the same? the "vs" had me briefly thinking that they are supposed to look different.

could also rename it to issue-4188-lr-corner-brackets to link it to that issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve changed the vs to an =.

crates/typst/src/math/fragment.rs Outdated Show resolved Hide resolved
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.

None yet

5 participants