fix(core): store picker colours in the document's colour profile (#724)
The OKHCL picker baked every colour to sRGB coordinates, so editing a fill in a Display-P3 document stored sRGB numbers in a document that declares P3: rendering stayed correct because the perceptual payload drives it, but the stored value and any export disagreed with the profile. `okhclToRGBA` and `rgbaToOkHCL` now take the colour space to read or write, and both storage paths — the Figma API proxy and the picker actions — pass the document's profile. sRGB documents are unchanged, since that is the default.
This commit is contained in:
parent
0f6420d9cd
commit
e2de2471b9
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<SceneNode> {
|
||||
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<SceneNode> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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 ---
|
||||
|
|
|
|||
|
|
@ -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<OkHCLColor>) {
|
||||
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<OkHCLColor>) {
|
||||
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'
|
||||
)
|
||||
}
|
||||
|
|
|
|||
75
tests/engine/color/okhcl/document-color-space.test.ts
Normal file
75
tests/engine/color/okhcl/document-color-space.test.ts
Normal file
|
|
@ -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))
|
||||
})
|
||||
})
|
||||
Loading…
Reference in a new issue