Skip to content

Commit

Permalink
Merge pull request #593 from Shopify/deprecate/measureText
Browse files Browse the repository at this point in the history
Deprecate measureText
  • Loading branch information
chrfalch authored Jun 14, 2022
2 parents 99ac853 + 98eabdd commit a344a09
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ export const Control = ({
progress,
active,
}: ControlProps) => {
const pos = font.measureText(label);
const labelWidth = font
.getGlyphWidths(font.getGlyphIDs(label))
.reduce((a, b) => a + b, 0);
return (
<Group transform={translate({ x: x + 30, y: y + 30 })}>
<Text
x={2 * r - pos.width - 16}
y={r + pos.height / 2}
x={2 * r - labelWidth - 16}
y={r + font.getSize() / 2}
font={font}
color="white"
text={label}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ export const ProgressBar = ({ progress }: ProgressBarProps) => {
if (font === null) {
return null;
}
const pos = font.measureText("00°C");
const textWidth = font
.getGlyphWidths(font.getGlyphIDs("00°C"))
.reduce((a, b) => a + b, 0);
return (
<Group transform={translate({ x: 100, y: 223 })}>
<Group>
Expand Down Expand Up @@ -72,8 +74,8 @@ export const ProgressBar = ({ progress }: ProgressBarProps) => {
/>
</Box>
<Text
x={c.x - pos.width / 2}
y={c.y + pos.height / 2}
x={c.x - textWidth / 2}
y={c.y + font.getSize() / 2}
font={font}
text={text}
color="white"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ export const Title = ({ title }: Title) => {
if (!font) {
return null;
}
const pos = font.measureText(title);
const titleWidth = font
.getGlyphWidths(font.getGlyphIDs(title))
.reduce((a, b) => a + b, 0);
const offsetX = 30 + BUTTON_SIZE;
const space = 298 - offsetX;
return (
Expand All @@ -24,8 +26,8 @@ export const Title = ({ title }: Title) => {
</Button>
<Text
text={title}
x={offsetX + (space - pos.width) / 2}
y={BUTTON_SIZE - pos.height}
x={offsetX + (space - titleWidth) / 2}
y={BUTTON_SIZE - font.getSize()}
font={font}
color="white"
/>
Expand Down
6 changes: 3 additions & 3 deletions example/src/Examples/Severance/Symbol.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const Symbol = ({ i, j, font, pointer, clock }: SymbolProps) => {
const y = j * SIZE.height;
const noise = new SimplexNoise(`${i}-${j}`);
const text = DIGITS[Math.round(Math.random() * 9)];
const pos = font.measureText(text);
const [symbolWidth] = font.getGlyphWidths(font.getGlyphIDs(text));
const origin = vec(x + SIZE.width / 2, y + SIZE.height / 2);
const transform = useDerivedValue(
() => [
Expand All @@ -60,11 +60,11 @@ export const Symbol = ({ i, j, font, pointer, clock }: SymbolProps) => {
);
const dx = useDerivedValue(() => {
const d = A * noise.noise2D(x, clock.current * F);
return origin.x - pos.width / 2 + d;
return origin.x - symbolWidth / 2 + d;
}, [clock]);
const dy = useDerivedValue(() => {
const d = A * noise.noise2D(y, clock.current * F);
return origin.y + pos.height / 2 + d;
return origin.y + font.getSize() / 2 + d;
}, [clock]);
return (
<Group transform={transform} origin={origin}>
Expand Down
15 changes: 9 additions & 6 deletions example/src/Examples/Wallet/components/Label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ interface LabelProps {
export const Label = ({ state, y, graphs, width, height }: LabelProps) => {
const titleFont = useFont(sfMono, 64);
const subtitleFont = useFont(sfMono, 24);
console.log({ titleFont });
const translateY = height + PADDING;
const AJUSTED_SIZE = height - PADDING * 2;
const text = useDerivedValue(() => {
Expand All @@ -49,14 +48,18 @@ export const Label = ({ state, y, graphs, width, height }: LabelProps) => {
return 0;
}
const graph = graphs[state.current.current];
return (
width / 2 - titleFont.measureText(format(graph.data.maxPrice)).width / 2
);
const title = format(graph.data.maxPrice);
const titleWidth = titleFont
.getGlyphWidths(titleFont.getGlyphIDs(title))
.reduce((a, b) => a + b, 0);
return width / 2 - titleWidth / 2;
}, [state, titleFont]);
if (!titleFont || !subtitleFont) {
return null;
}
const subtitlePos = subtitleFont.measureText(subtitle);
const subtitleWidth = subtitleFont
.getGlyphWidths(subtitleFont.getGlyphIDs(subtitle))
.reduce((a, b) => a + b, 0);
return (
<>
<Text
Expand All @@ -67,7 +70,7 @@ export const Label = ({ state, y, graphs, width, height }: LabelProps) => {
color="white"
/>
<Text
x={width / 2 - subtitlePos.width / 2}
x={width / 2 - subtitleWidth / 2}
y={translateY - 60}
text={subtitle}
font={subtitleFont}
Expand Down
5 changes: 4 additions & 1 deletion package/cpp/api/JsiSkFont.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
#include <vector>

#include <jsi/jsi.h>

#include "JsiSkHostObjects.h"
#include <RNSkLog.h>

#include "JsiSkPaint.h"
#include "JsiSkRect.h"
#include "JsiSkTypeface.h"
#include "JsiSkPoint.h"


#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdocumentation"

Expand All @@ -37,6 +39,7 @@ namespace RNSkia

JSI_HOST_FUNCTION(measureText)
{
RNSkLogger::warnToJavascriptConsole(runtime, "measureText() is deprecated. Clients should use 'Font.getGlyphWidths' instead (the latter does no shaping)");
auto textVal = arguments[0].asString(runtime).utf8(runtime);
auto text = textVal.c_str();
SkRect rect;
Expand Down
23 changes: 9 additions & 14 deletions package/src/skia/types/Font/Font.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@ export interface FontMetrics {
}

export interface SkFont extends SkJSIInstance<"Font"> {
/**
* Retrieves the advanceX measurements for each glyph.
* If paint is not null, its stroking, PathEffect, and MaskFilter fields are respected.
* One width per glyph is returned in the returned array.
* @param glyphs
* @param paint
*/
getGlyphWidths(glyphs: number[], paint?: SkPaint): number[];

/** Returns the advance width of text.
The advance is the normal distance to move before drawing additional text.
Returns the bounding box of text if bounds is not nullptr. The paint
Expand Down Expand Up @@ -39,20 +48,6 @@ export interface SkFont extends SkJSIInstance<"Font"> {
*/
getGlyphIDs(str: string, numCodePoints?: number): number[];

/**
* Retrieves the advanceX measurements for each glyph.
* If paint is not null, its stroking, PathEffect, and MaskFilter fields are respected.
* One width per glyph is returned in the returned array.
* @param glyphs
* @param paint
* @param output - if provided, the results will be copied into this array.
*/
getGlyphWidths(
glyphs: number[],
paint?: SkPaint | null,
output?: Float32Array
): Float32Array;

/**
* Computes any intersections of a thick "line" and a run of positionsed glyphs.
* The thick line is represented as a top and bottom coordinate (positive for
Expand Down
10 changes: 7 additions & 3 deletions package/src/skia/web/JsiSkFont.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
SkTypeface,
} from "../types";

import { HostObject, NotImplementedOnRNWeb, toValue, ckEnum } from "./Host";
import { HostObject, toValue, ckEnum } from "./Host";
import { JsiSkRect } from "./JsiSkRect";
import { JsiSkTypeface } from "./JsiSkTypeface";

Expand All @@ -20,7 +20,11 @@ export class JsiSkFont extends HostObject<Font, "Font"> implements SkFont {
}

measureText(_text: string, _paint?: SkPaint): SkRect {
throw new NotImplementedOnRNWeb();
console.warn(
`measureText() is deprecated an returns an empty rectangle on React Native Web.
Clients should use "Font.getGlyphWidths" instead (the latter does no shaping)`
);
return new JsiSkRect(this.CanvasKit, this.CanvasKit.XYWHRect(0, 0, 0, 0));
}

getMetrics() {
Expand All @@ -42,7 +46,7 @@ export class JsiSkFont extends HostObject<Font, "Font"> implements SkFont {

// TODO: Fix return value in the C++ implementation, it return float32
getGlyphWidths(glyphs: number[], paint?: SkPaint | null) {
return this.ref.getGlyphWidths(glyphs, paint ? toValue(paint) : null);
return [...this.ref.getGlyphWidths(glyphs, paint ? toValue(paint) : null)];
}

getGlyphIntercepts(
Expand Down

0 comments on commit a344a09

Please sign in to comment.