Skip to content

feat(ui): Raster Layer Color Adjusters #8420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

dunkeroni
Copy link
Contributor

Summary

Client-side support for making image adjustments in the canvas raster layers. These effects are applied using konva filters. They can be kept on to automatically apply to any new edits of the layer or finalized to bake the results in with the Finish button. Can be collapsed to take up less space or temporarily disabled to compare against the original.

Generating, creating from bounding box, and saving the canvas will use the current visible state of the layer (no need to finalize). If a saved canvas is recalled with the Remix option, the original layer with any pending adjustments will appear again. Duplicating a layer creates a duplicate with identical pending adjustments if they exist. Select Object will persist any pending adjustments if the user selects Apply, but not Save As (select object uses the underlying layer data directly instead of the view).

Has two modes:
Simple: Brightness, Contrast, Saturation, Temperature, Tint, Sharpness
"Brightness" is an absolute shift applied to all pixels and causes clipping on the ends. Some users might expect that to be called "Exposure" depending on the software they are coming from. Open to suggestions if we want to swap terms.
image

Curves: Master, Red, Green, Blue
image

Right click menu to access:
image

Related Issues / Discussions

Similar to the filters exposed in #7594 but in the frontend. Backend filters can still be useful for specific colorspace actions, but most of the common use cases should be solved with these settings.

A note on performance: For large layers, the filters can take noticeable time to complete. Konva filters re-apply any time the settings are changed or when the layer has its data updated. If a layer has pending adjustments, they will recalculate when:

  • User finishes drawing a line or erasing on that layer
  • User clicks Apply on a transformation edit

Moving layers, generating new layers, or making edits to other layers does not trigger recalculation.

QA Instructions

Demo video of interactions (uses old right-click menu)
Screencast From 2025-08-13 02-22-58.webm

Merge Plan

Ready to merge

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions bot added the frontend PRs that change frontend files label Aug 13, 2025
Copy link
Collaborator

@psychedelicious psychedelicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice to have this in the app, thanks for working on it.

  • As you noted, perf is not great as layer size increases. It seems easy to accidentally end up with a super big layer and stack of adjustments that bogs the system down. I think this is OK for a v1 implementation but we should do our best to make it faster ASAP. I imagine the only real way to do that is native code. Maybe https://wasm-vips.kleisauke.nl/ or port to a diff lang and compile to wasm.
  • I ended up with a full UI crash at some point. Somethign was trying to access a property of undefined. IIRC I was adjusting sharpness. I refreshed the page and it started working again. Sorry but I didn't capture the error; I am unable to reproduce it now.

Some UI/UX feedback:

  • It's not clear if you have selected simple or curves. Suggest setting the colorScheme on the button to invokeBlue to indicate active mode. Added code suggestion
  • When disabled, you can still fiddle w/ the sliders and curves editor but they don't do anything. Suggest disabling the knobs n dials when disabled
  • There's no way to dismiss the adjustment without applying/finishing it. And finishing always rasterizes the layer. Suggest comparing selected adjust mode and its settings and if unchagned from defaults, when finishing, do not rasterize
  • Layout is a bit wonky when panel is at min width - also needs some padding:
    image
  • Curves editor canvas does not scale when resizing panel, resulting in stretched appearance:
    image
  • The font size for channel labels in curve editor is too small
  • Curves editor needs padding around it
  • Some wonkiness when dragging a point across the x coord of another point:
    Screen.Recording.2025-08-15.at.1.09.56.pm.mov
  • Some awkward behaviour w/ doubleclicking, I see we are calculating the nearest point and remvoing it on double click but it's awkward when double clicking away from an existing point:
    Screen.Recording.2025-08-15.at.1.12.21.pm.mov
  • Some wonkiness when clicking on the top-right point:
    Screen.Recording.2025-08-15.at.1.13.41.pm.mov

