refactor: Reorganize shapes

[samestep]

Description

Fixes #700, fixes #701, fixes #709, fixes #712, fixes #714. Joint work with @joshsunshine and @maxkrieger.

This PR does the following:

• Remove the following shapes:
  • Callout (it's gross)
  • FreeformPolygon (it's redundant with Polygon)
  • PathString (it's redundant with Path)
  • Square (it's redundant with Rectangle, and it's not in SVG, unlike Circle)
  • Arrow (it's redundant with Line)
• Delete renderer/ShapeDef.
• Add a new type hierarchy for shapes:
  • Create types/shapes with several new interfaces holding commonly shared properties
  • Create shapes/ directory holding most of the other things:
    • Create shapes/Samplers holding samplers, which now just sample rather than returning sampler functions
    • Create a file for each individual shape type:
      • Export an interface for the properties of that shape
      • Export a sampler function to sample a random instance of that shape's properties
      • Export a type for a concrete instance of that shape
      • Export a constructor function to get a concrete instance of that shape given some subset of its properties
    • Create shapes/Shapes:
      • Add a Shape type that is the union of all individual concrete shape types
      • Migrate a modified version of the previous ShapeDef type, which holds metadata about shape types
      • Export a shapedefs object which maps from shape type names to ShapeDefs
• Move bbox computation functions into engine/BBox
  • Move bbox tests into the test file for engine/BBox
• Do a couple minor refactorings:
  • Move propertiesOf and propertiesNotOf from compiler/Style to engine/EngineUtils
  • Move getShapeName from renderer/ShapeDef to engine/PropagateUpdate
  • Type State's uninitializedPaths field as Path[] instead of any
  • Move arrowHead and makeRoomForArrows from renderer/Arrow to renderer/Line
• Modify compiler/Style to use the new shapedefs and samplers
• Modify renderer/Resample to very inefficiently use the new samplers instead of the old ones (need to fix this)
• Modify all examples/ and TypeScript code to get rid of the deleted shape types and migrate property names

expand to see subtask tracking used while developing this

• Rectangle
• the other shapes
• name the properties :)
• reconnect to ShapeDef.test.ts
• samplers
• connect it to the compiler
  • Update Style.ts, in particular the function that initializes shapes:

    penrose/packages/core/src/compiler/Style.ts

    Lines 2688 to 2692 in d73815d

    	const initShape = (
    	tr: Translation,
    	[n, field]: [string, Field]
    	): Translation => {
    	const path = mkPath([n, field]);

• use these types in the optimizer
• use them in the renderer
  • update the property names that were changed
  • Move content of render functions, e.g. the Arrow function below, to render methods in the Shape types:

    penrose/packages/core/src/renderer/Arrow.ts

    Lines 84 to 87 in d73815d

    	const Arrow = ({ shape, canvasSize }: ShapeProps) => {
    	const elem = document.createElementNS("http://www.w3.org/2000/svg", "g");
    	elem.setAttribute("pointer-events", "bounding-box");
    	const id = `arrowhead_${shape.properties.name.contents}`;

  • Update Render.ts, in particular the function that renders each shape to call the new render methods:

    penrose/packages/core/src/renderer/Renderer.ts

    Line 26 in d73815d

    export const RenderShape = ({

• (temporarily?) remove all the dragging stuff
• get rid of types/shape probably
• fix all the bugs I introduced
  • src/compiler/Style.test.ts
  • src/engine/BBox.test.ts
  • src/__tests__/overall.test.ts

Checklist

• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• New and existing tests pass locally using yarn test
• I ran yarn docs and there were no errors when generating the HTML site
• My code follows the style guidelines of this project (e.g.: no ESLint warnings)

(We don't currently check ESLint, but #755 will be a step on the path toward letting us do that. 🙂)

Future work

• feat: renderer/style: Passthrough unknown properties to Svg output #759
• Use the new shape types throughout the codebase
• Move things from fields on the ShapeDef to methods on the Shape

[cloudflare-pages]

Deploying with    Cloudflare Pages

Latest commit:	bdd312d
Status:	✅  Deploy successful!
Preview URL:	https://6fc3026d.penrose-panes.pages.dev

View logs

[codecov]

Codecov Report

Merging #743 (bdd312d) into main (bfc7b2e) will increase coverage by 0.54%.
The diff coverage is 87.39%.

@@            Coverage Diff             @@
##             main     #743      +/-   ##
==========================================
+ Coverage   65.01%   65.56%   +0.54%     
==========================================
  Files          46       53       +7     
  Lines        8235     7939     -296     
  Branches     1534     1460      -74     
==========================================
- Hits         5354     5205     -149     
+ Misses       2872     2725     -147     
  Partials        9        9              

Impacted Files	Coverage Δ
packages/core/src/engine/Evaluator.ts	64.65% <ø> (ø)
packages/core/src/renderer/Polygon.ts	25.00% <ø> (+1.92%)	⬆️
packages/core/src/renderer/Polyline.ts	25.00% <ø> (+1.92%)	⬆️
packages/core/src/renderer/Text.ts	100.00% <ø> (ø)
packages/core/src/renderer/shapeMap.ts	100.00% <ø> (ø)
packages/core/src/utils/CollectLabels.ts	87.30% <ø> (ø)
packages/core/src/utils/Util.ts	25.30% <0.00%> (+0.59%)	⬆️
packages/core/src/renderer/Resample.ts	25.45% <10.00%> (+2.12%)	⬆️
packages/core/src/renderer/Renderer.ts	32.87% <33.33%> (ø)
packages/core/src/utils/Error.ts	34.08% <50.00%> (ø)
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfc7b2e...bdd312d. Read the comment docs.

[maxkrieger]

LGTM

[wodeni]

Very nice! I liked the design in general. I had one minor comment about propTags in Shape and initShape in the Style compiler. All the shape definitions look great to me!

[keenancrane]

@samestep Confused about one thing: when we spoke about strokeLineCaps you mentioned that this is not yet supported by generic SVG pass through. But it looks like @cmumatt already merged #749… right?

[samestep]

Confused about one thing: when we spoke about strokeLineCaps you mentioned that this is not yet supported by generic SVG pass through. But it looks like @cmumatt already merged #749… right?

@keenancrane In order to pass strokeLineCap through, you'd need to write it in the Style program as stroke-linecap, which you can't do until #759 is merged due to a limitation in the lexer.