From ca5e246074c393edd120b54eea2111f30fc76544 Mon Sep 17 00:00:00 2001 From: Aakansha Doshi Date: Fri, 7 Apr 2023 12:06:48 +0530 Subject: [PATCH] Revert "use DOM height only for safari to fix LS" This reverts commit d75889238da59b7954ec3a6ac2c29dc0ba420635. --- src/element/textElement.ts | 26 +++++++++++++++++++------- src/element/textWysiwyg.tsx | 16 ++++++++++------ src/renderer/renderElement.ts | 6 +----- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/element/textElement.ts b/src/element/textElement.ts index 338b80287..9ba5a5f97 100644 --- a/src/element/textElement.ts +++ b/src/element/textElement.ts @@ -289,13 +289,10 @@ export const measureText = ( .map((x) => x || " ") .join("\n"); const fontSize = parseFloat(font); - let height = getTextHeight(text, fontSize, lineHeight); + const height = getTextHeight(text, fontSize, lineHeight); const width = getTextWidth(text, font); - const domMetrics = getDOMMetrics(text, font, lineHeight); - if (isSafari) { - height = domMetrics.height; - } - return { width, height, baseline: domMetrics.baseline }; + const { baseline } = getDOMMetrics(text, font, lineHeight); + return { width, height, baseline }; }; export const getDOMMetrics = ( @@ -316,6 +313,7 @@ export const getDOMMetrics = ( } container.style.lineHeight = String(lineHeight); + const canvasHeight = getTextHeight(text, parseFloat(font), lineHeight); container.innerText = text; @@ -328,9 +326,23 @@ export const getDOMMetrics = ( span.style.width = "1px"; span.style.height = "1px"; container.appendChild(span); - const baseline = span.offsetTop + span.offsetHeight; + let baseline = span.offsetTop + span.offsetHeight; const height = container.offsetHeight; + if (isSafari) { + // In Safari sometimes DOM height could be less than canvas height due to + // which text could go out of the bounding box hence shifting the baseline + // to make sure text is rendered correctly + if (canvasHeight > height) { + baseline += canvasHeight - height; + } + // In Safari sometimes DOM height could be more than canvas height due to + // which text could go out of the bounding box hence shifting the baseline + // to make sure text is rendered correctly + if (height > canvasHeight) { + baseline -= height - canvasHeight; + } + } document.body.removeChild(container); return { baseline, height }; }; diff --git a/src/element/textWysiwyg.tsx b/src/element/textWysiwyg.tsx index 95ef11f26..c1e26c11d 100644 --- a/src/element/textWysiwyg.tsx +++ b/src/element/textWysiwyg.tsx @@ -35,6 +35,8 @@ import { getMaxContainerHeight, getMaxContainerWidth, computeContainerDimensionForBoundText, + getDOMMetrics, + splitIntoLines, } from "./textElement"; import { actionDecreaseFontSize, @@ -44,7 +46,6 @@ import { actionZoomIn, actionZoomOut } from "../actions/actionCanvas"; import App from "../components/App"; import { LinearElementEditor } from "./linearElementEditor"; import { parseClipboard } from "../clipboard"; -import { splitIntoLines } from "./textElement"; const getTransform = ( width: number, @@ -272,13 +273,16 @@ export const textWysiwyg = ({ } else { textElementWidth += 0.5; } + const { height: domHeight } = getDOMMetrics( + updatedTextElement.text, + getFontString(updatedTextElement), + updatedTextElement.lineHeight, + ); let lineHeight = element.lineHeight; - if (isSafari) { - //@ts-ignore - lineHeight = `${ - element.height / splitIntoLines(element.text).length - }px`; + if (isSafari && domHeight > textElementHeight) { + lineHeight = (Math.floor(element.lineHeight * element.fontSize) / + element.fontSize) as ExcalidrawTextElement["lineHeight"]; } // Make sure text editor height doesn't go beyond viewport diff --git a/src/renderer/renderElement.ts b/src/renderer/renderElement.ts index 8e4e640cc..be6114aae 100644 --- a/src/renderer/renderElement.ts +++ b/src/renderer/renderElement.ts @@ -34,7 +34,6 @@ import { AppState, BinaryFiles, Zoom } from "../types"; import { getDefaultAppState } from "../appState"; import { BOUND_TEXT_PADDING, - isSafari, MAX_DECIMALS_FOR_SVG_EXPORT, MIME_TYPES, SVG_NS, @@ -286,13 +285,10 @@ const drawElementOnCanvas = ( : element.textAlign === "right" ? element.width : 0; - let lineHeightPx = getLineHeightInPx( + const lineHeightPx = getLineHeightInPx( element.fontSize, element.lineHeight, ); - if (isSafari) { - lineHeightPx = element.height / lines.length; - } const verticalOffset = element.height - element.baseline; for (let index = 0; index < lines.length; index++) { context.fillText(