From 404718c776c8c71e4d5b539efea2d1356d60caa7 Mon Sep 17 00:00:00 2001 From: Enoch Riese Date: Tue, 28 Jun 2022 16:40:01 -0500 Subject: [PATCH] clean up and document --- packages/core/src/part.js | 1 + packages/core/src/pattern.js | 33 ------- .../workbench/layout/draft/index.js | 87 ---------------- .../components/workbench/layout/draft/part.js | 98 +++++++++++++------ 4 files changed, 68 insertions(+), 151 deletions(-) diff --git a/packages/core/src/part.js b/packages/core/src/part.js index 289b2d06a3d..635a1521e92 100644 --- a/packages/core/src/part.js +++ b/packages/core/src/part.js @@ -350,6 +350,7 @@ Part.prototype.shorthand = function () { return shorthand } +/** generate the transform attributes needed for the given part */ export const generatePartTransform = (x, y, rotate, flipX, flipY, part) => { const center = { x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2, diff --git a/packages/core/src/pattern.js b/packages/core/src/pattern.js index 73ab1150fe6..6ad0df971e2 100644 --- a/packages/core/src/pattern.js +++ b/packages/core/src/pattern.js @@ -531,39 +531,6 @@ Pattern.prototype.pack = function () { if (this.parts[partId]) { let transforms = this.settings.layout.parts[partId] this.parts[partId].generateTransform(transforms); - // // Moving - // if (typeof transforms.move === 'object') { - // this.parts[partId].attributes.set( - // 'transform', - // 'translate(' + transforms.move.x + ', ' + transforms.move.y + ')' - // ) - // } - // // Mirrorring - // let center = this.parts[partId].topLeft.shiftFractionTowards( - // this.parts[partId].bottomRight, - // 0.5 - // ) - // let anchor = { x: 0, y: 0 } - // if (transforms.flipX) { - // let dx = anchor.x - center.x - // let transform = `translate(${center.x * -1}, ${center.y * -1})` - // transform += ' scale(-1, 1)' - // transform += ` translate(${center.x * -1 + 2 * dx}, ${center.y})` - // this.parts[partId].attributes.add('transform', transform) - // } - // if (transforms.flipY) { - // let dy = anchor.y - center.y - // let transform = `translate(${center.x * -1}, ${center.y * -1})` - // transform += ' scale(1, -1)' - // transform += ` translate(${center.x}, ${center.y * -1 + 2 * dy})` - // this.parts[partId].attributes.add('transform', transform) - // } - // if (transforms.rotate) { - // let transform = `rotate(${transforms.rotate}, ${center.x - anchor.x}, ${ - // center.y - anchor.y - // })` - // this.parts[partId].attributes.add('transform', transform) - // } } } } diff --git a/sites/shared/components/workbench/layout/draft/index.js b/sites/shared/components/workbench/layout/draft/index.js index ba02626af78..95fe267019d 100644 --- a/sites/shared/components/workbench/layout/draft/index.js +++ b/sites/shared/components/workbench/layout/draft/index.js @@ -1,77 +1,3 @@ -/* - * This React component is a long way from perfect, but it's a start for - * handling custom layouts. - * - * There are two reasons that (at least in my opinion) implementing this is non-trivial: - * - * 1) React re-render vs DOM updates - * - * For performance reasons, we can't re-render with React when the user drags a - * pattern part (or rotates it). It would kill performance. - * So, we don't re-render with React upon dragging/rotating, but instead manipulate - * the DOM directly. - * - * So far so good, but of course we don't want a pattern that's only correctly laid - * out in the DOM. We want to updat the pattern gist so that the new layout is stored. - * For this, we re-render with React on the end of the drag (or rotate). - * - * Handling this balance between DOM updates and React re-renders is a first contributing - * factor to why this component is non-trivial - * - * 2) SVG vs DOM coordinates - * - * When we drag or rotate with the mouse, all the events are giving us coordinates of - * where the mouse is in the DOM. - * - * The layout uses coordinates from the embedded SVG which are completely different. - * So we need to make this translation and that adds complexity. - * - * 3) Part-level transforms - * - * It's not just that the DOM coordinates and SVG coordinate system is different, each - * part also has it's own transforms applied and because of this behaves as if they have - * their own coordinate system. - * - * In other words, a point (0,0) in the part is not the top-left corner of the page. - * In the best-case scenario, it's the top-left corner of the part. But even this is - * often not the case as parts will have transforms applied to them. - * - * 4) Flip along X or Y axis - * - * Parts can be flipped along the X or Y axis to facilitate a custom layout. - * This is handled in a transform, so the part's coordinate's don't actually change. They - * are flipped late into the rendering process (by the browser displaying the SVG). - * - * Handling this adds yet more mental overhead - * - * 5) Bounding box - * - * While moving and rotating parts around is one thing. Recalculating the bounding box - * (think auto-cropping the pattern) gets kinda complicated because of the reasons - * outlined above. - * - * We are currently handling a lot in the frontend code. It might be more elegant to move - * some of this to core. For example, core expects the custom layout to set the widht and height - * rather than figuring it out on its own as it does for auto-generated layouts. - * - * - * - * Known issues - * - * - Rotating gets a little weird sometimes as the part rotates around it's center in the - * SVG coordinate system, but the mouse uses it's own coordinates as the center point that's - * used to calculate the angle of the rotation - * - * - Moving parts into the negative space (minus X or Y coordinated) does not extend the bounding box. - * - * - Rotation gets weird when a part is mirrored - * - * - The bounding box update when a part is rotated is not entirely accurate - * - * - * I've sort of left it at this because I'm starting to wonder if we should perhaps re-think - * how custom layouts are supported in the core. And I would like to discuss this with the core team. - */ import { useEffect, useRef, useState } from 'react' import Svg from '../../draft/svg' import Defs from '../../draft/defs' @@ -89,7 +15,6 @@ const Draft = props => { if (!layout) { // On the initial draft, core does the layout, so we set the layout to the auto-layout // After this, core won't handle layout anymore. It's up to the user from this point onwards - // FIXME: Allow use the option to clear the layout again updateGist(['layout'], { ...patternProps.autoLayout, width: patternProps.width, @@ -104,13 +29,6 @@ const Draft = props => { const updateLayout = (name, config) => { // Start creating new layout const newLayout = {...layout} - const matrix = svgRef.current.getScreenCTM().inverse(); - - // convert topLeft and bottom right from DOM coordinates to svg coordinates - if (config.tl) { - config.tl = DOMPointReadOnly.fromPoint(config.tl).matrixTransform(matrix) - config.br = DOMPointReadOnly.fromPoint(config.br).matrixTransform(matrix); - } newLayout.parts[name] = config // Pattern topLeft and bottomRight @@ -128,15 +46,10 @@ const Draft = props => { } } - // move the pages to the top left corner of the viewBox - // newLayout.parts.pages.move.x = Math.min(0, topLeft.x); - // newLayout.parts.pages.move.y = Math.min(0, topLeft.y) - newLayout.width = bottomRight.x - topLeft.x newLayout.height = bottomRight.y - topLeft.y newLayout.bottomRight = bottomRight newLayout.topLeft = topLeft - console.log(newLayout.topLeft, newLayout.bottomRight); updateGist(['layout'], newLayout) } diff --git a/sites/shared/components/workbench/layout/draft/part.js b/sites/shared/components/workbench/layout/draft/part.js index bf0a84248f4..07bad4d255e 100644 --- a/sites/shared/components/workbench/layout/draft/part.js +++ b/sites/shared/components/workbench/layout/draft/part.js @@ -1,3 +1,48 @@ +/* + * This React component is a long way from perfect, but it's a start for + * handling custom layouts. + * + * There are a few reasons that (at least in my opinion) implementing this is non-trivial: + * + * 1) React re-render vs DOM updates + * + * For performance reasons, we can't re-render with React when the user drags a + * pattern part (or rotates it). It would kill performance. + * So, we don't re-render with React upon dragging/rotating, but instead manipulate + * the DOM directly. + * + * So far so good, but of course we don't want a pattern that's only correctly laid + * out in the DOM. We want to update the pattern gist so that the new layout is stored. + * For this, we re-render with React on the end of the drag (or rotate). + * + * Handling this balance between DOM updates and React re-renders is a first contributing + * factor to why this component is non-trivial + * + * 2) SVG vs DOM coordinates + * + * When we drag or rotate with the mouse, all the events are giving us coordinates of + * where the mouse is in the DOM. + * + * The layout uses coordinates from the embedded SVG which are completely different. + * + * We run `getScreenCTM().inverse()` on the svg element to pass to `matrixTransform` on a `DOMPointReadOnly` for dom to svg space conversions. + * + * 3) Part-level transforms + * + * All parts use their center as the transform-origin to simplify transforms, especially flipping and rotating. + * + * 4) Bounding box + * + * We use `getBoundingClientRect` rather than `getBBox` because it provides more data and factors in the transforms. + * We then use our `domToSvg` function to move the points back into the SVG space. + * + * + * Known issues + * - currently none + * + * I've sort of left it at this because I'm starting to wonder if we should perhaps re-think + * how custom layouts are supported in the core. And I would like to discuss this with the core team. + */ import Path from '../../draft/path' import Point from '../../draft/point' import Snippet from '../../draft/snippet' @@ -8,6 +53,7 @@ import { drag } from 'd3-drag' import { select } from 'd3-selection' import { useRef, useState, useEffect} from 'react' +/** buttons for manipulating the part */ const Buttons = ({ transform, flip, rotate, setRotate, resetPart }) => { const letter = 'F' const style = { style: {fill: 'white', fontSize: 18, fontWeight: 'bold', textAnchor: 'middle'} } @@ -34,27 +80,6 @@ const Buttons = ({ transform, flip, rotate, setRotate, resetPart }) => { ) } -const generateTransform = (x, y, rot, flipX, flipY, part) => { - // const center = { - // x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2, - // y: part.topLeft.y + (part.bottomRight.y - part.topLeft.y)/2, - // } - // const dx = part.topLeft.x - center.x - // const dy = part.topLeft.y - center.y - const transforms = [`translate(${x},${y})`] - if (flipX) transforms.push( - 'scale(-1, 1)', - ) - if (flipY) transforms.push( - 'scale(1, -1)', - ) - if (rot) transforms.push( - `rotate(${rot}, ${center.x}, ${center.y})` - ) - - return transforms.join(' ') -} - const Part = props => { const { layout, gist, name, part} = props @@ -87,29 +112,33 @@ const Part = props => { let translateY = partLayout.move.y let partRotation = partLayout.rotate || 0; let rotation = partRotation; - let flipX = partLayout.flipX ? true : false - let flipY = partLayout.flipY ? true : false - // let partRect + let flipX = !!partLayout.flipX + let flipY = !!partLayout.flipY const center = { x: part.topLeft.x + (part.bottomRight.x - part.topLeft.x)/2, y: part.topLeft.y + (part.bottomRight.y - part.topLeft.y)/2, } + /** get the delta rotation from the start of the drag event to now */ const getRotation = (event) => angle(center, event.subject) - angle(center, { x:event.x, y: event.y }); const handleDrag = drag() + // subject allows us to save data from the start of the event to use throughout event handing .subject(function(event) { - return rotate ? { x: event.x, y: event.y } : {x: translateX, y: translateY} - }) - .on('start', function(event) { - // partRect = partRef.current.getBoundingClientRect() + return rotate ? + // if we're rotating, the subject is the mouse position + { x: event.x, y: event.y } : + // if we're moving, the subject is the part's x,y coordinates + {x: translateX, y: translateY} }) .on('drag', function(event) { if (rotate) { let newRotation = getRotation(event); + // reverse the rotation direction one time per flip. if we're flipped both directions, rotation will be positive again if (flipX) newRotation *= -1 if (flipY) newRotation *= -1 + rotation = partRotation + newRotation } else { @@ -117,14 +146,16 @@ const Part = props => { translateY = event.y } + // get the transform attributes const transforms = generatePartTransform(translateX, translateY, rotation, flipX, flipY, part); - console.log(transforms) + const me = select(this); for (var t in transforms) { me.attr(t, transforms[t]) } }) .on('end', function(event) { + // save to gist updateLayout() }) @@ -140,8 +171,13 @@ const Part = props => { } const updateLayout = () => { const partRect = partRef.current.getBoundingClientRect(); - const tl = {x: partRect.left, y: partRect.top}; - const br = {x: partRect.right, y: partRect.bottom}; + const matrix = partRef.current.ownerSVGElement.getScreenCTM().inverse(); + + const domToSvg = (point) => DOMPointReadOnly.fromPoint(point).matrixTransform(matrix) + + // include the new top left and bottom right to ease calculating the pattern width and height + const tl = domToSvg({x: partRect.left, y: partRect.top}); + const br = domToSvg({x: partRect.right, y: partRect.bottom}); props.updateLayout(name, { move: {