-
-
Notifications
You must be signed in to change notification settings - Fork 807
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
Support highlight, overline, underline, and strike for math equations #3953
base: main
Are you sure you want to change the base?
Conversation
crates/typst/src/math/equation.rs
Outdated
} | ||
last_frame_index = Some(index); | ||
} | ||
if let Some(index) = first_frame_index { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand what's going on here. Why are we only background-decorating the first and foreground-decorating the last MathParItem::Frame
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, wait, are you creating an overlarge decoration for all the items, but applying it to the first one? That seems problematic when the equation breaks over multiple lines. We should probably rather create individual decorations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Now each frame is decorated on its own.
Special case will be any empty space between two frames, in that case we need to fill the void and decorate the space as well. For such space in between frames, we use the last frame and append the decorations to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, interesting. I tried the following new test case, where the equation breaks over multiple lines:
--- highlight-inline-math-multiline ---
#line(length: 100%)
#h(1fr)
#highlight[$a + b + c + d + e + f + g$]
I think the trailing space after the "+" shouldn't be there. However, this is not a problem with your PR, but rather the math equation line breaking. However, if that bug was fixed, the solution of decorating the space together with the previous frame wouldn't work anymore, as the highlighting would continue into the margin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not figured out a good way of doing highlight for the trailing space. It seems that in order to do that we need to do decoration after the linebreak, which would require us to do the decoration in commit
(just like the normal text case).
In order to do this, we need to associate the style to the frame (just like how a style is asscoited with Item::Text(shaped)
. It seems a little bit tricky and honestly I am not so sure this might be a good way.
Or another possibility is to just do not care about the space, which would create some void between two or more MathParItems).
BTW, what is this bug with math equation line breaking and how is it going to be solved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug is that there is a space at the end of the first line in the picture above. I think the spacing should be "weak" and collapse at the end of the line, just like normal text spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug is that there is a space at the end of the first line in the picture above. I think the spacing should be "weak" and collapse at the end of the line, just like normal text spaces.
Sorry for the late update. While I have not figured out a good solution, I added a predecessor PR that eats white space introduced by the math equation in #4087 I think I will find a new way for the decoration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's really quite tricky. Ideally, we'd also not couple math and paragraph layout too tightly.
Maybe we should apply the highlighting completely in paragraph layout rather than math, on a per-line base (i.e. highlight consecutive stuff together with a larger bounding box, no matter whether it is text, math, h or whatever)? This could also fix #1716.
bf92854
to
3446de0
Compare
Can this format individual symbols inside math mode and keep them in the same font? For instance, the second example "unitalicizes" the |
@freddyholms Okay this is interesting, it seems that the n(n+1) (as long as it is wrapped by a []) is considered as normal text instead of math, that being said, this is a bug that is orthogonal to the changes made in this PR, which would have its own issue. Specifically, in the main branch (without the changes in this PR), if you try this sample
it would already result in this |
Okay I just picked up this PR again recently and have made a new solution (albeit being very complicated) but breaks many other tests, mainly because in |
I think the best solution is to reconsider highlighting from scratch and do it at the paragraph level. That way, for instance, adjacent text and math could also get a shared bounding box. If we want to go down that route, I'd like to wait with that until after I'm done with my layout refactoring though (or take a stab at it during it). |
Yes I am working towards this direction, a principled approach that handles text, math (and other frames) properly. Now other than some refactoring ( am sure there will be some ways), one other thing that I am struggling with is the issue of slack adjust as in
so this slackness adjustment would modify in place of the frame height/width, so if we do decoration of the math frames in line commit (after line-break is done), the frame.size() does not truthfully corresponds to the box where we want to do highlight. It would be great if the line height adjust can keep the math frames as it is, and only do the line height adjustment in line commit. Recently I discovered that #4236 seems to tackling the same issue, but it seems difficult to preserve and pass the existing tests... |
3446de0
to
bd50400
Compare
Just made a new draft PR, now the solution is limited to only highlight (because highlight is background) which is easier. Some limitations:
That being said, this PR is now only a very rough draft. The main thing that is blocking the issue is that the true frame size is being modified in slackness adjust, #4318 is one way to solve it; but there can be other ways, say, if the line height determination is refactored and merged #4236, in which case the slackness adjustment is no more. @laurmaedje I think the issue that the frame size is no longer being modified is fixed, I can make a more proper PR. (for example, completely merging the two functions "decorate_frame" and "decorate_shaped_text') |
Regarding #2200, it turns out the highlight is supported for normal text but not for math equations. This PR implement the highlight for math equations.
Like highlight, other line-based decorations are supported now as well, including overline, underline, strikethrough. Result
(tests added in
deco.typ
)Old
This is the second of two PR for solving #2200 (and shoud be easier to review if #3941 is merged first) (or Just merge this PR directly)
It continues on #3941 that add support for overline underline and strike for math equations. Some refactoring and corresponding tests are added as well.