Skip to content

New Node: Sharpen node - #4516

Open
Zadfar wants to merge 4 commits into
GraphiteEditor:masterfrom
Zadfar:add-sharpen-filter
Open

New Node: Sharpen node#4516
Zadfar wants to merge 4 commits into
GraphiteEditor:masterfrom
Zadfar:add-sharpen-filter

Conversation

@Zadfar

@Zadfar Zadfar commented Sep 10, 2026

Copy link
Copy Markdown

This PR adds a new sharpen node to the Raster: Filter category, allowing users to sharpen images using unsharp masking.

Changes made

  • Added new sharpen node with three inputs (amount, radius and threshold)
  • Implemented sharpen_algorithm() which uses the unsharp mask technique to sharpen images
  • Made a new fn for PremultipliedGammaPixel called to_unpremultiplied_channels(), as this was required by both the sharpening algorithm and gaussian blur algorithm

Notes on implementation

  • This implementation borrows heavily from how photoshop did unsharp mask (inputs and ranges), i thought this might be more intuitive for users
  • I opted to use the gaussian blur in gamma space as this provided the output most similar to unsharp mask tools from other software.
  • There is a hard limit on the threshold variable (255), this comes from the maximum constrast difference in 8bit images
  • The soft limits of 100%, 50px and 30 for amount, radius and threshold are arbitrary and can be changed if necessary

Part of #912

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file

Confidence score: 4/5

  • In node-graph/nodes/raster/src/filter.rs, the threshold fade begins applying sharpening below the configured threshold, so the default threshold of 30 does not behave as an exact cutoff; verify or adjust the fade mask boundaries.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/nodes/raster/src/filter.rs">

<violation number="1" location="node-graph/nodes/raster/src/filter.rs:397">
P2: The threshold fade (`threshold * 0.75`) begins applying sharpening below the configured threshold, because the mask only reaches 0 at `diff.abs() <= threshold - fade_width`. At the default threshold of 30 this sharpens pixels that differ by ~8/255, and at high threshold values the deviation grows — contradicting the parameter's documented 'before sharpening is applied' behavior. Consider gating the mask at 0 for `diff.abs() <= threshold` while keeping fade only above the threshold (e.g. ramp from `threshold` to `threshold + fade_width`) if the intent is an unsharp-mask-style hard gate.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread node-graph/nodes/raster/src/filter.rs
Comment thread node-graph/nodes/raster/src/filter.rs Outdated
let amount = amount / 100.;
let threshold = threshold / 255.;
// Width of the linear transition around the threshold
let threshold_fade_width = threshold * 0.75;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The threshold fade (threshold * 0.75) begins applying sharpening below the configured threshold, because the mask only reaches 0 at diff.abs() <= threshold - fade_width. At the default threshold of 30 this sharpens pixels that differ by ~8/255, and at high threshold values the deviation grows — contradicting the parameter's documented 'before sharpening is applied' behavior. Consider gating the mask at 0 for diff.abs() <= threshold while keeping fade only above the threshold (e.g. ramp from threshold to threshold + fade_width) if the intent is an unsharp-mask-style hard gate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/raster/src/filter.rs, line 397:

<comment>The threshold fade (`threshold * 0.75`) begins applying sharpening below the configured threshold, because the mask only reaches 0 at `diff.abs() <= threshold - fade_width`. At the default threshold of 30 this sharpens pixels that differ by ~8/255, and at high threshold values the deviation grows — contradicting the parameter's documented 'before sharpening is applied' behavior. Consider gating the mask at 0 for `diff.abs() <= threshold` while keeping fade only above the threshold (e.g. ramp from `threshold` to `threshold + fade_width`) if the intent is an unsharp-mask-style hard gate.</comment>

