New Node: Sharpen node - #4516
Conversation
There was a problem hiding this comment.
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
| let amount = amount / 100.; | ||
| let threshold = threshold / 255.; | ||
| // Width of the linear transition around the threshold | ||
| let threshold_fade_width = threshold * 0.75; |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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.
This PR adds a new sharpen node to the Raster: Filter category, allowing users to sharpen images using unsharp masking.
Changes made
Notes on implementation
Part of #912