Extending Penrose viz-à-viz documentation #362

strout18 · 2020-07-23T02:47:17Z

Description

This branch has multiple projects, which are each independent but share one common goal: increasing Penrose's visual documentation and enabling future documentation. Current code focuses mainly on shapes.

Mod tab on Inspector.

Added a tab to the inspector that lets you modify shapes declared in a Substance file. It works with any sub/sty/dsl files, but recommended as base examples are any of the sub files in examples/vizdoc/baseshapes. Select a shape on the left. You should then be able to adjust any of the attributes on the selected shape pre- or post-optimization. To add custom shapes or adjust input types just check out the “Adding shapes to mod” section inside of the Inspector README. The goal of this was to provide an in-Penrose shape explorer allowing the user to see in real-time how different attributes affect the shape. Could also see this being used to 'tweak' a final diagram post-optimization.

runpenrose vizdoc/baseshapes/circle.sub vizdoc/baseshapes/baseshapes.sty vizdoc/shapes.dsl

Docshapes

The examples/vizdoc/docshapes directory contains multiple .sub .sty .dsl triplets that can be used to create simple shape diagrams. The goal with these was to create aesthetically pleasing, informative diagrams accessible to the casual user that demonstrate relevant properties of the shapes. A couple examples and the paths to reproduce them are below.

runpenrose vizdoc/docshapes/circle.sub vizdoc/docshapes/circle.sty vizdoc/shapes.dsl

runpenrose vizdoc/docshapes/line2.sub vizdoc/docshapes/line2.sty vizdoc/shapes.dsl
All example PNGs so far can be found in the Google Drive. Current shapes are Square, Rectangle, Line, Ellipse, Arrow, Text, Circle, and Image.

Shape modifier - HTML style

Added Penrose-independent shape-explorers, similar to the Mod tab. They can be found in examples/vizdoc/html. There are shape-specific explorers for current shapes in Penrose, as well as template.html and template.js files that support custom shapes. The template files currently work only on shape attributes that have a one-to-one correspondence with an SVG attribute or a one-to-many (like how Square.side in Penrose maps to both the width and height properties of a corresponding SVG rectangle), but not a many-to-one or many-to-many. The template file could be extended further, but the Mod tab in the inspector covers most of its functionality so it's probably deprecated. The immediate advantage of the pure HTML/JS file is that it could be embedded in a documentation page, whereas the Mod tab is only accessible through Penrose. To run just open the HTML file in Chrome. If this were to be implemented in documentation, we might want to implement CSS styling. Currently very bare bones. The example below is examples/vizdoc/html/rectangle.html.

Heatmap

Implemented an energy heatmap generator for constraints/objectives at examples/vizdoc/html/heatmap.html. Currently, the expression for the constraint/objective is entered manually in the file, and a heatmap is rendered using d3. It also supports adjusting x and y coordinates of shapes that objectives are being applied to. Still in most basic stage. Would not advise using in documentation until further work is done. Example below uses the expression for the sameCenter objective.

Square hotfix

The Square shape did not support the strokeStyle property. I added that functionality. Can repro by runpenrose vizdoc/baseshapes/square.sub vizdoc/baseshapes/baseshapes.sty vizdoc/shapes.dsl and modifying strokeStyle in the mod view of the inspector.

Remaining issues

Added an option in the Mod tab to support setting minima and maxima of ranges to Canvas boundaries. However, Canvas boundaries are currently hardcoded (see Issue Canvas size shouldn't be hardcoded #361). Once/if that changes, the onCanvas function in LabeledInput.tsx will need to be updated. For more information on this, see the "Adding shapes to mod” section in the inspector README.
Mod tab does not support changing the string property of a Text/Label element. I think this is because the string property is set during the collectLabels method that occurs during optimization and so will not be affected by a change in Canvas state (which Mod changes in order to modify the shapes on the canvas). Perhaps this issue could be fixed in Mod, but the most straightforward fix seems to be to not gather labels during optimization...
Mod tab has been tested on simple Curve instances with points only: runpenrose linear-algebra-domain/twoVectorsPerp.sub linear-algebra-domain/linear-algebra-paper-simple.sty linear-algebra-domain/linear-algebra.dsl
- Mod tab has not been tested on Curve instances. This should happen eventually, but most of the functions (in Functions.hs on master) that are used to generate points on a Curve have not yet been ported to web-runtime and I wasn't sure about manually defining points.

…ables (but still evaluate the new state for label rendering)

…xamples of shapes in vizdoc

maxkrieger · 2020-07-24T00:12:39Z

This is dope omg

wodeni · 2020-07-26T23:14:34Z

The heatmap demo is super cool! I'm assuming this is done with respect to one shape and it's positional properties. Would be interesting to see if this can be generalized to other types of properties (for optimized colors, a heatmap on the color space??).

kai-qu · 2020-07-28T03:03:28Z

Yeah this is all great work! Stella and I have talked about extending the shape mod tab to incorporate information from the computational graph of the diagram. As I wrote in Slack:

Also, at some point, may be good to think through which values may need to be recomputed or reoptimized to preserve the relationships in the style program. e.g. if x.shape.z was set to a.shape.y + 2 in Style, then the user wants to change x.shape.z, what should happen?

kai-qu · 2020-10-10T20:50:45Z

Thanks @strout18, I was able to use the mod tab on some examples pre- and post-optimization and it looks really cool. I can do a general review Mon or Tues so we can get this into web-runtime and merge web-runtime ASAP.

@maxkrieger or @wodeni, can you review how @strout18's mod tab integrates with the inspector, and in general, how well it works with the React canvas update cycle, and anything else relevant to web-runtime? (since it updates the shapes on drag) Before Weds would be great for the merge.

examples/vizdoc/html/heatmap.html

wodeni

Looks great overall. Thanks for making this possible @strout18 ! Excited to integrate things like mod into the diagramming workflow eventually (#21 !). Most of my comments are for our future reference, not requests for changes.

wodeni · 2020-10-12T22:40:23Z

react-renderer/src/inspector/views/Mod.tsx

+              ({ properties, shapeType }: Shape, key: number) => {
+		  // If the inspector is crashing around here, then probably the shape doesn't have the width/height properties, so add a special case as below
+		  // console.log("properties, shapeType", properties, shapeType, properties.w, properties.h);
+


Looks like this is for computing the viewbox of the preview svg? This is okay for now. In the future, I think it would be helpful to have a generic bbox function on or property of IShape types.

Also the formatting looks a bit off. I'd recommend a linter. Lmk what editor you use @strout18 !

That code is copied and pasted directly from Shapeview, so maybe to change this we should involve @maxkrieger . That's also probably why the formatting is off (I use VSCode anyway).

wodeni · 2020-10-12T22:43:29Z

react-renderer/src/inspector/views/mod/LabeledInput.tsx

+
+// if we move to not hardcoding canvas size this needs to be changed
+// returns number corresponding to x/y coord boundary of canvas if necessary
+const toCanvas = (jsonVal: string) : string => {


Our frontend should really have a base module that exports the canvas size. Will be a TODO if we resolve #361

react-renderer/src/inspector/views/mod/LabeledInput.tsx

wodeni · 2020-10-12T23:03:00Z

react-renderer/src/inspector/views/mod/shapedefs/arrow.json

@@ -0,0 +1,55 @@
+{


Great to finally have shapedefs in the frontend! We will eventually move the backend shapedefs into the frontend as well. A more principled way to generate these mod widgets: each shapedef will also include the datatype for each property. We can then derive the inputType based on it (e.g. FloatT -> range, StringT -> select). The new shapedefs should also define the domain of their values (e.g. arrowheadStyle -> ["arrowhead-1", "arrowhead-2"]). WIth this information, we can generate sample functions for resampling shapes as well.

@wodeni I did consider that, but I opted for extra customization, in case someone thinks a number input is more appropriate than range for their custom property of type FloatT. Also, I think the same types can map to multiple input options; e.g, the option to edit the content of a text should be presented as a text field, but arrowheadStyle should be a select, though both have values of type String. But I think once the backend shapedefs are moved to the front end it could be possible to do what you're suggesting.

Good point. My current thought is to annotate shape properties with types and a range of values, and then make the decision on how to present them. Maybe an extension point within vizdoc to override defaults will give users enough flexibility.

examples/vizdoc/html/heatmap.html

kai-qu · 2020-10-12T23:39:59Z

react-renderer/src/inspector/README.md

@@ -25,8 +25,37 @@ A useful library component is `import { ObjectInspector } from "react-inspector"

 See `ShapeView`, `Timeline`, and `Frames` for useful examples.

+## Adding shape support to Mod view


Thanks for this thorough documentation!

kai-qu · 2020-10-12T23:41:56Z

react-renderer/src/inspector/views/Mod.tsx

+              right: 0,
+            }}
+          >
+            {frame.shapes.map(


This looks like a copy-paste of code from ShapeView to calculate the bbox of a shape. Can you factor it out into Util.tsx and use it in both files?

Fixed in 0e5d3a8

kai-qu · 2020-10-12T23:43:13Z

react-renderer/src/inspector/views/mod/LabeledInput.tsx

+interface IProps {
+    inputProps: IInputProps,
+    eAttr: string;
+    eValue: Value<any>; // is this the best typing?


Note, I should come back to this when merging, not sure if it should be Value<number> or Value<VarAD> (probably the former?)

kai-qu · 2020-10-12T23:45:06Z

react-renderer/src/inspector/views/mod/AttrPicker.tsx

+    properties: object;
+}
+
+// Q - should this update shape properties in state? not really necessary functionally but maybe ideologically


Does this not update shape properties in the state? I expected it to do so, but if not can you say more about how what it is updating?

^Possibly related: my comments in Mod.tsx (github isn't letting me link them directly)

@hypotext It doesn't change the state of that component specifically, just passes up the changes to App.tsx where that state gets modified (which then triggers a re-render, so it does change the state indirectly...)

react-renderer/src/inspector/views/mod/LabeledInput.tsx

kai-qu

LGTM in general, thanks for the awesome work @strout18. If you can address the minor changes I pointed out in comments (forgot to put them in the GitHub review feature...) then we can move along with the merge (pending @maxkrieger's thoughts on the React integration). Thanks!

react-renderer/src/App.tsx

maxkrieger · 2020-10-13T00:15:34Z

react-renderer/src/inspector/views/IViewProps.tsx

@@ -7,5 +7,6 @@ export interface IViewProps {
  frame: State | null;
  // Either a valid index in History
  frameIndex: number;
+  modShapes?(state: IState): void; // todo - null check


why is it nullable?

Not sure... I'll change it.

Addressed in commit 2b9315a

maxkrieger · 2020-10-13T00:23:18Z

react-renderer/src/inspector/views/Mod.tsx

+    if (frameIndex === -1 || frameIndex === history.length - 1) { // make it only work on last frame
+      const shap = frame!.shapes[(selectedShape === -1 ? 0 : selectedShape)]; // the -1 index behaves weirdly so workaround
+      if (shap.properties.hasOwnProperty(attrname)) {
+        shap.properties[attrname] = attrval;


This seems prone to bugs because you're not supposed to mutate properties in-place if they're from props. Props should always be immutable https://reactjs.org/docs/optimizing-performance.html#the-power-of-not-mutating-data.

It shouuuuld be as simple a replacement as:

const shape = frame!.shapes[(selectedShape === -1 ? 0 : selectedShape)]; if (attrname in shape) { const newFrame = {...frame, shapes: {...frame.shapes, [(selectedShape === -1 ? 0 : selectedShape)]: {...shape, properties: {...shape.properties, [attrname]: attrval } } }; modShapes(newFrame); }

@maxkrieger frame.shapes is an array, so that code won't exactly work. I tried replacing it with the following code:

const shapeIndex = selectedShape === -1 ? 0 : selectedShape; const newShapes = _.cloneDeep(frame!.shapes); let shape = newShapes[shapeIndex]; shape.properties[attrname] = attrval; const newFrame = { ...frame, shapes: newShapes } as IState; modShapes(newFrame);

but the compiler didn't want to let me cast to IState.

but the compiler didn't want to let me cast to IState.

Maybe check the type of attrval and see if that conform to IState's types?

Fixed in 0e5d3a8

maxkrieger

Overall looks good, but there are some issues for mod that we'll have to refactor eventually because of readability/immutability/maintainability issues (see inline comments incl those not attached below).

maxkrieger · 2020-10-13T00:25:02Z

react-renderer/src/inspector/views/Mod.tsx

+    const {frame, modShapes, frameIndex, history} = this.props;
+    const {selectedShape} = this.state;
+    if (frameIndex === -1 || frameIndex === history.length - 1) { // make it only work on last frame
+      const shap = frame!.shapes[(selectedShape === -1 ? 0 : selectedShape)]; // the -1 index behaves weirdly so workaround


would be interested in why:

-1 index behaves weirdly

frame! is possibly null: just check if it's null at the top level of the function?

maxkrieger · 2020-10-13T00:25:26Z

react-renderer/src/inspector/views/Mod.tsx

+            boxSizing: "border-box"
+          }}
+        >
+          {/* todo enable only on last frame */}


Expired comment... I'll get it rid of it.

Addressed in 2b9315a

maxkrieger · 2020-10-13T00:29:54Z

react-renderer/src/inspector/views/mod/AttrPicker.tsx

+    properties: object;
+}
+
+// Q - should this update shape properties in state? not really necessary functionally but maybe ideologically


^Possibly related: my comments in Mod.tsx (github isn't letting me link them directly)

react-renderer/src/inspector/views/mod/LabeledInput.tsx

maxkrieger · 2020-10-13T00:37:02Z

react-renderer/src/inspector/views/mod/LabeledInput.tsx

+        this.updateAttr(event.target.id, teval); // a little hacky - id is name of property
+    }
+    // when an input corresponding to a part of an attribute is modified instead of an entire attribute
+    public handleSubChange = (event: React.ChangeEvent<HTMLInputElement>) => {


I have no idea what this does but thats fine I guess

Currently it modifies inputs that map to only part of a shape attribute, e.g. how both the opacity and color input map to the color attribute. I plan on factoring it out.

maxkrieger · 2020-10-13T00:43:27Z

react-renderer/src/inspector/views/mod/LabeledInput.tsx

+    }
+    // when an input corresponding to a part of an attribute is modified instead of an entire attribute
+    public handleSubChange = (event: React.ChangeEvent<HTMLInputElement>) => {
+        const ptinfo = event.target.id.split("_");


Don't encode data into a string and parse it afterwards! There's always, ALWAYS a better way!

Addressed in 2b9315a

react-renderer/src/inspector/views/mod/LabeledInput.tsx

kai-qu · 2020-10-16T21:21:15Z

@strout18 Let us know when this is ready for re-review! (The merge conflicts between this and master look pretty minor, btw, in case you have time to deal with those as well.)

strout18 · 2020-10-20T08:30:28Z

@hypotext This is ready for re-review! Also dealt with the merge conflicts.

kai-qu

LGTM, the heatmap looks fine now, though on trying to re-run your branch (pulling and building) I got this error:

Failed to compile.

./src/inspector/views/mod/LabeledInput.tsx
Module not found: Can't resolve '../../../Util' in '~/Desktop/Code/penrose2/react-renderer/src/inspector/views/mod'

Can you fix this first?

In GitHub web it says there are several merge conflicts, but on CLI I can merge with no problem (which I just did).

kai-qu · 2020-10-26T22:45:19Z

@maxkrieger Can you give Stella's changes a quick re-review? If all looks ok (after Stella fixes the Utils import problem above), I'll merge into master.

strout18 · 2020-10-27T02:52:34Z

The import problem should be fixed in 6153a1b !

wodeni · 2020-10-27T18:13:26Z

penrose-web/src/inspector/views/mod/LabeledInput.tsx

@@ -1,5 +1,5 @@
 import * as React from "react";
-import {toHex} from "../../../Util";
+import {toHex} from "../../../utils/Util"


BTW I changed the module resolution rules and you can just use absolute paths! utils/Util would work just fine.

maxkrieger

LGTM, thanks for fixing this stuff!

kai-qu · 2020-11-11T17:32:58Z

Thanks @strout18 for building this! And @wodeni @maxkrieger for reviewing.

strout18 and others added 6 commits July 22, 2020 21:01

Added shape examples trios to examples/vizdoc

3c8a5ab

[vizdoc] add Box type

5f6b4c4

[web-runtime] [bugfix] skip optimization if there are no varying vari…

6ec7eed

…ables (but still evaluate the new state for label rendering)

fixed square to have strokeWidth, added mod tab to inspector, added e…

e0a5491

…xamples of shapes in vizdoc

Merge remote-tracking branch 'origin/web-runtime' into vizdoc

cae7cfc

removed extraneous line in square definition file

278e264

kai-qu mentioned this pull request Jul 28, 2020

Porting runtime to the web #336

Merged

11 tasks

strout18 added 3 commits August 18, 2020 15:28

made sliders slide more smoothly

55b1b29

pulled from web-runtime

f439b71

added curve support for mod

33ea15a

kai-qu requested review from kai-qu, maxkrieger and wodeni October 10, 2020 20:50

kai-qu reviewed Oct 12, 2020

View reviewed changes

examples/vizdoc/html/heatmap.html Outdated Show resolved Hide resolved

wodeni approved these changes Oct 12, 2020

View reviewed changes

kai-qu reviewed Oct 12, 2020

View reviewed changes

examples/vizdoc/html/heatmap.html Show resolved Hide resolved

kai-qu reviewed Oct 12, 2020

View reviewed changes

react-renderer/src/inspector/views/mod/LabeledInput.tsx Outdated Show resolved Hide resolved

kai-qu suggested changes Oct 13, 2020

View reviewed changes

maxkrieger reviewed Oct 13, 2020

View reviewed changes

react-renderer/src/App.tsx Outdated Show resolved Hide resolved

maxkrieger reviewed Oct 13, 2020

View reviewed changes

maxkrieger requested changes Oct 13, 2020

View reviewed changes

Base automatically changed from web-runtime to master October 13, 2020 19:27

strout18 added 2 commits October 13, 2020 20:03

support for opacity modification

0edabfd

minor refactoring of code per Katherine, Nimo, and Max's suggestions

2b9315a

strout18 added 2 commits October 20, 2020 01:17

refactoring of code in shapeView and Mod

0e5d3a8

merged with master

1b03c70

kai-qu requested review from kai-qu and maxkrieger October 26, 2020 22:41

Merge branch 'master' of github.com:penrose/penrose into vizdoc

8ff2594

kai-qu approved these changes Oct 26, 2020

View reviewed changes

updated import path to Util

6153a1b

wodeni reviewed Oct 27, 2020

View reviewed changes

maxkrieger approved these changes Nov 10, 2020

View reviewed changes

kai-qu merged commit 4a0f0ef into master Nov 11, 2020

kai-qu deleted the vizdoc branch November 11, 2020 17:32

strout18 mentioned this pull request Dec 13, 2020

Front end crashes with "TypeError: Cannot read property 'contents' of undefined" #409

Closed

		@@ -25,8 +25,37 @@ A useful library component is `import { ObjectInspector } from "react-inspector"

		See `ShapeView`, `Timeline`, and `Frames` for useful examples.

		## Adding shape support to Mod view

Extending Penrose viz-à-viz documentation #362

Extending Penrose viz-à-viz documentation #362

Conversation

strout18 commented Jul 23, 2020 • edited by kai-qu Loading

Description

Mod tab on Inspector.

Docshapes

Shape modifier - HTML style

Heatmap

Square hotfix

Remaining issues

maxkrieger commented Jul 24, 2020

wodeni commented Jul 26, 2020

kai-qu commented Jul 28, 2020

kai-qu commented Oct 10, 2020

wodeni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strout18 Oct 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wodeni Oct 12, 2020 • edited by kai-qu Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kai-qu Oct 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kai-qu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxkrieger Oct 13, 2020 • edited Loading

Choose a reason for hiding this comment

strout18 Oct 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxkrieger left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kai-qu commented Oct 16, 2020

strout18 commented Oct 20, 2020

kai-qu left a comment • edited Loading

Choose a reason for hiding this comment

kai-qu commented Oct 26, 2020

strout18 commented Oct 27, 2020 • edited Loading

Choose a reason for hiding this comment

maxkrieger left a comment

Choose a reason for hiding this comment

kai-qu commented Nov 11, 2020

strout18 commented Jul 23, 2020 •

edited by kai-qu

Loading

strout18 Oct 16, 2020 •

edited

Loading

wodeni Oct 12, 2020 •

edited by kai-qu

Loading

kai-qu Oct 12, 2020 •

edited

Loading

maxkrieger Oct 13, 2020 •

edited

Loading

strout18 Oct 16, 2020 •

edited

Loading

maxkrieger left a comment •

edited

Loading

kai-qu left a comment •

edited

Loading

strout18 commented Oct 27, 2020 •

edited

Loading