feat: basic group shape #1294
feat: basic group shape #1294
Conversation
± Registry diff📊 PerformanceKeyNote that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms. Data |
Codecov Report
@@ Coverage Diff @@
## main #1294 +/- ##
==========================================
+ Coverage 61.28% 61.43% +0.15%
==========================================
Files 63 65 +2
Lines 7828 8053 +225
Branches 1845 1887 +42
==========================================
+ Hits 4797 4947 +150
- Misses 2924 2998 +74
- Partials 107 108 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Deploying with
|
| Latest commit: |
3c3f9f8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9ff79c08.penrose-72l.pages.dev |
| Branch Preview URL: | https://group-shape.penrose-72l.pages.dev |
There was a problem hiding this comment.
Thanks for doing this @liangyiliang! Very important first step towards better structure and reusability in Style.
My main request is to take a closer look at group graph construction in the renderer and figure out if we can avoid re-computation of the graph as much as possible.
I haven't thought deeply about the usability and performance implications of the saturation rules. At a glance it makes sense to me and the implementation seems straightforward. I'll leave it to @joshsunshine and @samestep to evaluate.
Generally, this PR has some duplicated code and some functions tend to have very long names. I'd suggest refactoring them to avoid duplication. If a function doesn't have a great name and it's called once in your code, I think you can consider inlining it to improve readability. See my code-level comments below.
| const rawShapeProps = gpi.contents[1]; | ||
| if (!isShapeType(shapeType)) { | ||
| throw new Error("Unknown shape in Group bbox: " + shapeType); | ||
| } else { |
There was a problem hiding this comment.
Unrelated to this PR: we should really remove the need for boilerplate like this.
There was a problem hiding this comment.
Yep, agreed; #715 touches on this, but doesn't entirely cover the problem.
I agree! I will store the Group Graph in the State. |
|
@liangyiliang you modified some newline at the end of |
There was a problem hiding this comment.
I still need to review the rest of the code, but while I'm doing that, could you add information about Group to the docs on the website in this PR? You'll at least need to add a packages/docs-site/docs/ref/style/shapes/group.md file, but you should probably also put more info in packages/docs-site/docs/ref/style.md too.
Done!
Yup I will do it! |
Done! |
Description
Resolves #783 .
We added a basic
Groupshape that contains multiple already-declared shapes. Each group is now rendered as a<g>tag which contains the internal shapes.As an example,
can now generate
New Shape
We add a new shape,
Group, to the shape library. As of now,Groupshape only has one property,shapes, and an automatically generatedname. Propertyshapesis a list of paths to previously-defined shapes.Changes to the Compiler
The Style language now allows users to have a list of shapes, denoted
[path1, path2, ...]wherepath1,path2, ... are paths to shapes.GPI or ShapeAD Objects?
We go through and process each Style statement in the
translationprocess, at which point only the GPIs of each shape can be accessed. The actualShapeADobjects are available near the end of the compilation process. Therefore, a Group shape, initially, stores a list of GPIs. Near the end of the compilation process, after we already have a list of all ShapeADs, we switch each GPI into the corresponding ShapeAD objects.Group Graph
The compiler uses a directed graph, named$g \rightarrow s$ ) if and only if shape $s$ is a child of group $g$ .
GroupGraph, to represent group-shape relationships between shapes. Formally, a directed edge exists between two shapes (For example,
s1, s2, s3, s4, s5, s6s1, s2belong to groupg1s3, s4belong to groupg2g1andg2, and shapes5, all belong to groupg3s6does not belong to any groups.Then, the group graph should look like
We can use this group graph to check whether or not the grouping relations are well-defined, which means
The compiler fires a warning if any of these checks fail. A well-defined grouping relation should look like a tree (as in the example).
Layering
Suppose all layering directives have the form
layer s above s'(layer s below s'is equivalent tolayer s' above s). We want the following nice properties: (these are not the actual rules we follow for the algorithm)Concretely, in the group graph above, writing
layer s1 above g2implies:layer s1 above g3andlayer s3 above s4(sinces3, s4are members ofg2)layer g1 above s3andlayer g1 above s4(sinces1is a member ofg1)layer s2 above s3andlayer s2 above s4(sinces2is a member ofg1)layer g1 above g2(sinces1is a member ofg1)These are the "saturation" rules which saturates the entire layer graph and hence is not efficient. The algorithm leverages upon the observation that layering directives are only useful when two shapes are siblings within the same group. Suppose we encounter the layering directive
layer s above s'.When encountering
layer s1 above s2, the algorithm attempts to "walk up" the group graph from boths1ands2towards the root, until when the algorithm findss1'ands2'which are siblings. Then, the algorithm applies the same layering directive tos1'ands2'.In the concrete example of
layer s1 above g2, the algorithm would "walk up"s1to getg1. Sinceg1andg2are siblings, we addlayer g1 above g2to the layering graph. As another example,layer s1 above s6involves walking ups1twice to getg3which is a sibling ofs6, so we addlayer g3 above s6to the layer graph.A few more (more trivial) examples:
layer s3 above s4gets added to the layer graph directly, sinces3ands4are already siblings.layer g3 above g3also gets added to the layer graph directly, since we considerg3to be a "sibling" of itself. This will cause a cycle, which is perfectly expected.layer s4 above g3would involve walking ups4twice to getg3. Similarly, we addlayer g3 above g3.The final layer graph would only contain edges between siblings.
Keeping Groups Together
The layering directives would force the layering to be
bottom s2 s3 s1 top. This itself does not cause a loop, but does break up the groupg. The compiler should generate a warning.Running the algorithm, we have
layer s1 above s2just gets added to the layer graph becauses1, s2are siblings.layer s3 above s2requires one "walk up" froms2tog, which is a sibling ofs3, solayer s3 above ggets added to the layer graph.layer s3 below s1requires one "walk up" froms1tog, which is a sibling ofs3, solayer s3 below ggets added to the layer graph.We hence have both
layer s3 above s2andlayer s3 below s1, which causes a loop and a warning should fire.Changes to Compilation
The style compilation results in a
Stateobject whoseshapesfield contains a flat, ordered list of all shapes. With groups, the list of shapes should no longer be a flat list. It still storesShapeADobjects, but eachGroupshape's members should be actualShapeADobjects. That is, the list of shapes should contain onlyShapeADobjects for the top-level shapes, and all lower-level shapes should be within their respective parent groups. Furthermore, the list of top-level shapes, and for each group, the list of its child shapes, should all be ordered according to the layering. This is easy simply by sorting the elements of the lists based on the layer ordering. The list of top-level shapes should then be passed into the renderer.Previously, after
translation, the compiler does the following in order:computeShapeOrdering)ShapeADobjects (getShapes)The change goes,
Groupshapes store GPIs, not actual shape objects, since shape objects don't exist yet).ShapeADs, this time each group storing its actual sub-shapes. This is what we pass into the renderer.Changes to the Renderer
The renderer now takes a list of top-level shapes, and renders them. Whenever it encounters a group, it recursively renders all the member shapes before packaging everything into one
<g> ... </g>tag.Examples with steps to reproduce them
The
Wet Floorexample now uses groups.Can also do bbox-related constraints on groups: here
Checklist
Open questions
Turns out, warnings don't show up in the IDE.