penrose icon indicating copy to clipboard operation
penrose copied to clipboard

Use global transformation of final SVG, rather than `toScreen`

Open keenancrane opened this issue 4 years ago • 0 comments

Currently we implement a bunch of shapes by applying toScreen to each attribute (center, start, end, etc.), which transforms coordinates to the canvas shape/size shown in the current browser window. In other words, we change the actual coordinate values of the SVG shapes according to the current view.

This design isn't great for a number of reasons:

  • It causes the downloaded SVG to be scaled based on the user's browser window size. This means, for instance, that if you download a diagram A, tweak some content, change the browser window size, then download the new diagram B, the two diagrams A and B will be scaled by different, arbitrary amounts. Now if you try to include them both in the same document (e.g., a paper or slide presentation) they'll show up as different sizes, and you'll have to guess what the scale factor is between them.
  • It's brittle—in fact, there are already a number of places where the toScreen calls have been forgotten, omitted, leading to broken rendering (see for instance #724, which basically happens because toScreen isn't being applied to elements of the Polygon point list).
  • It makes the code hard to read/debug by littering it with repeated calls to the same transformation.

In general, a reasonable principle to live by when writing graphics code is:

Don't transform the internal state just to fit something on screen.

In this case, there is a much simpler and more elegant solution: just wrap the entire generated SVG in a single group tag (<g>) that scales and translates the diagram so that it shows up at the correct size/location in the browser window. Or better yet: include the generated SVG in the webpage in a way that performs this transformation, so that it's never "baked in" to the SVG.

This change will also help to keep coordinate systems consistent in headless mode.

keenancrane avatar Jan 04 '22 16:01 keenancrane