* Fix rotation recalculation corrupting compound transforms Two bugs in the rotation center recalculation triggered by attribute changes on rotated elements: 1. The recalculation fires for ALL attribute changes (stroke-width, fill, opacity, etc.) even though only geometric attributes (x, y, width, height, d, points, etc.) can change the bbox center. This causes unnecessary and incorrect rotation center updates. 2. In undo.js, the center is computed through ALL remaining transforms via transformListToTransform(tlist).matrix, but for compound transform lists like [translate(tx,ty), rotate(angle)], the pre-rotation translate leaks into the center calculation, producing rotate(angle, bcx+tx, bcy+ty) instead of the correct rotate(angle, bcx, bcy). 3. In history.js (undo/redo), the code replaces the ENTIRE transform attribute with just rotate(...), completely destroying any other transforms (translate, scale, etc.) in the list. Fixes: - Guard recalculation with a BBOX_AFFECTING_ATTRS set so non-geometric attribute changes skip the rotation block entirely. - In undo.js, use transformListToTransform(tlist, n, max) to compute the center through only post-rotation transforms. - In history.js, replace string-based transform replacement with transform list API that finds and updates only the rotation entry, preserving all other transforms. Extract shared logic into relocateRotationCenter() helper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add tests for rotation recalculation with compound transforms Tests demonstrate and verify fixes for three bugs: 1. Non-geometric attributes (stroke-width, fill, opacity) no longer trigger rotation center recalculation — the transform list is left untouched. 2. Geometric attribute changes on elements with compound transforms (e.g. translate + rotate) compute the rotation center using only post-rotation transforms, preventing the translate from leaking into the center calculation. 3. ChangeElementCommand.apply/unapply preserve compound transform structure instead of replacing the entire transform attribute with just rotate(...). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add visual demo and screenshot for rotation recalculation bug Includes an HTML demo showing both bugs side-by-side: - Bug 1: Non-geometric attribute changes (stroke-width) on compound transforms corrupt character positions (e.g., word art on curves) - Bug 2: Rotation center computed through ALL transforms instead of only post-rotation transforms, causing translate to leak into center Each panel shows Original / Buggy / Fixed comparisons with the actual SVG transform math applied. A Playwright script generates screenshots. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add sample SVG demonstrating compound transform corruption bug A pinwheel of 6 colored rectangles using translate(200,200) rotate(N). Loading this into SVG-Edit and changing stroke-width on the group triggers the rotation recalculation bug on unfixed builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix standard linting: object properties on separate lines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Centralize BBOX_AFFECTING_ATTRS and fix bbox-before-remove safety Address two code review comments: 1. Export BBOX_AFFECTING_ATTRS from history.js and import it in undo.js so the attribute list is defined in one place. 2. Compute getBBox() before calling tlist.removeItem() so that a falsy bbox causes an early return/break without having already destroyed the rotation transform. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
309 lines
12 KiB
JavaScript
309 lines
12 KiB
JavaScript
/**
|
|
* Tools for undo.
|
|
* @module undo
|
|
* @license MIT
|
|
* @copyright 2011 Jeff Schiller
|
|
*/
|
|
import * as draw from './draw.js'
|
|
import * as hstry from './history.js'
|
|
import { BBOX_AFFECTING_ATTRS } from './history.js'
|
|
import {
|
|
getRotationAngle, getBBox as utilsGetBBox, setHref, getStrokedBBoxDefaultVisible
|
|
} from './utilities.js'
|
|
import {
|
|
isGecko
|
|
} from '../common/browser.js'
|
|
import {
|
|
transformPoint, transformListToTransform, getTransformList
|
|
} from './math.js'
|
|
|
|
const {
|
|
UndoManager, HistoryEventTypes
|
|
} = hstry
|
|
|
|
let svgCanvas = null
|
|
|
|
/**
|
|
* @function module:undo.init
|
|
* @param {module:undo.undoContext} undoContext
|
|
* @returns {void}
|
|
*/
|
|
export const init = (canvas) => {
|
|
svgCanvas = canvas
|
|
canvas.undoMgr = getUndoManager()
|
|
}
|
|
|
|
export const getUndoManager = () => {
|
|
return new UndoManager({
|
|
/**
|
|
* @param {string} eventType One of the HistoryEvent types
|
|
* @param {module:history.HistoryCommand} cmd Fulfills the HistoryCommand interface
|
|
* @fires module:undo.SvgCanvas#event:changed
|
|
* @returns {void}
|
|
*/
|
|
handleHistoryEvent (eventType, cmd) {
|
|
const EventTypes = HistoryEventTypes
|
|
// TODO: handle setBlurOffsets.
|
|
if (eventType === EventTypes.BEFORE_UNAPPLY || eventType === EventTypes.BEFORE_APPLY) {
|
|
svgCanvas.clearSelection()
|
|
} else if (eventType === EventTypes.AFTER_APPLY || eventType === EventTypes.AFTER_UNAPPLY) {
|
|
const cmdType = cmd.type()
|
|
const isApply = (eventType === EventTypes.AFTER_APPLY)
|
|
if (cmdType === 'ChangeElementCommand' && cmd.elem === svgCanvas.getSvgContent()) {
|
|
const values = isApply ? cmd.newValues : cmd.oldValues
|
|
if (values.width !== null && values.width !== undefined && values.width !== '') {
|
|
const newContentW = Number(values.width)
|
|
if (Number.isFinite(newContentW) && newContentW > 0) {
|
|
svgCanvas.contentW = newContentW
|
|
}
|
|
}
|
|
if (values.height !== null && values.height !== undefined && values.height !== '') {
|
|
const newContentH = Number(values.height)
|
|
if (Number.isFinite(newContentH) && newContentH > 0) {
|
|
svgCanvas.contentH = newContentH
|
|
}
|
|
}
|
|
}
|
|
const elems = cmd.elements()
|
|
svgCanvas.pathActions.clear()
|
|
svgCanvas.call('changed', elems)
|
|
if (cmdType === 'MoveElementCommand') {
|
|
const parent = isApply ? cmd.newParent : cmd.oldParent
|
|
if (parent === svgCanvas.getSvgContent()) {
|
|
draw.identifyLayers()
|
|
}
|
|
} else if (cmdType === 'InsertElementCommand' || cmdType === 'RemoveElementCommand') {
|
|
if (cmd.parent === svgCanvas.getSvgContent()) {
|
|
draw.identifyLayers()
|
|
}
|
|
if (cmdType === 'InsertElementCommand') {
|
|
if (isApply) {
|
|
svgCanvas.restoreRefElements(cmd.elem)
|
|
}
|
|
} else if (!isApply) {
|
|
svgCanvas.restoreRefElements(cmd.elem)
|
|
}
|
|
if (cmd.elem?.tagName === 'use') {
|
|
svgCanvas.setUseData(cmd.elem)
|
|
}
|
|
} else if (cmdType === 'ChangeElementCommand') {
|
|
// if we are changing layer names, re-identify all layers
|
|
if (cmd.elem.tagName === 'title' &&
|
|
cmd.elem.parentNode.parentNode === svgCanvas.getSvgContent()
|
|
) {
|
|
draw.identifyLayers()
|
|
}
|
|
const values = isApply ? cmd.newValues : cmd.oldValues
|
|
// If stdDeviation was changed, update the blur.
|
|
if (values.stdDeviation) {
|
|
svgCanvas.setBlurOffsets(cmd.elem.parentNode, values.stdDeviation)
|
|
}
|
|
if (cmd.elem.tagName === 'text') {
|
|
const [dx, dy] = [cmd.newValues.x - cmd.oldValues.x,
|
|
cmd.newValues.y - cmd.oldValues.y]
|
|
|
|
const tspans = cmd.elem.children
|
|
|
|
for (let i = 0; i < tspans.length; i++) {
|
|
let x = Number(tspans[i].getAttribute('x'))
|
|
let y = Number(tspans[i].getAttribute('y'))
|
|
|
|
const unapply = (eventType === EventTypes.AFTER_UNAPPLY)
|
|
x = unapply ? x - dx : x + dx
|
|
y = unapply ? y - dy : y + dy
|
|
|
|
tspans[i].setAttribute('x', x)
|
|
tspans[i].setAttribute('y', y)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
})
|
|
}
|
|
|
|
/**
|
|
* Hack for Firefox bugs where text element features aren't updated or get
|
|
* messed up. See issue 136 and issue 137.
|
|
* This function clones the element and re-selects it.
|
|
* @function module:svgcanvas~ffClone
|
|
* @todo Test for this bug on load and add it to "support" object instead of
|
|
* browser sniffing
|
|
* @param {Element} elem - The (text) DOM element to clone
|
|
* @returns {Element} Cloned element
|
|
*/
|
|
export const ffClone = (elem) => {
|
|
if (!isGecko()) { return elem }
|
|
const clone = elem.cloneNode(true)
|
|
elem.before(clone)
|
|
elem.remove()
|
|
svgCanvas.selectorManager.releaseSelector(elem)
|
|
svgCanvas.setSelectedElements(0, clone)
|
|
svgCanvas.selectorManager.requestSelector(clone).showGrips(true)
|
|
return clone
|
|
}
|
|
|
|
/**
|
|
* This function makes the changes to the elements. It does not add the change
|
|
* to the history stack.
|
|
* @param {string} attr - Attribute name
|
|
* @param {string|Float} newValue - String or number with the new attribute value
|
|
* @param {Element[]} elems - The DOM elements to apply the change to
|
|
* @returns {void}
|
|
*/
|
|
export const changeSelectedAttributeNoUndoMethod = (attr, newValue, elems) => {
|
|
if (attr === 'id') {
|
|
// if the user is changing the id, then de-select the element first
|
|
// change the ID, then re-select it with the new ID
|
|
// as this change can impact other extensions, a 'renamedElement' event is thrown
|
|
const elem = elems[0]
|
|
const oldId = elem.id
|
|
if (oldId !== newValue) {
|
|
svgCanvas.clearSelection()
|
|
elem.id = newValue
|
|
svgCanvas.addToSelection([elem], true)
|
|
svgCanvas.call('elementRenamed', { elem, oldId, newId: newValue })
|
|
}
|
|
return
|
|
}
|
|
const selectedElements = svgCanvas.getSelectedElements()
|
|
const zoom = svgCanvas.getZoom()
|
|
if (svgCanvas.getCurrentMode() === 'pathedit') {
|
|
// Editing node
|
|
svgCanvas.pathActions.moveNode(attr, newValue)
|
|
}
|
|
elems = elems ?? selectedElements
|
|
let i = elems.length
|
|
const noXYElems = ['g', 'polyline', 'path']
|
|
|
|
while (i--) {
|
|
let elem = elems[i]
|
|
if (!elem) { continue }
|
|
|
|
// Set x,y vals on elements that don't have them
|
|
if ((attr === 'x' || attr === 'y') && noXYElems.includes(elem.tagName)) {
|
|
const bbox = getStrokedBBoxDefaultVisible([elem])
|
|
const diffX = attr === 'x' ? parseFloat(newValue) - bbox.x : 0
|
|
const diffY = attr === 'y' ? parseFloat(newValue) - bbox.y : 0
|
|
svgCanvas.moveSelectedElements(diffX * zoom, diffY * zoom, true)
|
|
continue
|
|
}
|
|
|
|
let oldval = attr === '#text' ? elem.textContent : elem.getAttribute(attr)
|
|
if (!oldval) { oldval = '' }
|
|
if (oldval !== String(newValue)) {
|
|
if (attr === '#text') {
|
|
// const oldW = utilsGetBBox(elem).width;
|
|
elem.textContent = newValue
|
|
|
|
// FF bug occurs on on rotated elements
|
|
if ((/rotate/).test(elem.getAttribute('transform'))) {
|
|
elem = ffClone(elem)
|
|
}
|
|
// Hoped to solve the issue of moving text with text-anchor="start",
|
|
// but this doesn't actually fix it. Hopefully on the right track, though. -Fyrd
|
|
} else if (attr === '#href') {
|
|
setHref(elem, newValue)
|
|
} else if (newValue) {
|
|
elem.setAttribute(attr, isNaN(parseFloat(newValue)) ? newValue : parseFloat(newValue))
|
|
} else if (typeof newValue === 'number') {
|
|
elem.setAttribute(attr, newValue)
|
|
} else {
|
|
elem.removeAttribute(attr)
|
|
}
|
|
|
|
// Go into "select" mode for text changes
|
|
// NOTE: Important that this happens AFTER elem.setAttribute() or else attributes like
|
|
// font-size can get reset to their old value, ultimately by svgEditor.updateContextPanel(),
|
|
// after calling textActions.toSelectMode() below
|
|
if (svgCanvas.getCurrentMode() === 'textedit' && attr !== '#text' && elem.textContent.length) {
|
|
svgCanvas.textActions.toSelectMode(elem)
|
|
}
|
|
|
|
// Use the Firefox ffClone hack for text elements with gradients or
|
|
// where other text attributes are changed.
|
|
if (isGecko() &&
|
|
elem.nodeName === 'text' &&
|
|
(/rotate/).test(elem.getAttribute('transform')) &&
|
|
(String(newValue).startsWith('url') || (['font-size', 'font-family', 'x', 'y'].includes(attr) && elem.textContent))) {
|
|
elem = ffClone(elem)
|
|
}
|
|
// Timeout needed for Opera & Firefox
|
|
// codedread: it is now possible for this to be called with elements
|
|
// that are not in the selectedElements array, we need to only request a
|
|
// selector if the element is in that array
|
|
if (selectedElements.includes(elem)) {
|
|
setTimeout(function () {
|
|
// Due to element replacement, this element may no longer
|
|
// be part of the DOM
|
|
if (!elem.parentNode) { return }
|
|
svgCanvas.selectorManager.requestSelector(elem).resize()
|
|
}, 0)
|
|
}
|
|
// Only recalculate rotation center for attributes that change element geometry.
|
|
// Non-geometric attributes (stroke-width, fill, opacity, etc.) don't affect
|
|
// the bbox center, so the rotation is already correct and must not be touched.
|
|
// BBOX_AFFECTING_ATTRS is imported from history.js to keep the list in one place.
|
|
const angle = getRotationAngle(elem)
|
|
if (angle !== 0 && attr !== 'transform' && BBOX_AFFECTING_ATTRS.has(attr)) {
|
|
const tlist = getTransformList(elem)
|
|
let n = tlist.numberOfItems
|
|
while (n--) {
|
|
const xform = tlist.getItem(n)
|
|
if (xform.type === 4) {
|
|
// Compute bbox BEFORE removing the rotation so we can bail out
|
|
// safely if getBBox returns nothing (avoids losing the rotation).
|
|
const box = utilsGetBBox(elem)
|
|
if (!box) break
|
|
|
|
tlist.removeItem(n)
|
|
|
|
// Transform bbox center through only the transforms that come
|
|
// AFTER the rotation in the list (not the pre-rotation transforms).
|
|
// After removeItem(n), what was at n+1 is now at n.
|
|
let centerMatrix
|
|
if (n < tlist.numberOfItems) {
|
|
centerMatrix = transformListToTransform(tlist, n, tlist.numberOfItems - 1).matrix
|
|
} else {
|
|
centerMatrix = svgCanvas.getSvgRoot().createSVGMatrix() // identity
|
|
}
|
|
const center = transformPoint(
|
|
box.x + box.width / 2, box.y + box.height / 2, centerMatrix
|
|
)
|
|
|
|
const newrot = svgCanvas.getSvgRoot().createSVGTransform()
|
|
newrot.setRotate(angle, center.x, center.y)
|
|
tlist.insertItemBefore(newrot, n)
|
|
break
|
|
}
|
|
}
|
|
}
|
|
} // if oldValue != newValue
|
|
} // for each elem
|
|
}
|
|
|
|
/**
|
|
* Change the given/selected element and add the original value to the history stack.
|
|
* If you want to change all `selectedElements`, ignore the `elems` argument.
|
|
* If you want to change only a subset of `selectedElements`, then send the
|
|
* subset to this function in the `elems` argument.
|
|
* @function module:svgcanvas.SvgCanvas#changeSelectedAttribute
|
|
* @param {string} attr - String with the attribute name
|
|
* @param {string|Float} val - String or number with the new attribute value
|
|
* @param {Element[]} elems - The DOM elements to apply the change to
|
|
* @returns {void}
|
|
*/
|
|
export const changeSelectedAttributeMethod = (attr, val, elems) => {
|
|
const selectedElements = svgCanvas.getSelectedElements()
|
|
elems = elems || selectedElements
|
|
svgCanvas.undoMgr.beginUndoableChange(attr, elems)
|
|
|
|
changeSelectedAttributeNoUndoMethod(attr, val, elems)
|
|
|
|
const batchCmd = svgCanvas.undoMgr.finishUndoableChange()
|
|
if (!batchCmd.isEmpty()) {
|
|
// svgCanvas.addCommandToHistory(batchCmd);
|
|
svgCanvas.undoMgr.addCommandToHistory(batchCmd)
|
|
}
|
|
}
|