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

Adding an option for use Asymptote to build svg graphics #1145

Closed
wants to merge 17 commits into from

Conversation

mmatera
Copy link
Contributor

@mmatera mmatera commented Feb 5, 2021

This PR partially fix #1078.

The idea is to be able of producing SVG graphics that do not contain MathML code inside. If Asymptote is available in the system, and the symbol Settings`UseAsyForGraphics2D is set to True, when
GraphicsBox.boxes_to_xml is called, instead of using the internal code for building svg images,
GraphicsBox.boxes_to_tex is called to produce a set of asy instructions. These instructions are saved in a temporary file, which is sent to the asy interpreter in order to produce an svg file. Finally, GraphicsBox.boxes_to_xml returns and <img> tag, with the contain of this file, codified as a b64 string as src.

This allows also us to show expressions with nested graphics and equations.

image

@mmatera mmatera changed the title Adding an option for use Asyptote to build svg graphics Adding an option for use Asymptote to build svg graphics Feb 5, 2021
@mmatera mmatera requested review from rocky and GarkGarcia February 5, 2021 16:49
@mmatera
Copy link
Contributor Author

mmatera commented Feb 7, 2021

If we agree to add this, it would be useful to set Settings`UseAsyForGraphics2D=True in mathics-django, as well as adding asymptote as a dependency there.

Copy link
Member

@rocky rocky left a comment

Choose a reason for hiding this comment

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

I can't get this to work:

mathics-async

I like the overall intent though.

@@ -3142,7 +3142,7 @@ def get_range(min, max):

return elements, calc_dimensions

def boxes_to_tex(self, leaves, **options):
def boxes_to_tex(self, leaves, forxml=False, **options):
Copy link
Member

Choose a reason for hiding this comment

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

Adding a parameter like this feels really hacky.

The front end should be involved here. The backend merely needs to provide the underlying building blocks.

Copy link
Contributor Author

@mmatera mmatera Feb 21, 2021

Choose a reason for hiding this comment

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

Adding a parameter like this feels really hacky.
Agree. Maybe I should hide it inside **options

The front end should be involved here. The backend merely needs to provide the underlying building blocks.

OK, but for it, I would need to merge #1140 first...

Copy link
Member

Choose a reason for hiding this comment

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

Let's wait until #1140 clears then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea of these modifications is to have a working graphics engine without changing the current core. Most of the code is there, and if the control variable is not set (or set to false), we get the previous (buggy) behavior.
On the other hand, this does not clash against #1140, this is why I put this in a separated PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a parameter like this feels really hacky.

The front end should be involved here. The backend merely needs to provide the underlying building blocks.
Regarding this, I put forxml as an optional parameter.

Copy link
Member

@rocky rocky left a comment

Choose a reason for hiding this comment

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

I believe there is code in the tree drawing routines of pymathics-graph that suggest how to scale the text font size based the distance between nodes which here would be applied to axis labels.

@mmatera
Copy link
Contributor Author

mmatera commented Feb 21, 2021

I can't get this to work:

mathics-async

I like the overall intent though.

Stupid questions:

  • Do you have asymptote installed?
  • Can your asymptote installation to get svg`s from asy text files?

@@ -2137,7 +2137,7 @@ def __neg__(self) -> 'Rational':

@property
def is_zero(self) -> bool:
return self.numerator().is_zero and not self.denominator().is_zero()
return self.numerator().is_zero and not(self.denominator().is_zero)

Copy link
Member

Choose a reason for hiding this comment

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

Should this be in a separate PR. It fixes a current bug and is not specific to asymptote, right?

Copy link
Contributor Author

@mmatera mmatera Feb 21, 2021

Choose a reason for hiding this comment

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

I put this because I got CI errors due to PossibleZeroQ. I removed this change from this PR. Please look at #1164.

@rocky
Copy link
Member

rocky commented Feb 21, 2021

I can't get this to work:
mathics-async
I like the overall intent though.

Stupid questions:

  • Do you have asymptote installed?
  • Can your asymptote installation to get svg`s from asy text files?
  • I have asymptote installed. BTW if I didn't I would expect changing Settings`UseAsyForGraphics2D=True would warn me that I am doing something stupid, right?
  • I am getting the SVG's created in my tempdir with the text added, so I guess the answer is "yes" here too.

Given the above, I am guessing there is some coordination that needs to go on with Mathics-Django. In other words this gets created, but Mathics-Django doesn't know that it has to do something special go get this image pulled in, right?

@rocky
Copy link
Member

rocky commented Feb 21, 2021

BTW although this is really good stuff, I am delighted we didn't delay the last release further to try to get this in.

@mmatera
Copy link
Contributor Author

mmatera commented Feb 21, 2021

I can't get this to work:
mathics-async
I like the overall intent though.

Stupid questions:

  • Do you have asymptote installed?
  • Can your asymptote installation to get svg`s from asy text files?
* I have asymptote installed. BTW if I didn't I would expect changing  `` Settings`UseAsyForGraphics2D=True `` would warn me that I am doing something stupid, right?

* I am getting the SVG's created in my tempdir with the text added, so I guess the answer is "yes" here too.

Given the above, I am guessing there is some coordination that needs to go on with Mathics-Django. In other words this gets created, but Mathics-Django doesn't know that it has to do something special go get this image pulled in, right?

I tried with the master branch of mathics-django and it works for me, with this branch of Mathics. I have created a PR in mathics-django that sets the variable to True. @rocky, please have a look when you have time.

@rocky
Copy link
Member

rocky commented Feb 27, 2021

I spent some time tracking down what's up. If I use firefox I see the information. But if I use google chrome I don't.

In the chrome cases I do see:

Request URL: 
data:image/svg+xml;base64,PHN2ZyB2ZXJzaW9uPScxL ... Zz4=

getting sent back.

@rocky
Copy link
Member

rocky commented Feb 27, 2021

It also doesn't work on the opera browser

@mmatera
Copy link
Contributor Author

mmatera commented Mar 2, 2021

Updates:

  • I moved this code to a pymathics module, in order to reduce the overhead in the core CI. See pymathics-asy
  • The issues with Opera and Chrome are related to the way in which tags are interpreted. To fix this, I think we need to work on the js interface of mathics-django. The other possibility would be to modify mathics core, but it would require more sophisticated changes.

@rocky
Copy link
Member

rocky commented Mar 2, 2021

I moved this code to a pymathics module, in order to reduce the overhead in the core CI. See pymathics-asy

@mmatera Ok. So is this branch obsolete? Or in need of code removal/update?

@mmatera
Copy link
Contributor Author

mmatera commented Mar 4, 2021

I close this because all the code is inside pymathics-asy.

@mmatera mmatera closed this Mar 4, 2021
@mmatera mmatera deleted the useasyforxmlgraphics branch March 14, 2021 18:28
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.

Text (in Graphics) is broken
2 participants