Fix gradient crash on non-RGB NSColor and add regression test by probonopd · Pull Request #69 · gnustep/libs-back

probonopd · 2026-01-27T23:03:48Z

Fixes an uncaught NSInternalInconsistencyException when gradients include non-RGB colors (e.g., pattern or named color lists) by normalizing colors to an RGB-compatible color space before accessing components in CairoGState (libs-back).
Adds a regression test libs-back/Tests/gradient/gradient_test.m that creates a non-RGB color (pattern) and draws an NSGradient into an image with lockFocus to ensure no exception is raised.

Testing:

Built and installed libs-back and verified the assistant no longer throws the "Called redComponent on non-RGB colour" exception in UI runs.
The regression test is included and can be built via gmake -C libs-back/Tests/gradient (or by running its tool target) — it draws a gradient into an offscreen image and exits non-zero on failure.

…for non-RGB gradient

Copilot

Pull request overview

This PR fixes a crash that occurs when gradients contain non-RGB colors (such as pattern or named color lists) by implementing color space normalization before accessing RGB components. The crash was caused by NSColor throwing an exception when RGB component accessors were called on colors in incompatible color spaces.

Changes:

Adds color space normalization logic to both radial and linear gradient drawing methods in CairoGState.m, with fallback handling for colors that cannot be converted to RGB
Introduces a regression test that creates a pattern-based gradient and verifies it can be drawn without exceptions
Adds test infrastructure with a GNUmakefile for building the gradient test

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
Tests/gradient/gradient_test.m	New regression test that creates a pattern color and draws it in a gradient to verify the fix
Tests/gradient/GNUmakefile	Build configuration for the gradient regression test tool
Source/cairo/CairoGState.m	Implements color normalization to RGB-compatible color spaces before accessing color components, with try-catch error handling for both radial and linear gradient methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T23:09:31Z

Source/cairo/CairoGState.m