Comment on lines +111 to +127
const slider = useMemo(
() =>
({
row: (label: string, value: number, onChange: (v: number) => void, min = -1, max = 1, step = 0.01) => (
<FormControl pr={2}>
<Flex alignItems="center" gap={3} mb={1}>
<FormLabel m={0} flexShrink={0} minW="90px">
{label}
</FormLabel>
<CompositeNumberInput value={value} onChange={onChange} min={min} max={max} step={step} flex="0 0 96px" />
</Flex>
<CompositeSlider value={value} onChange={onChange} min={min} max={max} step={step} marks />
</FormControl>
),
}) as const,
[]
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just be a separate component not a memoized object

Comment on lines +50 to +76
const width = 256;
const height = 160;
// inner margins to keep a small buffer from edges (left/right/bottom)
const MARGIN_LEFT = 8;
const MARGIN_RIGHT = 8;
const MARGIN_TOP = 8;
const MARGIN_BOTTOM = 10;
const INNER_WIDTH = width - MARGIN_LEFT - MARGIN_RIGHT;
const INNER_HEIGHT = height - MARGIN_TOP - MARGIN_BOTTOM;

// helpers to map value-space [0..255] to canvas pixels (respecting inner margins)
const valueToCanvasX = useCallback(
(x: number) => MARGIN_LEFT + (clamp(x, 0, 255) / 255) * INNER_WIDTH,
[INNER_WIDTH]
);
const valueToCanvasY = useCallback(
(y: number) => MARGIN_TOP + INNER_HEIGHT - (clamp(y, 0, 255) / 255) * INNER_HEIGHT,
[INNER_HEIGHT]
);
const canvasToValueX = useCallback(
(cx: number) => clamp(Math.round(((cx - MARGIN_LEFT) / INNER_WIDTH) * 255), 0, 255),
[INNER_WIDTH]
);
const canvasToValueY = useCallback(
(cy: number) => clamp(Math.round(255 - ((cy - MARGIN_TOP) / INNER_HEIGHT) * 255), 0, 255),
[INNER_HEIGHT]
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of these can be extracted outside the component.

);

return (
<div style={{ display: 'flex', flexDirection: 'column', gap: 4 }}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the chakra components e.g. <Flex flexDir='column' gap={4} />

Comment on lines +328 to +330
const layer = useAppSelector((s) => selectEntity(s.canvas.present, entityIdentifier)) as
| CanvasRasterLayerState
| undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type cast here is unnecessary, selectEntity already narrows it based on the identifier (which is itself narrowed).

const entityIdentifier = useEntityIdentifierContext<'raster_layer'>();
const adapter = useEntityAdapterContext<'raster_layer'>('raster_layer');
const { t } = useTranslation();
const layer = useAppSelector((s) => selectEntity(s.canvas.present, entityIdentifier)) as
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates a new selector function on every render, which borks memoization. Instead, create a selector wtih a stable ref in memory:

  const selectLayer = useMemo(
    () => createSelector(selectCanvasSlice, (canvas) => selectEntity(canvas, entityIdentifier)),
    [entityIdentifier]
  );
  const layer = useAppSelector(selectLayer);

Comment on lines +120 to +122
<CompositeNumberInput value={value} onChange={onChange} min={min} max={max} step={step} flex="0 0 96px" />
</Flex>
<CompositeSlider value={value} onChange={onChange} min={min} max={max} step={step} marks />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shoudl add defaultValue to each of these

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest splitting this component into 3 components:

  • Main adjustments panel
  • Simple adjustment editor
  • Curves adjustment editor

Comment on lines +47 to +53
const onToggleEnabled = useCallback(
(v: boolean) => {
// Only toggle the enabled state; preserve current mode/collapsed so users can A/B compare
dispatch(rasterLayerAdjustmentsSet({ entityIdentifier, adjustments: { enabled: v } }));
},
[dispatch, entityIdentifier]
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic and be merged in to handleToggleEnabled as it is not used anywhere else

Adjustments
</Text>
<ButtonGroup size="sm" isAttached variant="outline">
<Button onClick={onClickModeSimple} isActive={mode === 'simple'}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Button onClick={onClickModeSimple} isActive={mode === 'simple'}>
<Button onClick={onClickModeSimple} colorScheme={mode === 'simple' ? 'invokeBlue' : undefined}>

[255, 255],
];

type Channel = 'master' | 'r' | 'g' | 'b';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the zod schemas and types are split up as suggested below then we can use them as the source of truth for this type and others in this file

@dunkeroni
Copy link
Contributor Author

Thanks for the thorough feedback while I bumble my way through frontend code like a hog in a chandelier showroom. It'll take me some time to work through it, and I might ping you for thoughts as I go.

Some points for right now:

  • There's no way to dismiss the adjustment without applying/finishing it. And finishing always rasterizes the layer. Suggest comparing selected adjust mode and its settings and if unchanged from defaults, when finishing, do not rasterize

It can be removed without finishing with the Right Click menu. I had wanted a Cancel button, but it's already quite cluttered when the panel is shrunk, and Reset felt more important to have there. Another trashcan/X button on the layer is small but might trip users up.
Maybe the Simple/Curves selector should be moved down into the collapsible area to keep Enable/Reset/Finish/Cancel at the top? Not super happy with the layout as it is. Also it's more colorful than other layers are, but that kinda helps because it nags my attention to either finish or remove it instead of leaving them on.
image

  • Layout is a bit wonky when panel is at min width - also needs some padding:

This one confused me because I was sure I checked that, but it turns out to be browser-specific.
Fine on Chrome:
Screenshot From 2025-08-15 01-57-58

Wonky on Firefox:
Screenshot From 2025-08-15 01-57-47
I'm guessing there is some better more general way that I should be defining those to fit correctly in all cases. I'll play with it and check on both for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants