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

Fixes a bug in msqrt when computing clearance value #412

Merged
merged 1 commit into from
Jan 7, 2020
Merged

Fixes a bug in msqrt when computing clearance value #412

merged 1 commit into from
Jan 7, 2020

Conversation

lin
Copy link
Contributor

@lin lin commented Dec 23, 2019

The formula to compute clearance in sqrt is contradicted with TeXBook definition. The result of this is that the base element is too close to the root bar. This might be a mistake in manual derivation as argued bellow unless this is purposely designed.

According to Rule 11 on TeXBook page 443 (Appendix G ).

The clearance value psi, which is q value in getPQ, is originally defined as psi = t + p / 4.

Under condition dy > bbox.h + bbox.d + psi, where dy = sbox.h + sbox.d - t, psi will be changed to a new value psi + (dy - (bbox.h + bbox.d + psi)) / 2 as stated in TeXBook.

The derivation for the condition dy > bbox.h + bbox.d + psi is right.

// since
dy = sbox.h + sbox.d - t
dy > bbox.h + bbox.d + psi
// so
sbox.h + sbox.d - t > bbox.h + bbox.d + psi
sbox.h + sbox.d  > bbox.h + bbox.d + psi + t
sbox.h + sbox.d  > bbox.h + bbox.d + t + p / 4 + t
// since
this.surdH = bbox.h + bbox.d + 2 * t + p / 4
// the condition is same as:
sbox.h + sbox.d  > this.surdH

However, the derivation for new assignment psi = psi + (dy - (bbox.h + bbox.d + psi)) / 2 is wrong.

// since
psi =  psi + (dy - (bbox.h + bbox.d + psi)) / 2
dy = sbox.h + sbox.d - t
// then
psi =  psi + ( sbox.h + sbox.d - t - (bbox.h + bbox.d + psi)) / 2
psi =  (sbox.h + sbox.d - t - (bbox.h + bbox.d - psi)) / 2
psi = (sbox.h + sbox.d - (bbox.h + bbox.d + t - psi)) / 2

Here is where I believe the derivation got wrong. If the last minus sign was a plus sign.

// if when wrong
psi = (sbox.h + sbox.d - (bbox.h + bbox.d + t + psi)) / 2
// since
this.surdH = bbox.h + bbox.d + 2 * t + p / 4
// then
psi = (sbox.h + sbox.d - (this.surdH - t)) / 2

This is the formula in the develop branch. The correct derivation should be

psi = (sbox.h + sbox.d - (bbox.h + bbox.d + t - psi)) / 2
// since
this.surdH = bbox.h + bbox.d + 2 * t + p / 4
// then
psi = (sbox.h + sbox.d - (this.surdH - 2 * psi)) / 2
psi = (sbox.h + sbox.d - (this.surdH - 2 * t - p / 2)) / 2

This is how this pull request computes psi, the clearance value.

@lin
Copy link
Contributor Author

lin commented Dec 23, 2019

comparison

This is a comparison preview for this request. Other TeX libraries render similar results as in this pull request.

@dpvc dpvc self-assigned this Jan 3, 2020
@dpvc
Copy link
Member

dpvc commented Jan 7, 2020

I've had the chance to check this out, and your computations are correct. The results are much better. Thanks for the correction, and fo finding exactly where the error was!

@dpvc dpvc merged commit 4879222 into mathjax:develop Jan 7, 2020
@dpvc dpvc removed their assignment Jan 7, 2020
@dpvc dpvc added this to the 3.0.1 milestone Jan 7, 2020
@lin lin deleted the develop branch January 8, 2020 13:53
@lin
Copy link
Contributor Author

lin commented Jan 8, 2020

Thanks, wish I could contribute more. 😄

@dpvc dpvc removed this from the 3.0.1 milestone Feb 5, 2020
@dpvc dpvc added this to the 3.0.1 milestone Mar 5, 2020
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

2 participants