I followed the developer guide for image comparisons,ref, and I pass the tests locally.
Although, going through theresult_images, I find there weren't anysvg extensions generated for the test I appended - It only generatedpng andpdf extensions.
For all the previousbaseline_images, there aresvgs along withpngs andpdfs.

Is there something I missed, since previous PRs related tomathtext do havesvgs in them? For example,#8151.

Hey@QuLogic, could you take a look at this too? Thanks :D

Removed the extra commits

Doc build failure seems due to#19163.@aitikgupta could you please rebase on master to pick up that change, so that docs build cleanly?

@anntzer I'd like to have your review as our "master of fonts" 😄. Feel free to turn the request down if you don't have time.

I didn't really follow@QuLogic's discussion about alignments, but right now looking at

rcParams["text.latex.preamble"]=r"\usepackage{amsmath}"s=r"$\overset{a}{a}\overset{p}{a}\overset{a}{b}\overset{p}{b}\underset{a}{a}\underset{b}{a}\underset{a}{p}\underset{b}{p}$"figtext(.5,.6,s,usetex=True)figtext(.5,.4,s)

we get

It looks like TeX's approach is to make sure the baselines of the main symbols stay always aligned (which seems more useful than trying to align the decorators?); likely we should do the same?

Looks like what@QuLogic said aboutunderset being a bit lower thanoverset is in TeX backend too:

usetex=True

usetex=False

With the latest commit, alignment looks like this:

I would think it would be better if the p's baselines could also be aligned?

I would think it would be better if the p's baselines could also be aligned?

@anntzer Something like this?

Yes (unless someone wants to argue in favor of the previous behavior...).

But thT is still not the same?

But thT is still not the same?

@jklymak I'm not sure I understand, do you mean the baselines do not match? Maybe this will help:

The overset and underset baselines are not aligned

They are not aligned in TeX either, but the spread here is a bit larger than in TeX. Personally, I don't find that too bad.

The spread is a bit larger even for a normal text:

We could reduce the gap in the finalhlist theoretically, it will look something like this (have not pushed):

Just for reference, here is the current version (which is currently in the PR):

Should I update the PR to remove the gap?
@jklymak@timhoffm

@aitikgupta I meant the spread of the baselines of the sub/supersets between a and p.
But you are right that the character spread is a bit larger too.

Personally I'm ok with both variants in both cases, but I'm not the most critical one here.

@timhoffm Given your comment at#19186 (comment), do you want to comment wrt. baseline images? (Everything else looks great to me.)

@anntzer TL;DR For the sake of simplicity, you can just merge with the new baseline images.

Here it's "only" a little over 40kB of baseline images and the positioning here is much more complex. I think this should be tested and adding the baseline images is okish. (The alternative would be splitting the tests into a separte PR and keep that open until we have less data heavy ways of doing the test - either with separated baseline images or by only using a subset; probably we don't need all combinations of fonts and output formats?).

Btw. what's the status with separating baseline images from the code repo?

I agree positioning matters here much more than for the sqrt PR (for which I agree we could do without the test images...).

Btw. what's the status with separating baseline images from the code repo?

Some day I'll have time to get back to it. But that day is not particularly close :(

@aitikgupta Actually let's see whether we can resolve#19186 (comment) first to save a lot of disk space on the baseline images problem. However I don't want you to believe that I'm needlessly temporizing on this so regardless of whether the proposal at#19186 (comment) gets implemented I will make sure this is merged before 3.4 is released (hence the self-assignment).

(I can do the rebase if you prefer, just let me know.)

No that's fine, I'll do it

I think just rebasing wouldn't work, I'd need to generate the baseline images too?

Yes, sorry I forgot to mention that.

Is it fine the way it is now?
Or should I consider breaking down the current test into 2 (one for overset, one for underset), since the tests only compare a single png now

Thanks for the PR :)

Uh, while rebasing the last time to add the new baseline images, one of my commits vanished: the whatsnew entry.
It was this commit I suppose:0a030d3

Now this is merged, but without the whatsnew entry.
Should we do something about it?
@anntzer@QuLogic

Open a separate PR.