Skip to content

Commit

Permalink
Alternate fix: Store higher-precision copy of transform matrix
Browse files Browse the repository at this point in the history
  • Loading branch information
personalizedrefrigerator committed Sep 18, 2023
1 parent dde9736 commit 9fa9349
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 252 deletions.
62 changes: 38 additions & 24 deletions packages/js-draw/src/SVGLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,32 +291,46 @@ export default class SVGLoader implements ImageLoader {
// If given, 'supportedAttrs' will have x, y, etc. attributes that were used in computing the transform added to it,
// to prevent storing duplicate transform information when saving the component.
private getTransform(elem: SVGElement, supportedAttrs?: string[], computedStyles?: CSSStyleDeclaration): Mat33 {
computedStyles ??= window.getComputedStyle(elem);

let transformProperty = computedStyles.transform;
if (transformProperty === '' || transformProperty === 'none') {
transformProperty = elem.style.transform || 'none';
// If possible, load the js-draw specific transform attribute
const highpTransformAttribute = 'data-highp-transform';
const rawTransformData = elem.getAttribute(highpTransformAttribute);
let transform;
if (rawTransformData) {
try {
transform = Mat33.fromCSSMatrix(rawTransformData);
supportedAttrs?.push(highpTransformAttribute);
} catch(e) {
console.warn(`Unable to parse raw transform data, ${rawTransformData}. Falling back to CSS data.`);
}
}

// Prefer the actual .style.transform
// to the computed stylesheet -- in some browsers, the computedStyles version
// can have lower precision.
let transform;
try {
transform = Mat33.fromCSSMatrix(elem.style.transform);
} catch(_e) {
console.warn('matrix parse error', _e);
transform = Mat33.fromCSSMatrix(transformProperty);
}

const elemX = elem.getAttribute('x');
const elemY = elem.getAttribute('y');
if (elemX || elemY) {
const x = parseFloat(elemX ?? '0');
const y = parseFloat(elemY ?? '0');
if (!isNaN(x) && !isNaN(y)) {
supportedAttrs?.push('x', 'y');
transform = transform.rightMul(Mat33.translation(Vec2.of(x, y)));
if (!transform) {
computedStyles ??= window.getComputedStyle(elem);

let transformProperty = computedStyles.transform;
if (transformProperty === '' || transformProperty === 'none') {
transformProperty = elem.style.transform || 'none';
}

// Prefer the actual .style.transform
// to the computed stylesheet -- in some browsers, the computedStyles version
// can have lower precision.
try {
transform = Mat33.fromCSSMatrix(elem.style.transform);
} catch(_e) {
console.warn('matrix parse error', _e);
transform = Mat33.fromCSSMatrix(transformProperty);
}

const elemX = elem.getAttribute('x');
const elemY = elem.getAttribute('y');
if (elemX || elemY) {
const x = parseFloat(elemX ?? '0');
const y = parseFloat(elemY ?? '0');
if (!isNaN(x) && !isNaN(y)) {
supportedAttrs?.push('x', 'y');
transform = transform.rightMul(Mat33.translation(Vec2.of(x, y)));
}
}
}

Expand Down
55 changes: 55 additions & 0 deletions packages/js-draw/src/rendering/renderers/SVGRenderer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Color4, Mat33 } from '@js-draw/math';
import Viewport from '../../Viewport';
import SVGRenderer from './SVGRenderer';

const makeSVGRenderer = () => {
const viewport = new Viewport(() => {});
const svgElement = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
return { svgElement, renderer: new SVGRenderer(svgElement, viewport) };
};

describe('SVGRenderer', () => {
it('text highp and CSS transforms should match', () => {
const testTransforms = [
Mat33.identity,
new Mat33(
1, 2, 3.45678910,
3, 4, 5.67891011,
0, 0, 1,
),
new Mat33(
-1, -2, 3,
4, 5, 6,
0, 0, 1,
),
new Mat33(
1, 2, 0.00001,
3, 4, -0.00000123,
0, 0, 1,
),
];

for (const transform of testTransforms) {
const { svgElement, renderer } = makeSVGRenderer();

renderer.drawText('test', transform, {
size: 12,
fontFamily: 'font',
fontWeight: 'bold',
renderingStyle: {
fill: Color4.red,
},
});

expect(svgElement.querySelectorAll('text')).toHaveLength(1);
const textElement = svgElement.querySelector('text')!;
expect(textElement.textContent).toBe('test');

const transformFromProperty = Mat33.fromCSSMatrix(textElement.getAttribute('data-highp-transform') ?? '');
const transformFromCSS = Mat33.fromCSSMatrix(textElement.style.transform);

expect(transformFromProperty).objEq(transformFromCSS);
expect(transformFromCSS).objEq(transform);
}
});
});
26 changes: 9 additions & 17 deletions packages/js-draw/src/rendering/renderers/SVGRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,24 +151,19 @@ export default class SVGRenderer extends AbstractRenderer {

// Apply [elemTransform] to [elem]. Uses both a `matrix` and `.x`, `.y` properties if `setXY` is true.
// Otherwise, just uses a `matrix`.
private transformFrom(elemTransform: Mat33, elem: SVGElement, inCanvasSpace: boolean = false, setXY: boolean = true) {
let transform = !inCanvasSpace ? this.getCanvasToScreenTransform().rightMul(elemTransform) : elemTransform;
const translation = transform.transformVec2(Vec2.zero);

if (setXY) {
transform = transform.rightMul(Mat33.translation(translation.times(-1)));
}
private transformFrom(elemTransform: Mat33, elem: SVGElement, inCanvasSpace: boolean = false) {
const transform = !inCanvasSpace ? this.getCanvasToScreenTransform().rightMul(elemTransform) : elemTransform;

if (!transform.eq(Mat33.identity)) {
elem.style.transform = transform.toSafeCSSTransformList();
const matrixString = transform.toCSSMatrix();
elem.style.transform = matrixString;

// Most browsers round the components of CSS transforms.
// Include a higher precision copy of the element's transform.
elem.setAttribute('data-highp-transform', matrixString);
} else {
elem.style.transform = '';
}

if (setXY) {
elem.setAttribute('x', `${toRoundedString(translation.x)}`);
elem.setAttribute('y', `${toRoundedString(translation.y)}`);
}
}

private textContainer: SVGTextElement|null = null;
Expand Down Expand Up @@ -212,10 +207,7 @@ export default class SVGRenderer extends AbstractRenderer {
const container = document.createElementNS(svgNameSpace, 'text');
container.appendChild(document.createTextNode(text));

// Don't set .x/.y properties (just use .style.transform).
// Child nodes aren't translated by .x/.y properties, but are by .style.transform.
const setXY = false;
this.transformFrom(transform, container, true, setXY);
this.transformFrom(transform, container, true);
applyTextStyles(container, style);

this.elem.appendChild(container);
Expand Down
52 changes: 0 additions & 52 deletions packages/math/src/Mat33.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,56 +206,4 @@ describe('Mat33 tests', () => {
// scaling2D only scales the x/y components of vectors it transforms
expect(Mat33.scaling2D(2).getColumn(2)).objEq(Vec3.of(0, 0, 1));
});

describe('toSafeCSSTransformList', () => {
it('should not add extra transforms when unnecessary', () => {
expect(Mat33.identity.toSafeCSSTransformList()).toBe(Mat33.identity.toCSSMatrix());

// Should only convert to a single matrix()
expect(
Mat33.translation(Vec2.of(0.1, 0.1)).toSafeCSSTransformList()
).toBe('matrix(1,0,0,1,0.1,0.1)');
expect(
Mat33.translation(Vec2.of(0.01, 0.001)).toSafeCSSTransformList()
).toBe('matrix(1,0,0,1,0.01,0.001)');
});

it('should convert small translations into combinations of translations and scales', () => {
expect(
Mat33.translation(Vec2.of(0.23456789, 0.111111112)).toSafeCSSTransformList()
).toBe('scale(.0001) translate(2345px, 1111px) scale(.0001) translate(6788px, 1111px) scale(.0001) translate(9999px, 2000px) scale(.0001) translate(9999px, 0px) scale(10000) scale(10000) scale(10000) matrix(10000,0,0,10000,0,0)');
});

it('should produce the same matrix when parsed with fromCSSMatrix', () => {
const testMatrices = [
new Mat33(
1, 2, -123.45678901,
3, 4, 456.78901,
0, 0, 1,
),
new Mat33(
1e6, -1e7, 0.000001,
2, 3, 12345.6,
0, 0, 1,
),
new Mat33(
1e6, 0.001, 0.002001,
2, 1.2345, -12345.6,
0, 0, 1,
),
];

for (const mat of testMatrices) {
const filteredMatrix = Mat33.fromCSSMatrix(mat.toSafeCSSTransformList());

expect(filteredMatrix).objEq(mat, 1e-3);

expect(
filteredMatrix.toArray().map(toRoundedString)
).toMatchObject(
mat.toArray().map(toRoundedString)
);
}
});
});
});
159 changes: 0 additions & 159 deletions packages/math/src/Mat33.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,165 +430,6 @@ export class Mat33 {
return `matrix(${this.a1},${this.b1},${this.a2},${this.b2},${this.a3},${this.b3})`;
}

/**
* @beta May change or even be removed between minor releases.
*
* Converts this matrix into a list of CSS transforms that attempt to preserve
* this matrix's translation.
*
* In Chrome/Firefox, translation attributes only support 6 digits (likely an artifact
* of using lower-precision floating point numbers). This works around
* that by expanding this matrix into the product of several CSS transforms.
*
* **Note**: Assumes `this.c1 = this.c2 = 0` and `this.c3 = 1`.
*/
public toSafeCSSTransformList(): string {
// Check whether it's safe to return just the CSS matrix
const translation = Vec2.of(this.a3, this.b3);
const translationRoundedX = toRoundedString(translation.x);
const translationRoundedY = toRoundedString(translation.y);

const nonDigitsRegex = /[^0-9]+/g;
const translationXDigits = translationRoundedX.replace(nonDigitsRegex, '').length;
const translationYDigits = translationRoundedY.replace(nonDigitsRegex, '').length;

// Is it safe to just return the default CSS matrix?
if (translationXDigits <= 5 && translationYDigits <= 5) {
return this.toCSSMatrix();
}

// Remove the last column (the translation column)
let transform = new Mat33(
this.a1, this.a2, 0,
this.b1, this.b2, 0,
0, 0, 1,
);

const transforms: string[] = [];
let lastScale: Vec2|null = null;

// Appends a translate() command to the list of `transforms`.
const addTranslate = (translation: Vec2) => {
lastScale = null;
if (!translation.eq(Vec2.zero)) {
transforms.push(`translate(${toRoundedString(translation.x)}px, ${toRoundedString(translation.y)}px)`);
}
};

// Appends a scale() command to the list of transforms, possibly merging with
// the last command, if a scale().
const addScale = (scale: Vec2) => {
// Merge with the last scale
if (lastScale) {
const newScale = lastScale.scale(scale);

// Don't merge if the new scale has very large values
if (newScale.maximumEntryMagnitude() < 1e7) {
const previousCommand = transforms.pop();
console.assert(
previousCommand!.startsWith('scale'),
'Invalid state: Merging scale commands'
);

scale = newScale;
}
}

if (scale.x === scale.y) {
transforms.push(`scale(${toRoundedString(scale.x)})`);
} else {
transforms.push(`scale(${toRoundedString(scale.x)}, ${toRoundedString(scale.y)})`);
}
lastScale = scale;
};

// Returns the number of digits before the `.` in the given number string.
const digitsPreDecimalCount = (numberString: string) => {
let decimalIndex = numberString.indexOf('.');
if (decimalIndex === -1) {
decimalIndex = numberString.length;
}

return numberString.substring(0, decimalIndex).replace(nonDigitsRegex, '').length;
};

// Returns the number of digits (positive for left shift, negative for right shift)
// required to shift the decimal to the middle of the number.
const getShift = (numberString: string) => {
const preDecimal = digitsPreDecimalCount(numberString);
const postDecimal = (numberString.match(/[.](\d*)/) ?? ['', ''])[1].length;

// The shift required to center the decimal point.
const toCenter = postDecimal - preDecimal;

// toCenter is positive for a left shift (adding more pre-decimals),
// so, after applying it,
const postShiftPreDecimal = preDecimal + toCenter;

// We want the digits before the decimal to have a length at most 4, however.
// Thus, right shift until this is the case.
const shiftForAtMost5DigitsPreDecimal = 4 - Math.max(postShiftPreDecimal, 4);

return toCenter + shiftForAtMost5DigitsPreDecimal;
};

const addShiftedTranslate = (translate: Vec2, depth: number = 0) => {
const xString = toRoundedString(translate.x);
const yString = toRoundedString(translate.y);

const xShiftDigits = getShift(xString);
const yShiftDigits = getShift(yString);
const shift = Vec2.of(Math.pow(10, xShiftDigits), Math.pow(10, yShiftDigits));
const invShift = Vec2.of(Math.pow(10, -xShiftDigits), Math.pow(10, -yShiftDigits));

addScale(invShift);

const shiftedTranslate = translate.scale(shift);
const roundedShiftedTranslate = Vec2.of(
Math.floor(shiftedTranslate.x),
Math.floor(shiftedTranslate.y),
);

addTranslate(roundedShiftedTranslate);

// Don't recurse more than 3 times -- the more times we recurse, the more
// the scaling is influenced by error.
if (!roundedShiftedTranslate.eq(shiftedTranslate) && depth < 3) {
addShiftedTranslate(
shiftedTranslate.minus(roundedShiftedTranslate),
depth + 1
);
}

addScale(shift);

return translate;
};

const adjustTransformFromScale = () => {
if (lastScale) {
const scaledTransform = transform.rightMul(Mat33.scaling2D(lastScale));

// If adding the scale to the transform leads to large values, avoid
// doing this.
if (scaledTransform.maximumEntryMagnitude() < 1e12) {
transforms.pop();
transform = transform.rightMul(Mat33.scaling2D(lastScale));
lastScale = null;
}
}
};

addShiftedTranslate(translation);
adjustTransformFromScale();

if (!transform.eq(Mat33.identity)) {
transforms.push(transform.toCSSMatrix());
}

return transforms.join(' ');
}

/**
* Converts a CSS-form `matrix(a, b, c, d, e, f)` to a Mat33.
*
Expand Down

0 comments on commit 9fa9349

Please sign in to comment.