+          /* Ensure color is in an RGB-compatible colorspace to avoid
+             -redComponent/-greenComponent/-blueComponent raising
+             for non-RGB colors (patterns, color lists, etc.). Try
+             device RGB first, then calibrated RGB, and finally
+             fall back to a safe opaque black if conversion fails. */
+          NSColor *rgbColor = [color colorUsingColorSpaceName: NSDeviceRGBColorSpace];
+          if (!rgbColor) rgbColor = [color colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
+          if (!rgbColor) rgbColor = [NSColor colorWithCalibratedWhite:0.0 alpha:1.0];
+
+          @try {
+            red = [rgbColor redComponent];
+            green = [rgbColor greenComponent];
+            blue = [rgbColor blueComponent];
+            alpha = [rgbColor alphaComponent];
+          } @catch (NSException *ex) {
+            /* Last-resort safe defaults */
+            red = green = blue = 0.0;
+            alpha = 1.0;
+          }


Code duplication: The color normalization logic (lines 1587-1605) is duplicated in the second gradient method (lines 1653-1666). This code should be extracted into a helper method to avoid duplication and ensure both gradient drawing methods handle color conversion consistently. This would improve maintainability and reduce the risk of divergence if the logic needs to be updated in the future.

Copilot · 2026-01-27T23:09:31Z

Tests/gradient/gradient_test.m

+    NSImage *img = [[NSImage alloc] initWithSize:NSMakeSize(100,100)];
+    @try {
+        [img lockFocus];
+        [g drawInRect:NSMakeRect(0,0,100,100) angle:90.0];


Incomplete test coverage: The test only exercises the drawInRect:angle: method (which calls the fromPoint:toPoint: variant internally). It should also test the radial gradient method (fromCenter:radius:toCenter:radius:) to ensure both code paths in CairoGState.m are covered, since both were modified to handle non-RGB colors.

Copilot · 2026-01-27T23:09:31Z

Source/cairo/CairoGState.m

+            blue = [rgbColor blueComponent];
+            alpha = [rgbColor alphaComponent];
+          } @catch (NSException *ex) {
+            /* Last-resort safe defaults */


Silent error handling may hide issues: When color conversion fails and the catch block assigns default black color values (red = green = blue = 0.0, alpha = 1.0), no warning or log message is generated. This makes debugging difficult if unexpected color conversions occur. Consider adding an NSLog statement in the catch block to help diagnose color conversion failures, similar to other error logging patterns in this file (e.g., line 160, 518, 567).

Suggested change

/* Last-resort safe defaults */

/* Last-resort safe defaults */

NSLog(@"[CairoGState] Failed to convert color %@ to RGB: %@. Using default black.",

color, ex);

Copilot · 2026-01-27T23:09:32Z

Tests/gradient/gradient_test.m

@@ -0,0 +1,31 @@
+#import <Foundation/Foundation.h>


Missing copyright header: The test file lacks a copyright and license header. Based on the convention seen throughout this repository (e.g., Tools/font_cacher.m, Source files), source files typically include a copyright header with LGPL license information. This should be added for consistency with the rest of the codebase.

Copilot · 2026-01-27T23:09:32Z

Tests/gradient/gradient_test.m

+    NSImage *pat = [[NSImage alloc] initWithSize:NSMakeSize(4,4)];
+    [pat lockFocus];
+    [[NSColor colorWithCalibratedRed:1.0 green:0.0 blue:0.0 alpha:1.0] setFill];
+    NSRectFill(NSMakeRect(0,0,4,4));
+    [pat unlockFocus];
+
+    NSColor *pattern = [NSColor colorWithPatternImage:pat];
+
+    NSGradient *g = [[NSGradient alloc] initWithStartingColor:pattern endingColor:[NSColor colorWithCalibratedWhite:1.0 alpha:1.0]];
+
+    NSImage *img = [[NSImage alloc] initWithSize:NSMakeSize(100,100)];


Memory leak: The test creates several objects (pat, pattern, g, img) using alloc/init but never releases them. While the autorelease pool will drain at the end, these objects should be explicitly released before draining the pool to follow proper memory management practices. This is especially important for a test that's meant to verify correct behavior without side effects.

Copilot · 2026-01-27T23:09:32Z

Tests/gradient/gradient_test.m

+    NSImage *img = [[NSImage alloc] initWithSize:NSMakeSize(100,100)];
+    @try {
+        [img lockFocus];
+        [g drawInRect:NSMakeRect(0,0,100,100) angle:90.0];
+        [img unlockFocus];


The test creates an NSImage and calls lockFocus/unlockFocus on it, but this may fail or not work correctly in a headless/non-GUI environment where no display connection is available. The test should either check if the application can be initialized properly, or handle the case where lockFocus might fail. Consider adding a check or using NSGraphicsContext directly to ensure the test can run in various environments.

Copilot · 2026-01-27T23:09:32Z

Source/cairo/CairoGState.m

+            green = [rgbColor greenComponent];
+            blue = [rgbColor blueComponent];
+            alpha = [rgbColor alphaComponent];
+          } @catch (NSException *ex) {


Silent error handling may hide issues: When color conversion fails and the catch block assigns default black color values, no warning or log message is generated. This makes debugging difficult if unexpected color conversions occur. Consider adding an NSLog statement in the catch block to help diagnose color conversion failures, similar to other error logging patterns in this file (e.g., line 160, 518, 567).

Suggested change

} @catch (NSException *ex) {

} @catch (NSException *ex) {

NSLog(@"CairoGState: Failed to convert color %@ to RGB components for gradient stop at location %f: %@ - %@",

rgbColor,

location,

[ex name],

[ex reason]);

Copilot · 2026-01-27T23:09:33Z

Tests/gradient/GNUmakefile

@@ -0,0 +1,9 @@
+include $(GNUSTEP_MAKEFILES)/common.make


Missing copyright header: The GNUmakefile lacks a copyright and license header. Based on the convention seen in Tools/GNUmakefile, makefiles in this repository typically include a copyright header with LGPL license information. This should be added for consistency with the rest of the codebase.

Copilot · 2026-01-27T23:09:33Z

Tests/gradient/gradient_test.m

@@ -0,0 +1,31 @@
+#import <Foundation/Foundation.h>


Discrepancy with PR description: The PR description mentions "Modified: gershwin-components/Assistants/AssistantFramework/GSAssistantWindow.m (readded guard)" as one of the changed files. However, this file is not included in the pull request. Either the file should be included or the PR description should be updated to accurately reflect the files being changed.

cairo: normalize gradient stop colors to RGB and add regression test …

126059b

…for non-RGB gradient

probonopd requested a review from fredkiefer as a code owner January 27, 2026 23:03

Copilot AI review requested due to automatic review settings January 27, 2026 23:03

Copilot started reviewing on behalf of probonopd January 27, 2026 23:04 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix gradient crash on non-RGB NSColor and add regression test#69

Fix gradient crash on non-RGB NSColor and add regression test#69
probonopd wants to merge 1 commit intognustep:masterfrom
probonopd:fix/gradient-normalize-rgb

probonopd commented Jan 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Comments

-          } @catch (NSException *ex) {
+          } @catch (NSException *ex) {
+            NSLog(@"CairoGState: Failed to convert color %@ to RGB components for gradient stop at location %f: %@ - %@",
+                  rgbColor,
+                  location,
+                  [ex name],
+                  [ex reason]);

Conversation

probonopd commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Comments

probonopd commented Jan 27, 2026 •

edited

Loading