Skip to content

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Jul 18, 2019

This is in preparation for turning on imgmath as a replacement
for KaTeX; we must not have width: 100% on images as it causes
rendering problems for img math.

Signed-off-by: Edward Z. Yang ezyang@fb.com

This is in preparation for turning on imgmath as a replacement
for KaTeX; we must not have width: 100% on images as it causes
rendering problems for img math.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>
@ezyang ezyang requested a review from brianjo July 18, 2019 17:09
ezyang added a commit to pytorch/pytorch that referenced this pull request Jul 18, 2019
Emprically, KaTeX is bad user experience for users who don't have
powerful CPUs.  With x4 CPU slowdown emulation in Chrome, I notice
that imgmath takes a third of the time to render than KaTeX.

This requires pytorch/pytorch_sphinx_theme#48 to
be landed first, as otherwise the style sheet renders img incorrectly.

Might be one step on the way to solving #20984.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>
ezyang added a commit to pytorch/pytorch that referenced this pull request Jul 18, 2019
Emprically, KaTeX is bad user experience for users who don't have
powerful CPUs.  With x4 CPU slowdown emulation in Chrome, I notice
that imgmath takes a third of the time to render than KaTeX.

This requires pytorch/pytorch_sphinx_theme#48 to
be landed first, as otherwise the style sheet renders img incorrectly.

Might be one step on the way to solving #20984.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ghstack-source-id: f20be2b
Pull Request resolved: #23025
@ezyang ezyang requested a review from ssnl July 18, 2019 17:14
@soumith
Copy link
Member

soumith commented Jul 18, 2019

I'd merge it but CI is failing

@patmellon
Copy link
Contributor

@ezyang Instead of removing width: 100% from _article.scss, I recommend that you add the following code to _sphinx_base.scss (beginning at line 777):

article.pytorch-article {
  .math {
    color: $not_quite_black;
    img {
      width: auto;
    }
  }
}

I'm just adding img { width: auto; } to the existing math class. This change should leave the tutorial images unaffected but ensure that the math images have the right size. Please let me know if this change works with the new math images.

ezyang added 2 commits July 23, 2019 11:18
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
@ezyang
Copy link
Contributor Author

ezyang commented Jul 23, 2019

@Pat878 This CSS was not quite sufficient, I also had to handle inline math elements correctly.

@ezyang ezyang merged commit fc25a41 into master Jul 23, 2019
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.

4 participants