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

SVG rendering clips points at the boundaries. #1091

Closed
rocky opened this issue Jan 1, 2021 · 8 comments · Fixed by #1407
Closed

SVG rendering clips points at the boundaries. #1091

rocky opened this issue Jan 1, 2021 · 8 comments · Fixed by #1407

Comments

@rocky
Copy link
Member

rocky commented Jan 1, 2021

To replicate

Needs["DiscreteMath`CombinatoricaV0.9`"]
FerrersDiagram[Range[3]]

Ferrersbug

@GarkGarcia
Copy link
Contributor

That's interesting, it looks like FerrersDiagram is setting the viewBox attribute of the image based on the coordinates of the center of the dots. How is FerrersDiagram implemented?

@GarkGarcia
Copy link
Contributor

Ok, I think I found the definition:

In[1]:= FullForm[FerrersDiagram[Range[3]]]
Out[1]= Graphics[List[PointSize[0.05], List[Point[List[1, 1]]], List[Point[List[1, 2]], Point[List[2, 2]]], List[Point[List[1, 3]], Point[List[2, 3]],
 Point[List[3, 3]]]], List[Rule[AspectRatio, 1], Rule[PlotRange, All]], Rule[$OptionSyntax, Ignore], Rule[AspectRatio, Automatic], Rule[Axes, False], R
ule[AxesStyle, List[]], Rule[Background, Automatic], Rule[ImageSize, Automatic], Rule[LabelStyle, List[]], Rule[PlotRange, Automatic], Rule[PlotRangePa
dding, Automatic], Rule[TicksStyle, List[]]]

So the definition should be

Graphics[stuff, {AspectRatio->1, PlotRange->All, $OptionSyntax->Ignore, AspectRatio->Automatic, Axes->False, AxesStyle->{}, Background->Automatic, ImageSize->Automatic, LabelStyle->{}, PlotRange->Automatic, PlotRangePadding->Automatic, TicksStyle->{}}]

@GarkGarcia
Copy link
Contributor

GarkGarcia commented Jan 2, 2021

This looks like an issue with Graphics, probably it's ImageSize->Automatic fault. We would have to refactor the algorithm used to figure out the dimensions of the image: it should consider the size of the points in the image as well as their position.

@GarkGarcia
Copy link
Contributor

This may also be a "problem" in asymptote: maybe it's the caller's responsibility to add some padding around the image to avoid this.

@rocky I don't quite understand how the images are generated, but I do have a lot of experience writing SVG by hand and I'm sure it's a problem with viewBox, width and height. To avoid this we need to add some padding around the image (the padding should be the radius of dots).

I don't know what's the most appropriate place for the patch. Maybe this is Combinatorica-specific and we should patch Combinatorica. I don't like this option because we would become responsible for maintaining the patch over time. Maybe this a generalized issue with Graphics and we should refactor the algorithm that determines the width and height of images. The latter is a more general solution, but it's also extremely hard to get right without the use of external tools.

All in all, here's what I think we should do:

  1. Figure out if it's a general issue with Graphics or not
  2. If it is then patch this when we're working on refactoring Graphics
  3. If it isn't then patch this in Combinatorica

@rocky
Copy link
Member Author

rocky commented Jan 2, 2021

Here is what I remember when I looked at this the other day. The points are positioned starting at (x,y) = (0,0). As you suggest, the problem is that the layout is not taking into account the radius of the points.

The specific place where things start to go wrong is in boxes_to_xml which I mentioned in my rant the other day. Intuitively obvious that this is where you'd look, right?

The xmin,ymin values (the dot with 3/4ths of it chopped off), has negative xmin,ymin.

Yes, one could add a padding, but I don't know if that kind of concept appears in WL. The approach I looked at was to use a SVG transform(translate()) to adjust (0,0), to xmin, ymin.

SVG transform seems to be implemented but I don't know that it is used anywhere. And Poke1024's incomplete graphics work seems to try to address that too, at least at some point, which you can see in the pdf he copied.

I think this is related to #1078 which also probably needs translate to work properly.

Getting translate fixed would go a long way towards improving our graphics overall since this is probably the reason our axes do not have labels on them.

@GarkGarcia
Copy link
Contributor

GarkGarcia commented Jan 4, 2021

Yes, one could add a padding, but I don't know if that kind of concept appears in WL. The approach I looked at was to use a SVG transform(translate()) to adjust (0,0), to xmin, ymin.

Yeah, I was looking into fixing the specific example you provided an it looks like we'll have to use translate. I thought that setting xmin/ymin to negative values would make it so that (0, 0) is displaced towards to center of image, but apparently that's not the case (so we'll have to displace it using translate as you've mentioned).

@rocky
Copy link
Member Author

rocky commented Jan 4, 2021

Also checkout https://github.com/mathics/Mathics/blob/master/mathics/builtin/graphics.py#L3021-L3031 and the call of that in boxes_to_mathml listed above.

We know that this is returning the wrong width and height (and h and w) values when xmin/ymin are less than 0. That's also true when xmax, and ymax are greater than what it returns as well.

BTW if translate is fixed, that would open the way for fixing a lot of other places where it is needed or used incorrectly. So thanks for undertaking this. See next comment.

@rocky
Copy link
Member Author

rocky commented Jun 5, 2021

Here is what's going on. Basically PointSize is not implemented. And equally bad a PointBox is computed assuming that points have a radiius/diameter/area of zero! But when the formatting routines eventually get called, of course they set the radius to some nonzero value. So of course those are going to extend outside of the SVG viewport.

PointSize here is specified as a fraction of the overall width. And I don't see that this is known until very late graphics processing, such as after at least an initial guess is made.

@mmatera here is an example where you could conceive of the format step making use of Mathics evaluation. A PointSize could be kept as a Mathics symbolic expression and so when the size is known the Mathics symbolic expression is turned into an actual value.

(Another approach is just to bind this to some sort of Python function which is passed the size when it is known.)

@rocky rocky changed the title SVG rendering clips circles at the boundaries. SVG rendering clips points at the boundaries. Jun 5, 2021
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 a pull request may close this issue.

2 participants