expanded highlighter geometry (#1929)

Currently, the highlighter shape uses a single 0-width line for its
geometry, same as the draw tool. For the draw tool this works ok - the
visual line is thin enough that unless you zoom right in, it's hard to
find areas where the hover should trigger but isn't. As the highlighter
tool is much thicker though, it's relatively easy to find those areas.

The fix is for the geometry to represent the line including its thick
stroke, instead of at 0-width. There are two possible approaches here:
1. Update the polyline geometry to allow passing a stroke width.
2. Instead of a polyline, make the highlighter shape be a polygon that
traces _around_ the stroke

1 is the more accurate approach, but is hard to fit into our geometry
system. Our geometry is based around two primitives: `getVertices` which
returns an array of points around the shape, and `nearestPoint` which
returns the nearest point on the geometry to a vector we pass in. We can
account for a stroke in `nearestPoint` pretty easily, including it in
`getVertices` is hard - we'd have to expand the vertices and handle line
join/caps etc. Just making the change in `nearestPoint` does fix the
issue here, but i'm not sure about the knock-on effect elsewhere and
don't really want to introduce 1-off hacks into the core geometry
system.

2 actually means addressing the same hard problem around outlining
strokes as 1, but it lets us do it in a more tightly-scoped one-off
change just to the highlighter shape, instead of trying to come up with
a generic solution for the whole geometry system. This is the approach
I've taken in this diff. We outline the stroke using perfect-freehand,
which works pretty well but produces inaccurate results at edge-cases,
particularly when a line rapidly changes direction:

![Kapture 2023-09-19 at 13 45
01](https://github.com/tldraw/tldraw/assets/1489520/1593ac5c-e7db-4360-b97d-ba66cdfb5498)

I think that given this is scoped to just the highlighter shape and is
imo an improvement over the stroke issue from before, it's a reasonable
solution for now. If we want to in the future we could implement real
non-freehand-based outlining.

### Change Type

- [x] `patch` — Bug fix

### Test Plan

1. Create a highlight shape
2. Zoom in
3. Make sure you can interact with the shape at its edges instead of
right in the center
This commit is contained in:
alex 2023-09-26 12:21:37 +01:00 committed by GitHub
parent 5cd74f4bd6
commit 79f46da199
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 50 additions and 22 deletions

View file

@ -412,6 +412,7 @@ export const debugFlags: {
resetConnectionEveryPing: DebugFlag<boolean>; resetConnectionEveryPing: DebugFlag<boolean>;
debugCursors: DebugFlag<boolean>; debugCursors: DebugFlag<boolean>;
forceSrgb: DebugFlag<boolean>; forceSrgb: DebugFlag<boolean>;
debugGeometry: DebugFlag<boolean>;
}; };
// @internal (undocumented) // @internal (undocumented)

View file

@ -15,6 +15,7 @@ import { useScreenBounds } from '../hooks/useScreenBounds'
import { Matrix2d } from '../primitives/Matrix2d' import { Matrix2d } from '../primitives/Matrix2d'
import { toDomPrecision } from '../primitives/utils' import { toDomPrecision } from '../primitives/utils'
import { debugFlags } from '../utils/debug-flags' import { debugFlags } from '../utils/debug-flags'
import { GeometryDebuggingView } from './GeometryDebuggingView'
import { LiveCollaborators } from './LiveCollaborators' import { LiveCollaborators } from './LiveCollaborators'
import { Shape } from './Shape' import { Shape } from './Shape'
import { ShapeIndicator } from './ShapeIndicator' import { ShapeIndicator } from './ShapeIndicator'
@ -110,7 +111,7 @@ export const Canvas = track(function Canvas({ className }: { className?: string
</div> </div>
<div className="tl-fixed-layer tl-overlays"> <div className="tl-fixed-layer tl-overlays">
<div ref={rHtmlLayer2} className="tl-html-layer"> <div ref={rHtmlLayer2} className="tl-html-layer">
{/* <GeometryDebuggingView /> */} {debugFlags.debugGeometry.value && <GeometryDebuggingView />}
<HandlesWrapper /> <HandlesWrapper />
<BrushWrapper /> <BrushWrapper />
<ScribbleWrapper /> <ScribbleWrapper />

View file

@ -1,18 +1,33 @@
import { track } from '@tldraw/state' import { track } from '@tldraw/state'
import { modulate } from '@tldraw/utils' import { modulate } from '@tldraw/utils'
import { useEffect, useState } from 'react'
import { HIT_TEST_MARGIN } from '../constants' import { HIT_TEST_MARGIN } from '../constants'
import { useEditor } from '../hooks/useEditor' import { useEditor } from '../hooks/useEditor'
function useTick(isEnabled = true) {
const [_, setTick] = useState(0)
const editor = useEditor()
useEffect(() => {
if (!isEnabled) return
const update = () => setTick((tick) => tick + 1)
editor.on('tick', update)
return () => {
editor.off('tick', update)
}
}, [editor, isEnabled])
}
export const GeometryDebuggingView = track(function GeometryDebuggingView({ export const GeometryDebuggingView = track(function GeometryDebuggingView({
showStroke = true, showStroke = true,
showVertices = true, showVertices = true,
showClosestPointOnOutline = false, showClosestPointOnOutline = true,
}: { }: {
showStroke?: boolean showStroke?: boolean
showVertices?: boolean showVertices?: boolean
showClosestPointOnOutline?: boolean showClosestPointOnOutline?: boolean
}) { }) {
const editor = useEditor() const editor = useEditor()
useTick(showClosestPointOnOutline)
const { const {
zoomLevel, zoomLevel,

View file

@ -52,6 +52,7 @@ export const debugFlags = {
defaults: { all: false }, defaults: { all: false },
}), }),
forceSrgb: createDebugValue('forceSrgbColors', { defaults: { all: false } }), forceSrgb: createDebugValue('forceSrgbColors', { defaults: { all: false } }),
debugGeometry: createDebugValue('debugGeometry', { defaults: { all: false } }),
} }
declare global { declare global {

View file

@ -27,6 +27,7 @@ import { MigrationFailureReason } from '@tldraw/editor';
import { Migrations } from '@tldraw/editor'; import { Migrations } from '@tldraw/editor';
import { NamedExoticComponent } from 'react'; import { NamedExoticComponent } from 'react';
import { ObjectValidator } from '@tldraw/editor'; import { ObjectValidator } from '@tldraw/editor';
import { Polygon2d } from '@tldraw/editor';
import { Polyline2d } from '@tldraw/editor'; import { Polyline2d } from '@tldraw/editor';
import { default as React_2 } from 'react'; import { default as React_2 } from 'react';
import * as React_3 from 'react'; import * as React_3 from 'react';
@ -654,11 +655,9 @@ export class HighlightShapeUtil extends ShapeUtil<TLHighlightShape> {
// (undocumented) // (undocumented)
component(shape: TLHighlightShape): JSX.Element; component(shape: TLHighlightShape): JSX.Element;
// (undocumented) // (undocumented)
expandSelectionOutlinePx(shape: TLHighlightShape): number;
// (undocumented)
getDefaultProps(): TLHighlightShape['props']; getDefaultProps(): TLHighlightShape['props'];
// (undocumented) // (undocumented)
getGeometry(shape: TLHighlightShape): Circle2d | Polyline2d; getGeometry(shape: TLHighlightShape): Circle2d | Polygon2d;
// (undocumented) // (undocumented)
hideResizeHandles: (shape: TLHighlightShape) => boolean; hideResizeHandles: (shape: TLHighlightShape) => boolean;
// (undocumented) // (undocumented)

View file

@ -47,19 +47,17 @@ const solidSettings = (strokeWidth: number): StrokeOptions => {
export function getHighlightFreehandSettings({ export function getHighlightFreehandSettings({
strokeWidth, strokeWidth,
showAsComplete, showAsComplete,
isPen,
}: { }: {
strokeWidth: number strokeWidth: number
showAsComplete: boolean showAsComplete: boolean
isPen: boolean
}): StrokeOptions { }): StrokeOptions {
return { return {
size: 1 + strokeWidth, size: 1 + strokeWidth,
thinning: 0.1, thinning: 0,
streamline: 0.5, streamline: 0.5,
smoothing: 0.5, smoothing: 0.5,
simulatePressure: !isPen, simulatePressure: false,
easing: isPen ? PEN_EASING : EASINGS.easeOutSine, easing: EASINGS.easeOutSine,
last: showAsComplete, last: showAsComplete,
} }
} }

View file

@ -1,7 +1,7 @@
/* eslint-disable react-hooks/rules-of-hooks */ /* eslint-disable react-hooks/rules-of-hooks */
import { import {
Circle2d, Circle2d,
Polyline2d, Polygon2d,
SVGContainer, SVGContainer,
ShapeUtil, ShapeUtil,
TLDefaultColorTheme, TLDefaultColorTheme,
@ -18,7 +18,9 @@ import {
import { getHighlightFreehandSettings, getPointsFromSegments } from '../draw/getPath' import { getHighlightFreehandSettings, getPointsFromSegments } from '../draw/getPath'
import { useDefaultColorTheme } from '../shared/ShapeFill' import { useDefaultColorTheme } from '../shared/ShapeFill'
import { FONT_SIZES } from '../shared/default-shape-constants' import { FONT_SIZES } from '../shared/default-shape-constants'
import { getStrokeOutlinePoints } from '../shared/freehand/getStrokeOutlinePoints'
import { getStrokePoints } from '../shared/freehand/getStrokePoints' import { getStrokePoints } from '../shared/freehand/getStrokePoints'
import { setStrokePointRadii } from '../shared/freehand/setStrokePointRadii'
import { getSvgPathFromStrokePoints } from '../shared/freehand/svg' import { getSvgPathFromStrokePoints } from '../shared/freehand/svg'
import { useColorSpace } from '../shared/useColorSpace' import { useColorSpace } from '../shared/useColorSpace'
import { useForceSolid } from '../shared/useForceSolid' import { useForceSolid } from '../shared/useForceSolid'
@ -47,8 +49,8 @@ export class HighlightShapeUtil extends ShapeUtil<TLHighlightShape> {
} }
getGeometry(shape: TLHighlightShape) { getGeometry(shape: TLHighlightShape) {
const strokeWidth = getStrokeWidth(shape)
if (getIsDot(shape)) { if (getIsDot(shape)) {
const strokeWidth = getStrokeWidth(shape)
return new Circle2d({ return new Circle2d({
x: -strokeWidth / 2, x: -strokeWidth / 2,
y: -strokeWidth / 2, y: -strokeWidth / 2,
@ -57,8 +59,13 @@ export class HighlightShapeUtil extends ShapeUtil<TLHighlightShape> {
}) })
} }
return new Polyline2d({ const { strokePoints, sw } = getHighlightStrokePoints(shape, strokeWidth, true)
points: getPointsFromSegments(shape.props.segments), const opts = getHighlightFreehandSettings({ strokeWidth: sw, showAsComplete: true })
setStrokePointRadii(strokePoints, opts)
return new Polygon2d({
points: getStrokeOutlinePoints(strokePoints, opts),
isFilled: true,
}) })
} }
@ -96,7 +103,6 @@ export class HighlightShapeUtil extends ShapeUtil<TLHighlightShape> {
const options = getHighlightFreehandSettings({ const options = getHighlightFreehandSettings({
strokeWidth, strokeWidth,
showAsComplete, showAsComplete,
isPen: shape.props.isPen,
}) })
const strokePoints = getStrokePoints(allPointsFromSegments, options) const strokePoints = getStrokePoints(allPointsFromSegments, options)
@ -110,10 +116,6 @@ export class HighlightShapeUtil extends ShapeUtil<TLHighlightShape> {
return <path d={strokePath} /> return <path d={strokePath} />
} }
override expandSelectionOutlinePx(shape: TLHighlightShape): number {
return getStrokeWidth(shape) / 2
}
override toSvg(shape: TLHighlightShape) { override toSvg(shape: TLHighlightShape) {
const theme = getDefaultColorTheme({ isDarkMode: this.editor.user.isDarkMode }) const theme = getDefaultColorTheme({ isDarkMode: this.editor.user.isDarkMode })
return highlighterToSvg(getStrokeWidth(shape), shape, OVERLAY_OPACITY, theme) return highlighterToSvg(getStrokeWidth(shape), shape, OVERLAY_OPACITY, theme)
@ -164,7 +166,11 @@ function getIndicatorDot(point: VecLike, sw: number) {
},0` },0`
} }
function getHighlightSvgPath(shape: TLHighlightShape, strokeWidth: number, forceSolid: boolean) { function getHighlightStrokePoints(
shape: TLHighlightShape,
strokeWidth: number,
forceSolid: boolean
) {
const allPointsFromSegments = getPointsFromSegments(shape.props.segments) const allPointsFromSegments = getPointsFromSegments(shape.props.segments)
const showAsComplete = shape.props.isComplete || last(shape.props.segments)?.type === 'straight' const showAsComplete = shape.props.isComplete || last(shape.props.segments)?.type === 'straight'
@ -176,13 +182,19 @@ function getHighlightSvgPath(shape: TLHighlightShape, strokeWidth: number, force
const options = getHighlightFreehandSettings({ const options = getHighlightFreehandSettings({
strokeWidth: sw, strokeWidth: sw,
showAsComplete, showAsComplete,
isPen: shape.props.isPen,
}) })
const strokePoints = getStrokePoints(allPointsFromSegments, options) const strokePoints = getStrokePoints(allPointsFromSegments, options)
return { strokePoints, sw }
}
function getHighlightSvgPath(shape: TLHighlightShape, strokeWidth: number, forceSolid: boolean) {
const { strokePoints, sw } = getHighlightStrokePoints(shape, strokeWidth, forceSolid)
const solidStrokePath = const solidStrokePath =
strokePoints.length > 1 strokePoints.length > 1
? getSvgPathFromStrokePoints(strokePoints, false) ? getSvgPathFromStrokePoints(strokePoints, false)
: getShapeDot(allPointsFromSegments[0]) : getShapeDot(shape.props.segments[0].points[0])
return { solidStrokePath, sw } return { solidStrokePath, sw }
} }

View file

@ -183,6 +183,7 @@ const DebugMenuContent = track(function DebugMenuContent({
<DropdownMenu.Group> <DropdownMenu.Group>
<DebugFlagToggle flag={debugFlags.debugSvg} /> <DebugFlagToggle flag={debugFlags.debugSvg} />
<DebugFlagToggle flag={debugFlags.forceSrgb} /> <DebugFlagToggle flag={debugFlags.forceSrgb} />
<DebugFlagToggle flag={debugFlags.debugGeometry} />
<DebugFlagToggle <DebugFlagToggle
flag={debugFlags.debugCursors} flag={debugFlags.debugCursors}
onChange={(enabled) => { onChange={(enabled) => {