diff --git a/CHANGELOG.md b/CHANGELOG.md index d0b9813ae..0d0bdd366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Fixed +- Store colours edited in the colour picker in the document's colour profile, so the values a Display-P3 document keeps and exports match the profile it declares. - Classify MCP `open_file` and `close_file` operations as read-only hints, and close opened document tabs through the new `close_file` tool with the usual unsaved-change prompt. - Animate the AI chat tool-call disclosure, which expanded and collapsed without motion because its animation classes were misspelled. - Explain why the local MCP server did not start — a missing `@open-pencil/mcp` install, a denied command, an early exit, a rejected local connection, or an unreachable address — with translated guidance and collapsible technical details instead of one generic health failure. diff --git a/packages/core/src/color/okhcl.ts b/packages/core/src/color/okhcl.ts index 0bcf93e5e..b9ca9ed39 100644 --- a/packages/core/src/color/okhcl.ts +++ b/packages/core/src/color/okhcl.ts @@ -21,8 +21,13 @@ export interface OkHCLPayload { } const toRGB = converter('rgb') +const toP3 = converter('p3') const toOkHCL = converter('oklch') const toDisplayableRGB = toGamut('rgb', 'oklch') +const toDisplayableP3 = toGamut('p3', 'oklch') + +/** Colour spaces a document's stored numbers can be in. */ +export type OkHCLColorSpace = 'srgb' | 'display-p3' const OKHCL_PLUGIN_KEY = 'okhcl' function clampUnit(value: number): number { @@ -45,17 +50,26 @@ function normalizeOkHCLColor(color: OkHCLColor): OkHCLColor { } } -export function okhclToRGBA(color: OkHCLColor): Color { +/** Coordinates in `colorSpace`, so callers can store colours in the document's profile. */ +export function okhclToRGBA(color: OkHCLColor, colorSpace: OkHCLColorSpace = 'srgb'): Color { const normalized = normalizeOkHCLColor(color) - const rgb = toRGB( - toDisplayableRGB({ - mode: 'oklch', - l: normalized.l, - c: normalized.c, - h: normalized.h, - alpha: normalized.a + const oklch = { + mode: 'oklch' as const, + l: normalized.l, + c: normalized.c, + h: normalized.h, + alpha: normalized.a + } + if (colorSpace === 'display-p3') { + const p3 = toP3(toDisplayableP3(oklch)) + return normalizeColor({ + r: p3.r, + g: p3.g, + b: p3.b, + a: p3.alpha ?? normalized.a }) - ) + } + const rgb = toRGB(toDisplayableRGB(oklch)) return normalizeColor({ r: rgb.r, g: rgb.g, @@ -64,14 +78,13 @@ export function okhclToRGBA(color: OkHCLColor): Color { }) } -export function rgbaToOkHCL(color: Color): OkHCLColor { - const oklch = toOkHCL({ - mode: 'rgb', - r: color.r, - g: color.g, - b: color.b, - alpha: color.a - }) +/** Reads `color` as coordinates in `colorSpace`, the inverse of {@link okhclToRGBA}. */ +export function rgbaToOkHCL(color: Color, colorSpace: OkHCLColorSpace = 'srgb'): OkHCLColor { + const oklch = toOkHCL( + colorSpace === 'display-p3' + ? { mode: 'p3', r: color.r, g: color.g, b: color.b, alpha: color.a } + : { mode: 'rgb', r: color.r, g: color.g, b: color.b, alpha: color.a } + ) return normalizeOkHCLColor({ h: oklch.h ?? 0, c: oklch.c, @@ -140,12 +153,13 @@ function filterOkHCLPayloads( export function setNodeFillOkHCL( node: SceneNode, index: number, - color: OkHCLColor + color: OkHCLColor, + colorSpace: OkHCLColorSpace = 'srgb' ): Partial { const fills = node.fills.map(copyFill) if (index < 0 || index >= fills.length) throw new Error(`Fill ${index} not found`) const fill = fills[index] - const rgba = okhclToRGBA(color) + const rgba = okhclToRGBA(color, colorSpace) fills[index] = { ...fill, color: rgba, @@ -168,12 +182,13 @@ export function setNodeFillOkHCL( export function setNodeStrokeOkHCL( node: SceneNode, index: number, - color: OkHCLColor + color: OkHCLColor, + colorSpace: OkHCLColorSpace = 'srgb' ): Partial { const strokes = node.strokes.map(copyStroke) if (index < 0 || index >= strokes.length) throw new Error(`Stroke ${index} not found`) const stroke = strokes[index] - const rgba = okhclToRGBA(color) + const rgba = okhclToRGBA(color, colorSpace) strokes[index] = { ...stroke, color: rgba, diff --git a/packages/core/src/figma-api/proxy.ts b/packages/core/src/figma-api/proxy.ts index 91e528ea7..22a10ebaa 100644 --- a/packages/core/src/figma-api/proxy.ts +++ b/packages/core/src/figma-api/proxy.ts @@ -348,7 +348,9 @@ export class FigmaNodeProxy { } setFillOkHCL(color: OkHCLColor, index = 0): void { - this._update(setNodeFillOkHCL(this._raw(), index, color)) + this._update( + setNodeFillOkHCL(this._raw(), index, color, this[INTERNAL_GRAPH].documentColorSpace) + ) } getStrokeOkHCL(index = 0): OkHCLPayload | null { @@ -356,7 +358,9 @@ export class FigmaNodeProxy { } setStrokeOkHCL(color: OkHCLColor, index = 0): void { - this._update(setNodeStrokeOkHCL(this._raw(), index, color)) + this._update( + setNodeStrokeOkHCL(this._raw(), index, color, this[INTERNAL_GRAPH].documentColorSpace) + ) } // --- Serialization --- diff --git a/packages/vue/src/controls/okhcl/helpers.ts b/packages/vue/src/controls/okhcl/helpers.ts index 809358dc6..620658672 100644 --- a/packages/vue/src/controls/okhcl/helpers.ts +++ b/packages/vue/src/controls/okhcl/helpers.ts @@ -8,7 +8,7 @@ import { setNodeFillOkHCL, setNodeStrokeOkHCL } from '@open-pencil/core/color' -import type { OkHCLColor } from '@open-pencil/core/color' +import type { OkHCLColor, OkHCLColorSpace } from '@open-pencil/core/color' import { BLACK } from '@open-pencil/core/constants' import type { Editor } from '@open-pencil/core/editor' import type { SceneNode } from '@open-pencil/scene-graph' @@ -29,19 +29,26 @@ export function getStrokeOkHCLColor(node: SceneNode | null, index: number): OkHC return node ? (getStrokeOkHCL(node, index)?.color ?? null) : null } -function fallbackFillOkHCL(node: SceneNode, index: number) { - return getFillOkHCLColor(node, index) ?? rgbaToOkHCL(node.fills[index]?.color ?? BLACK) +function fallbackFillOkHCL(node: SceneNode, index: number, colorSpace: OkHCLColorSpace) { + return ( + getFillOkHCLColor(node, index) ?? rgbaToOkHCL(node.fills[index]?.color ?? BLACK, colorSpace) + ) } -function fallbackStrokeOkHCL(node: SceneNode, index: number) { - return getStrokeOkHCLColor(node, index) ?? rgbaToOkHCL(node.strokes[index]?.color ?? BLACK) +function fallbackStrokeOkHCL(node: SceneNode, index: number, colorSpace: OkHCLColorSpace) { + return ( + getStrokeOkHCLColor(node, index) ?? rgbaToOkHCL(node.strokes[index]?.color ?? BLACK, colorSpace) + ) } export function createOkHCLActions(editor: Editor) { + // Colours are stored in the document's profile, so both directions use it. + const colorSpace = () => editor.graph.documentColorSpace + function ensureFillOkHCL(node: SceneNode, index: number) { editor.updateNodeWithUndo( node.id, - setNodeFillOkHCL(node, index, fallbackFillOkHCL(node, index)), + setNodeFillOkHCL(node, index, fallbackFillOkHCL(node, index, colorSpace()), colorSpace()), 'Update fill color model' ) } @@ -49,25 +56,25 @@ export function createOkHCLActions(editor: Editor) { function ensureStrokeOkHCL(node: SceneNode, index: number) { editor.updateNodeWithUndo( node.id, - setNodeStrokeOkHCL(node, index, fallbackStrokeOkHCL(node, index)), + setNodeStrokeOkHCL(node, index, fallbackStrokeOkHCL(node, index, colorSpace()), colorSpace()), 'Update stroke color model' ) } function updateFillOkHCL(node: SceneNode, index: number, patch: Partial) { - const current = fallbackFillOkHCL(node, index) + const current = fallbackFillOkHCL(node, index, colorSpace()) editor.updateNodeWithUndo( node.id, - setNodeFillOkHCL(node, index, { ...current, ...patch }), + setNodeFillOkHCL(node, index, { ...current, ...patch }, colorSpace()), 'Change fill OkHCL' ) } function updateStrokeOkHCL(node: SceneNode, index: number, patch: Partial) { - const current = fallbackStrokeOkHCL(node, index) + const current = fallbackStrokeOkHCL(node, index, colorSpace()) editor.updateNodeWithUndo( node.id, - setNodeStrokeOkHCL(node, index, { ...current, ...patch }), + setNodeStrokeOkHCL(node, index, { ...current, ...patch }, colorSpace()), 'Change stroke OkHCL' ) } diff --git a/tests/engine/color/okhcl/document-color-space.test.ts b/tests/engine/color/okhcl/document-color-space.test.ts new file mode 100644 index 000000000..3d055a0d9 --- /dev/null +++ b/tests/engine/color/okhcl/document-color-space.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from 'bun:test' + +import { + colorDistance, + getFillOkHCL, + okhclToRGBA, + rgbaToOkHCL, + setNodeFillOkHCL, + setNodeStrokeOkHCL +} from '@open-pencil/core/color' +import { SceneGraph } from '@open-pencil/scene-graph' + +const SATURATED = { l: 0.6, c: 0.3, h: 20, a: 1 } + +function createNode() { + const graph = new SceneGraph() + const page = graph.getPages()[0] + return graph.createNode('RECTANGLE', page.id, { + fills: [{ type: 'SOLID', color: { r: 0, g: 0, b: 0, a: 1 }, opacity: 1, visible: true }], + strokes: [ + { color: { r: 0, g: 0, b: 0, a: 1 }, weight: 1, opacity: 1, visible: true, align: 'INSIDE' } + ] + }) +} + +describe('OKHCL colours follow the document profile', () => { + test('resolves the same colour into the requested coordinates', () => { + const srgb = okhclToRGBA(SATURATED) + const p3 = okhclToRGBA(SATURATED, 'display-p3') + + // P3 is the wider space, so the same colour needs fewer of its primaries. + expect(p3.r).toBeLessThan(srgb.r) + expect(p3).not.toEqual(srgb) + // Round-tripping through the matching space returns the original colour. + expect(colorDistance(okhclToRGBA(rgbaToOkHCL(srgb), 'srgb'), srgb)).toBeLessThan(1) + expect( + colorDistance(okhclToRGBA(rgbaToOkHCL(p3, 'display-p3'), 'display-p3'), p3) + ).toBeLessThan(1) + }) + + test('reading stored coordinates in the wrong space shifts the picker values', () => { + const p3 = okhclToRGBA(SATURATED, 'display-p3') + const asP3 = rgbaToOkHCL(p3, 'display-p3') + const asSrgb = rgbaToOkHCL(p3) + // Chroma and lightness both drift when P3 coordinates are read as sRGB. + expect(Math.abs(asSrgb.c - asP3.c)).toBeGreaterThan(0.03) + expect(Math.abs(asSrgb.l - asP3.l)).toBeGreaterThan(0.015) + // Read in the document's own space, the stored colour comes back exactly. + expect(colorDistance(okhclToRGBA(asP3, 'display-p3'), p3)).toBeLessThan(0.001) + }) + + test('a Display-P3 document stores P3 coordinates for picker edits', () => { + const node = createNode() + const patch = setNodeFillOkHCL(node, 0, SATURATED, 'display-p3') + const stored = patch.fills?.[0]?.color + expect(stored).toBeDefined() + expect(stored?.r).toBeLessThan(okhclToRGBA(SATURATED).r) + expect( + colorDistance(stored ?? { r: 0, g: 0, b: 0, a: 1 }, okhclToRGBA(SATURATED, 'display-p3')) + ).toBeLessThan(1) + // The perceptual payload still travels with the fill. + expect(getFillOkHCL({ ...node, ...patch } as typeof node, 0)?.color.h).toBeCloseTo( + SATURATED.h, + 3 + ) + }) + + test('an sRGB document keeps storing sRGB coordinates', () => { + const node = createNode() + const patch = setNodeFillOkHCL(node, 0, SATURATED) + expect(patch.fills?.[0]?.color).toEqual(okhclToRGBA(SATURATED)) + const strokePatch = setNodeStrokeOkHCL(node, 0, SATURATED) + expect(strokePatch.strokes?.[0]?.color).toEqual(okhclToRGBA(SATURATED)) + }) +})