<file context>
@@ -341,3 +384,40 @@ fn median_quickselect(values: &mut [f32]) -> f32 {
+	let amount = amount / 100.;
+	let threshold = threshold / 255.;
+	// Width of the linear transition around the threshold
+	let threshold_fade_width = threshold * 0.75;
+
+	let sharpen_channel = |orig: f32, blur: f32| -> f32 {
</file context>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The unsharp mask style hard gate mentioned here produces output that differs from the output of other software like photoshop and gimp. The threshold_fade_width is a hacky way to get output that looks similar.

Comment thread node-graph/nodes/raster/src/filter.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Confidence score: 3/5

  • In node-graph/nodes/raster/src/filter.rs, translucent pixels can be darkened before sharpening because premultiplied linear RGB is gamma-encoded directly, producing visible halos or incorrect colors; unassociate each Color before gamma encoding, then premultiply the gamma-space values.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/nodes/raster/src/filter.rs">

<violation number="1" location="node-graph/nodes/raster/src/filter.rs:403">
P1: For translucent pixels, this gamma-space working buffer darkens RGB before sharpening because it gamma-encodes premultiplied linear channels. Unassociate each `Color` before gamma encoding, then premultiply the gamma channels once.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread node-graph/nodes/raster/src/filter.rs Outdated
.data
.iter()
.map(|c| {
let [r, g, b, a] = c.to_gamma_srgb_channels();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: For translucent pixels, this gamma-space working buffer darkens RGB before sharpening because it gamma-encodes premultiplied linear channels. Unassociate each Color before gamma encoding, then premultiply the gamma channels once.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/raster/src/filter.rs, line 403:

<comment>For translucent pixels, this gamma-space working buffer darkens RGB before sharpening because it gamma-encodes premultiplied linear channels. Unassociate each `Color` before gamma encoding, then premultiply the gamma channels once.</comment>

<file context>
@@ -386,13 +386,33 @@ fn median_quickselect(values: &mut [f32]) -> f32 {
+		.data
+		.iter()
+		.map(|c| {
+			let [r, g, b, a] = c.to_gamma_srgb_channels();
+			PremultipliedGammaPixel { r: r * a, g: g * a, b: b * a, a }
+		})
</file context>
Suggested change
let [r, g, b, a] = c.to_gamma_srgb_channels();
let [r, g, b, a] = c.to_unassociated_alpha().to_gamma_srgb_channels();

Comment thread node-graph/nodes/raster/src/filter.rs Outdated
@Zadfar
Zadfar marked this pull request as draft September 11, 2026 10:20
@Zadfar
Zadfar marked this pull request as ready for review September 11, 2026 11:52

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file

Confidence score: 3/5

  • In node-graph/nodes/raster/src/filter.rs, sharpening compares gamma-encoded premultiplied original RGB against un-premultiplied blurred values, which can produce incorrect sharpening results; make the premultiplication and color-space representation consistent before comparison.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/nodes/raster/src/filter.rs">

<violation number="1" location="node-graph/nodes/raster/src/filter.rs:417">
P2: The sharpening compares gamma values of the premultiplied original against un-premultiplied blur values. original.to_gamma_srgb_channels() gamma-encodes the premultiplied linear RGB stored in Color, while blurred.to_unpremultiplied_channels() is un-premultiplied. For semi-transparent pixels these representations diverge (the original is scaled by alpha, the blur is not), so the diff and threshold mask are computed on mismatched data, which can produce fringing/halos at soft alpha edges. Un-premultiply the original channels as well before computing the diff, so both sides are unassociated.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

};

for (original, blurred) in buffer.data.iter_mut().zip(&blurred_image.data) {
let [original_r, original_g, original_b, original_a] = original.to_gamma_srgb_channels();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The sharpening compares gamma values of the premultiplied original against un-premultiplied blur values. original.to_gamma_srgb_channels() gamma-encodes the premultiplied linear RGB stored in Color, while blurred.to_unpremultiplied_channels() is un-premultiplied. For semi-transparent pixels these representations diverge (the original is scaled by alpha, the blur is not), so the diff and threshold mask are computed on mismatched data, which can produce fringing/halos at soft alpha edges. Un-premultiply the original channels as well before computing the diff, so both sides are unassociated.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/raster/src/filter.rs, line 417:

<comment>The sharpening compares gamma values of the premultiplied original against un-premultiplied blur values. original.to_gamma_srgb_channels() gamma-encodes the premultiplied linear RGB stored in Color, while blurred.to_unpremultiplied_channels() is un-premultiplied. For semi-transparent pixels these representations diverge (the original is scaled by alpha, the blur is not), so the diff and threshold mask are computed on mismatched data, which can produce fringing/halos at soft alpha edges. Un-premultiply the original channels as well before computing the diff, so both sides are unassociated.</comment>

<file context>
@@ -341,3 +384,47 @@ fn median_quickselect(values: &mut [f32]) -> f32 {
+	};
+
+	for (original, blurred) in buffer.data.iter_mut().zip(&blurred_image.data) {
+		let [original_r, original_g, original_b, original_a] = original.to_gamma_srgb_channels();
+		let [blurred_r, blurred_g, blurred_b, _] = blurred.to_unpremultiplied_channels();
+
</file context>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is how it is handled in gaussian blur. so fixing this here would require a fix there as well. I dont know enough about this particular issue and the chances it will occur while using normally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant