Refactor Element Functions (#233)

* Remove `generatedraw` from element object

- Create a function that renders a single element
- Refactor rendering selected elements

* Replace getElementAbsoluteXY with getElementAbsoluteCoords

src/element/bounds.test.ts

@@ -0,0 +1,67 @@
+import { getElementAbsoluteCoords } from "./bounds";
+import { ExcalidrawElement } from "./types";
+
+const _ce = ({ x, y, w, h }: { x: number; y: number; w: number; h: number }) =>
+  ({
+    type: "test",
+    strokeColor: "#000",
+    backgroundColor: "#000",
+    fillStyle: "solid",
+    strokeWidth: 1,
+    roughness: 1,
+    opacity: 1,
+    x,
+    y,
+    width: w,
+    height: h
+  } as ExcalidrawElement);
+
+describe("getElementAbsoluteCoords", () => {
+  it("test x1 coordinate if width is positive or zero", () => {
+    const [x1] = getElementAbsoluteCoords(_ce({ x: 10, y: 0, w: 10, h: 0 }));
+    expect(x1).toEqual(10);
+  });
+
+  it("test x1 coordinate if width is negative", () => {
+    const [x1] = getElementAbsoluteCoords(_ce({ x: 20, y: 0, w: -10, h: 0 }));
+    expect(x1).toEqual(10);
+  });
+
+  it("test x2 coordinate if width is positive or zero", () => {
+    const [, , x2] = getElementAbsoluteCoords(
+      _ce({ x: 10, y: 0, w: 10, h: 0 })
+    );
+    expect(x2).toEqual(20);
+  });
+
+  it("test x2 coordinate if width is negative", () => {
+    const [, , x2] = getElementAbsoluteCoords(
+      _ce({ x: 10, y: 0, w: -10, h: 0 })
+    );
+    expect(x2).toEqual(10);
+  });
+
+  it("test y1 coordinate if height is positive or zero", () => {
+    const [, y1] = getElementAbsoluteCoords(_ce({ x: 0, y: 10, w: 0, h: 10 }));
+    expect(y1).toEqual(10);
+  });
+
+  it("test y1 coordinate if height is negative", () => {
+    const [, y1] = getElementAbsoluteCoords(_ce({ x: 0, y: 20, w: 0, h: -10 }));
+    expect(y1).toEqual(10);
+  });
+
+  it("test y2 coordinate if height is positive or zero", () => {
+    const [, , , y2] = getElementAbsoluteCoords(
+      _ce({ x: 0, y: 10, w: 0, h: 10 })
+    );
+    expect(y2).toEqual(20);
+  });
+
+  it("test y2 coordinate if height is negative", () => {
+    const [, , , y2] = getElementAbsoluteCoords(
+      _ce({ x: 0, y: 10, w: 0, h: -10 })
+    );
+    expect(y2).toEqual(10);
+  });
+});

src/element/bounds.ts

@@ -5,17 +5,13 @@ import { rotate } from "../math";
 // This set of functions retrieves the absolute position of the 4 points.
 // We can't just always normalize it since we need to remember the fact that an arrow
 // is pointing left or right.
-export function getElementAbsoluteX1(element: ExcalidrawElement) {
-  return element.width >= 0 ? element.x : element.x + element.width;
-}
-export function getElementAbsoluteX2(element: ExcalidrawElement) {
-  return element.width >= 0 ? element.x + element.width : element.x;
-}
-export function getElementAbsoluteY1(element: ExcalidrawElement) {
-  return element.height >= 0 ? element.y : element.y + element.height;
-}
-export function getElementAbsoluteY2(element: ExcalidrawElement) {
-  return element.height >= 0 ? element.y + element.height : element.y;
+export function getElementAbsoluteCoords(element: ExcalidrawElement) {
+  return [
+    element.width >= 0 ? element.x : element.x + element.width, // x1
+    element.height >= 0 ? element.y : element.y + element.height, // y1
+    element.width >= 0 ? element.x + element.width : element.x, // x2
+    element.height >= 0 ? element.y + element.height : element.y // y2
+  ];
 }
 
 export function getDiamondPoints(element: ExcalidrawElement) {

src/element/collision.ts

@@ -2,12 +2,9 @@ import { distanceBetweenPointAndSegment } from "../math";
 
 import { ExcalidrawElement } from "./types";
 import {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2,
   getArrowPoints,
-  getDiamondPoints
+  getDiamondPoints,
+  getElementAbsoluteCoords
 } from "./bounds";
 
 export function hitTest(
@@ -55,10 +52,7 @@ export function hitTest(
 
     return Math.hypot(a * tx - px, b * ty - py) < lineThreshold;
   } else if (element.type === "rectangle") {
-    const x1 = getElementAbsoluteX1(element);
-    const x2 = getElementAbsoluteX2(element);
-    const y1 = getElementAbsoluteY1(element);
-    const y2 = getElementAbsoluteY2(element);
+    const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
 
     // (x1, y1) --A-- (x2, y1)
     //    |D             |B
@@ -109,10 +103,7 @@ export function hitTest(
       distanceBetweenPointAndSegment(x, y, x4, y4, x2, y2) < lineThreshold
     );
   } else if (element.type === "text") {
-    const x1 = getElementAbsoluteX1(element);
-    const x2 = getElementAbsoluteX2(element);
-    const y1 = getElementAbsoluteY1(element);
-    const y2 = getElementAbsoluteY2(element);
+    const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
 
     return x >= x1 && x <= x2 && y >= y1 && y <= y2;
   } else if (element.type === "selection") {

src/element/index.ts

@@ -1,9 +1,6 @@
 export { newElement } from "./newElement";
 export {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2,
+  getElementAbsoluteCoords,
   getDiamondPoints,
   getArrowPoints
 } from "./bounds";
@@ -11,5 +8,4 @@ export {
 export { handlerRectangles } from "./handlerRectangles";
 export { hitTest } from "./collision";
 export { resizeTest } from "./resizeTest";
-export { generateDraw } from "./generateDraw";
 export { isTextElement } from "./typeChecks";

src/element/newElement.ts

@@ -1,6 +1,3 @@
-import { RoughCanvas } from "roughjs/bin/canvas";
-
-import { SceneState } from "../scene/types";
 import { randomSeed } from "../random";
 
 export function newElement(
@@ -17,24 +14,19 @@ export function newElement(
   height = 0
 ) {
   const element = {
-    type: type,
-    x: x,
-    y: y,
-    width: width,
-    height: height,
+    type,
+    x,
+    y,
+    width,
+    height,
+    strokeColor,
+    backgroundColor,
+    fillStyle,
+    strokeWidth,
+    roughness,
+    opacity,
     isSelected: false,
-    strokeColor: strokeColor,
-    backgroundColor: backgroundColor,
-    fillStyle: fillStyle,
-    strokeWidth: strokeWidth,
-    roughness: roughness,
-    opacity: opacity,
-    seed: randomSeed(),
-    draw(
-      rc: RoughCanvas,
-      context: CanvasRenderingContext2D,
-      sceneState: SceneState
-    ) {}
+    seed: randomSeed()
   };
   return element;
 }

src/history.ts

@@ -1,5 +1,4 @@
 import { ExcalidrawElement } from "./element/types";
-import { generateDraw } from "./element";
 
 class SceneHistory {
   private recording: boolean = true;
@@ -27,7 +26,6 @@ class SceneHistory {
     const newElements = JSON.parse(entry);
     elements.splice(0, elements.length);
     newElements.forEach((newElement: ExcalidrawElement) => {
-      generateDraw(newElement);
       elements.push(newElement);
     });
     // When restoring, we shouldn't add an history entry otherwise we'll be stuck with it and can't go back

src/index.tsx

@@ -4,9 +4,8 @@ import rough from "roughjs/bin/wrappers/rough";
 
 import { moveOneLeft, moveAllLeft, moveOneRight, moveAllRight } from "./zindex";
 import { randomSeed } from "./random";
-import { newElement, resizeTest, generateDraw, isTextElement } from "./element";
+import { newElement, resizeTest, isTextElement } from "./element";
 import {
-  renderScene,
   clearSelection,
   getSelectedIndices,
   deleteSelectedElements,
@@ -26,6 +25,8 @@ import {
   getElementAtPosition,
   createScene
 } from "./scene";
+
+import { renderScene } from "./renderer";
 import { AppState } from "./types";
 import { ExcalidrawElement, ExcalidrawTextElement } from "./element/types";
 
@@ -267,7 +268,6 @@ class App extends React.Component<{}, AppState> {
         element.fillStyle = pastedElement?.fillStyle;
         element.opacity = pastedElement?.opacity;
         element.roughness = pastedElement?.roughness;
-        generateDraw(element);
       }
     });
     this.forceUpdate();
@@ -303,7 +303,6 @@ class App extends React.Component<{}, AppState> {
     elements.forEach(element => {
       if (element.isSelected) {
         callback(element);
-        generateDraw(element);
       }
     });
 
@@ -697,7 +696,6 @@ class App extends React.Component<{}, AppState> {
               }
             }
 
-            generateDraw(element);
             elements.push(element);
             if (this.state.elementType === "text") {
               this.setState({
@@ -805,7 +803,6 @@ class App extends React.Component<{}, AppState> {
 
                     el.x = element.x;
                     el.y = element.y;
-                    generateDraw(el);
                   });
                   lastX = x;
                   lastY = y;
@@ -856,8 +853,6 @@ class App extends React.Component<{}, AppState> {
                 ? Math.abs(width) * Math.sign(height)
                 : height;
 
-              generateDraw(draggingElement);
-
               if (this.state.elementType === "selection") {
                 setSelection(elements, draggingElement);
               }
@@ -932,7 +927,6 @@ class App extends React.Component<{}, AppState> {
               return;
             }
 
-            generateDraw(element);
             elements.push(element);
 
             this.setState({
@@ -989,7 +983,6 @@ class App extends React.Component<{}, AppState> {
         parsedElement.x = dx ? parsedElement.x + dx : 10 - this.state.scrollX;
         parsedElement.y = dy ? parsedElement.y + dy : 10 - this.state.scrollY;
         parsedElement.seed = randomSeed();
-        generateDraw(parsedElement);
         elements.push(parsedElement);
       });
       this.forceUpdate();

src/renderer/index.ts

@@ -0,0 +1 @@
+export { renderScene } from "./renderScene";

src/renderer/renderElement.ts

@@ -2,27 +2,32 @@ import rough from "roughjs/bin/wrappers/rough";
 
 import { withCustomMathRandom } from "../random";
 
-import { ExcalidrawElement } from "./types";
-import { isTextElement } from "./typeChecks";
-import { getDiamondPoints, getArrowPoints } from "./bounds";
+import { ExcalidrawElement } from "../element/types";
+import { isTextElement } from "../element/typeChecks";
+import { getDiamondPoints, getArrowPoints } from "../element/bounds";
+import { RoughCanvas } from "roughjs/bin/canvas";
+import { SceneState } from "../scene/types";
 
 // Casting second argument (DrawingSurface) to any,
 // because it is requred by TS definitions and not required at runtime
 const generator = rough.generator(null, null as any);
 
-export function generateDraw(element: ExcalidrawElement) {
+export function renderElement(
+  element: ExcalidrawElement,
+  rc: RoughCanvas,
+  context: CanvasRenderingContext2D,
+  { scrollX, scrollY }: SceneState
+) {
   if (element.type === "selection") {
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      const fillStyle = context.fillStyle;
-      context.fillStyle = "rgba(0, 0, 255, 0.10)";
-      context.fillRect(
-        element.x + scrollX,
-        element.y + scrollY,
-        element.width,
-        element.height
-      );
-      context.fillStyle = fillStyle;
-    };
+    const fillStyle = context.fillStyle;
+    context.fillStyle = "rgba(0, 0, 255, 0.10)";
+    context.fillRect(
+      element.x + scrollX,
+      element.y + scrollY,
+      element.width,
+      element.height
+    );
+    context.fillStyle = fillStyle;
   } else if (element.type === "rectangle") {
     const shape = withCustomMathRandom(element.seed, () => {
       return generator.rectangle(0, 0, element.width, element.height, {
@@ -33,13 +38,12 @@ export function generateDraw(element: ExcalidrawElement) {
         roughness: element.roughness
       });
     });
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      context.globalAlpha = element.opacity / 100;
-      context.translate(element.x + scrollX, element.y + scrollY);
-      rc.draw(shape);
-      context.translate(-element.x - scrollX, -element.y - scrollY);
-      context.globalAlpha = 1;
-    };
+
+    context.globalAlpha = element.opacity / 100;
+    context.translate(element.x + scrollX, element.y + scrollY);
+    rc.draw(shape);
+    context.translate(-element.x - scrollX, -element.y - scrollY);
+    context.globalAlpha = 1;
   } else if (element.type === "diamond") {
     const shape = withCustomMathRandom(element.seed, () => {
       const [
@@ -68,13 +72,11 @@ export function generateDraw(element: ExcalidrawElement) {
         }
       );
     });
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      context.globalAlpha = element.opacity / 100;
-      context.translate(element.x + scrollX, element.y + scrollY);
-      rc.draw(shape);
-      context.translate(-element.x - scrollX, -element.y - scrollY);
-      context.globalAlpha = 1;
-    };
+    context.globalAlpha = element.opacity / 100;
+    context.translate(element.x + scrollX, element.y + scrollY);
+    rc.draw(shape);
+    context.translate(-element.x - scrollX, -element.y - scrollY);
+    context.globalAlpha = 1;
   } else if (element.type === "ellipse") {
     const shape = withCustomMathRandom(element.seed, () =>
       generator.ellipse(
@@ -91,13 +93,12 @@ export function generateDraw(element: ExcalidrawElement) {
         }
       )
     );
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      context.globalAlpha = element.opacity / 100;
-      context.translate(element.x + scrollX, element.y + scrollY);
-      rc.draw(shape);
-      context.translate(-element.x - scrollX, -element.y - scrollY);
-      context.globalAlpha = 1;
-    };
+
+    context.globalAlpha = element.opacity / 100;
+    context.translate(element.x + scrollX, element.y + scrollY);
+    rc.draw(shape);
+    context.translate(-element.x - scrollX, -element.y - scrollY);
+    context.globalAlpha = 1;
   } else if (element.type === "arrow") {
     const [x1, y1, x2, y2, x3, y3, x4, y4] = getArrowPoints(element);
     const options = {
@@ -115,30 +116,26 @@ export function generateDraw(element: ExcalidrawElement) {
       generator.line(x4, y4, x2, y2, options)
     ]);
 
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      context.globalAlpha = element.opacity / 100;
-      context.translate(element.x + scrollX, element.y + scrollY);
-      shapes.forEach(shape => rc.draw(shape));
-      context.translate(-element.x - scrollX, -element.y - scrollY);
-      context.globalAlpha = 1;
-    };
+    context.globalAlpha = element.opacity / 100;
+    context.translate(element.x + scrollX, element.y + scrollY);
+    shapes.forEach(shape => rc.draw(shape));
+    context.translate(-element.x - scrollX, -element.y - scrollY);
+    context.globalAlpha = 1;
     return;
   } else if (isTextElement(element)) {
-    element.draw = (rc, context, { scrollX, scrollY }) => {
-      context.globalAlpha = element.opacity / 100;
-      const font = context.font;
-      context.font = element.font;
-      const fillStyle = context.fillStyle;
-      context.fillStyle = element.strokeColor;
-      context.fillText(
-        element.text,
-        element.x + scrollX,
-        element.y + element.actualBoundingBoxAscent + scrollY
-      );
-      context.fillStyle = fillStyle;
-      context.font = font;
-      context.globalAlpha = 1;
-    };
+    context.globalAlpha = element.opacity / 100;
+    const font = context.font;
+    context.font = element.font;
+    const fillStyle = context.fillStyle;
+    context.fillStyle = element.strokeColor;
+    context.fillText(
+      element.text,
+      element.x + scrollX,
+      element.y + element.actualBoundingBoxAscent + scrollY
+    );
+    context.fillStyle = fillStyle;
+    context.font = font;
+    context.globalAlpha = 1;
   } else {
     throw new Error("Unimplemented type " + element.type);
   }

src/renderer/renderScene.ts

@@ -1,18 +1,17 @@
 import { RoughCanvas } from "roughjs/bin/canvas";
 
 import { ExcalidrawElement } from "../element/types";
-import {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2,
-  handlerRectangles
-} from "../element";
+import { getElementAbsoluteCoords, handlerRectangles } from "../element";
 
-import { roundRect } from "./roundRect";
-import { SceneState } from "./types";
-import { getScrollBars, SCROLLBAR_COLOR, SCROLLBAR_WIDTH } from "./scrollbars";
-import { getSelectedIndices } from "./selection";
+import { roundRect } from "../scene/roundRect";
+import { SceneState } from "../scene/types";
+import {
+  getScrollBars,
+  SCROLLBAR_COLOR,
+  SCROLLBAR_WIDTH
+} from "../scene/scrollbars";
+
+import { renderElement } from "./renderElement";
 
 export function renderScene(
   elements: ExcalidrawElement[],
@@ -44,8 +43,6 @@ export function renderScene(
   }
   context.fillStyle = fillStyle;
 
-  const selectedIndices = getSelectedIndices(elements);
-
   sceneState = {
     ...sceneState,
     scrollX: typeof offsetX === "number" ? offsetX : sceneState.scrollX,
@@ -53,14 +50,21 @@ export function renderScene(
   };
 
   elements.forEach(element => {
-    element.draw(rc, context, sceneState);
-    if (renderSelection && element.isSelected) {
+    renderElement(element, rc, context, sceneState);
+  });
+
+  if (renderSelection) {
+    const selectedElements = elements.filter(el => el.isSelected);
+
+    selectedElements.forEach(element => {
       const margin = 4;
 
-      const elementX1 = getElementAbsoluteX1(element);
-      const elementX2 = getElementAbsoluteX2(element);
-      const elementY1 = getElementAbsoluteY1(element);
-      const elementY2 = getElementAbsoluteY2(element);
+      const [
+        elementX1,
+        elementY1,
+        elementX2,
+        elementY2
+      ] = getElementAbsoluteCoords(element);
       const lineDash = context.getLineDash();
       context.setLineDash([8, 4]);
       context.strokeRect(
@@ -70,15 +74,15 @@ export function renderScene(
         elementY2 - elementY1 + margin * 2
       );
       context.setLineDash(lineDash);
+    });
 
-      if (element.type !== "text" && selectedIndices.length === 1) {
-        const handlers = handlerRectangles(element, sceneState);
-        Object.values(handlers).forEach(handler => {
-          context.strokeRect(handler[0], handler[1], handler[2], handler[3]);
-        });
-      }
+    if (selectedElements.length === 1 && selectedElements[0].type !== "text") {
+      const handlers = handlerRectangles(selectedElements[0], sceneState);
+      Object.values(handlers).forEach(handler => {
+        context.strokeRect(handler[0], handler[1], handler[2], handler[3]);
+      });
     }
-  });
+  }
 
   if (renderScrollbars) {
     const scrollBars = getScrollBars(

src/scene/data.ts

@@ -2,15 +2,9 @@ import rough from "roughjs/bin/wrappers/rough";
 
 import { ExcalidrawElement } from "../element/types";
 
-import {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2,
-  generateDraw
-} from "../element";
+import { getElementAbsoluteCoords } from "../element";
 
-import { renderScene } from "./render";
+import { renderScene } from "../renderer";
 import { AppState } from "../types";
 
 const LOCAL_STORAGE_KEY = "excalidraw";
@@ -94,10 +88,11 @@ export function exportAsPNG(
   let subCanvasY2 = 0;
 
   elements.forEach(element => {
-    subCanvasX1 = Math.min(subCanvasX1, getElementAbsoluteX1(element));
-    subCanvasX2 = Math.max(subCanvasX2, getElementAbsoluteX2(element));
-    subCanvasY1 = Math.min(subCanvasY1, getElementAbsoluteY1(element));
-    subCanvasY2 = Math.max(subCanvasY2, getElementAbsoluteY2(element));
+    const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
+    subCanvasX1 = Math.min(subCanvasX1, x1);
+    subCanvasY1 = Math.min(subCanvasY1, y1);
+    subCanvasX2 = Math.max(subCanvasX2, x2);
+    subCanvasY2 = Math.max(subCanvasY2, y2);
   });
 
   function distance(x: number, y: number) {
@@ -155,8 +150,6 @@ function restore(
           element.opacity === null || element.opacity === undefined
             ? 100
             : element.opacity;
-
-        generateDraw(element);
       });
     }
 

src/scene/index.ts

@@ -1,5 +1,4 @@
 export { isOverScrollBars } from "./scrollbars";
-export { renderScene } from "./render";
 export {
   clearSelection,
   getSelectedIndices,

src/scene/scrollbars.ts

@@ -1,10 +1,5 @@
 import { ExcalidrawElement } from "../element/types";
-import {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2
-} from "../element";
+import { getElementAbsoluteCoords } from "../element";
 
 const SCROLLBAR_MIN_SIZE = 15;
 const SCROLLBAR_MARGIN = 4;
@@ -24,10 +19,11 @@ export function getScrollBars(
   let maxY = 0;
 
   elements.forEach(element => {
-    minX = Math.min(minX, getElementAbsoluteX1(element));
-    maxX = Math.max(maxX, getElementAbsoluteX2(element));
-    minY = Math.min(minY, getElementAbsoluteY1(element));
-    maxY = Math.max(maxY, getElementAbsoluteY2(element));
+    const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
+    minX = Math.min(minX, x1);
+    minY = Math.min(minY, y1);
+    maxX = Math.max(maxX, x2);
+    maxY = Math.max(maxY, y2);
   });
 
   minX += scrollX;

src/scene/selection.ts

@@ -1,24 +1,23 @@
 import { ExcalidrawElement } from "../element/types";
-import {
-  getElementAbsoluteX1,
-  getElementAbsoluteX2,
-  getElementAbsoluteY1,
-  getElementAbsoluteY2
-} from "../element";
+import { getElementAbsoluteCoords } from "../element";
 
 export function setSelection(
   elements: ExcalidrawElement[],
   selection: ExcalidrawElement
 ) {
-  const selectionX1 = getElementAbsoluteX1(selection);
-  const selectionX2 = getElementAbsoluteX2(selection);
-  const selectionY1 = getElementAbsoluteY1(selection);
-  const selectionY2 = getElementAbsoluteY2(selection);
+  const [
+    selectionX1,
+    selectionY1,
+    selectionX2,
+    selectionY2
+  ] = getElementAbsoluteCoords(selection);
   elements.forEach(element => {
-    const elementX1 = getElementAbsoluteX1(element);
-    const elementX2 = getElementAbsoluteX2(element);
-    const elementY1 = getElementAbsoluteY1(element);
-    const elementY2 = getElementAbsoluteY2(element);
+    const [
+      elementX1,
+      elementY1,
+      elementX2,
+      elementY2
+    ] = getElementAbsoluteCoords(element);
     element.isSelected =
       element.type !== "selection" &&
       selectionX1 <= elementX1 &&