Merge pull request #59716 from nextcloud/jtr/fix-color2

fix(Color): blueF() to return correct channel and tighten validation/docs

Author: susnux

lib/composer/composer/autoload_classmap.php

@@ -1388,7 +1388,6 @@
     'OC\\Collaboration\\Resources\\Manager' => $baseDir . '/lib/private/Collaboration/Resources/Manager.php',
     'OC\\Collaboration\\Resources\\ProviderManager' => $baseDir . '/lib/private/Collaboration/Resources/ProviderManager.php',
     'OC\\Collaboration\\Resources\\Resource' => $baseDir . '/lib/private/Collaboration/Resources/Resource.php',
-    'OC\\Color' => $baseDir . '/lib/private/Color.php',
     'OC\\Command\\AsyncBus' => $baseDir . '/lib/private/Command/AsyncBus.php',
     'OC\\Command\\CommandJob' => $baseDir . '/lib/private/Command/CommandJob.php',
     'OC\\Command\\CronBus' => $baseDir . '/lib/private/Command/CronBus.php',

lib/composer/composer/autoload_static.php

@@ -11,32 +11,32 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
     );
 
     public static $prefixLengthsPsr4 = array (
-        'O' => 
+        'O' =>
         array (
             'OC\\Core\\' => 8,
             'OC\\' => 3,
             'OCP\\' => 4,
         ),
-        'N' => 
+        'N' =>
         array (
             'NCU\\' => 4,
         ),
     );
 
     public static $prefixDirsPsr4 = array (
-        'OC\\Core\\' => 
+        'OC\\Core\\' =>
         array (
             0 => __DIR__ . '/../../..' . '/core',
         ),
-        'OC\\' => 
+        'OC\\' =>
         array (
             0 => __DIR__ . '/../../..' . '/lib/private',
         ),
-        'OCP\\' => 
+        'OCP\\' =>
         array (
             0 => __DIR__ . '/../../..' . '/lib/public',
         ),
-        'NCU\\' => 
+        'NCU\\' =>
         array (
             0 => __DIR__ . '/../../..' . '/lib/unstable',
         ),
@@ -1429,7 +1429,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Collaboration\\Resources\\Manager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Manager.php',
         'OC\\Collaboration\\Resources\\ProviderManager' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/ProviderManager.php',
         'OC\\Collaboration\\Resources\\Resource' => __DIR__ . '/../../..' . '/lib/private/Collaboration/Resources/Resource.php',
-        'OC\\Color' => __DIR__ . '/../../..' . '/lib/private/Color.php',
         'OC\\Command\\AsyncBus' => __DIR__ . '/../../..' . '/lib/private/Command/AsyncBus.php',
         'OC\\Command\\CommandJob' => __DIR__ . '/../../..' . '/lib/private/Command/CommandJob.php',
         'OC\\Command\\CronBus' => __DIR__ . '/../../..' . '/lib/private/Command/CronBus.php',

lib/private/Color.php

@@ -1,5 +1,6 @@
 <?php
 
+declare(strict_types=1);
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -9,20 +10,21 @@
 
 /**
  * Simple RGB color container
+ *
  * @since 25.0.0
  */
 class Color {
-	private int $r;
-	private int $g;
-	private int $b;
-
 	/**
 	 * @since 25.0.0
 	 */
-	public function __construct($r, $g, $b) {
-		$this->r = $r;
-		$this->g = $g;
-		$this->b = $b;
+	public function __construct(
+		private int $r,
+		private int $g,
+		private int $b,
+	) {
+		self::assertChannelInRange($this->r, 'r');
+		self::assertChannelInRange($this->g, 'g');
+		self::assertChannelInRange($this->b, 'b');
 	}
 
 	/**
@@ -62,7 +64,7 @@ public function greenF(): float {
 	}
 
 	/**
-	 * Returns the green blue component of this color as an int from 0 to 255
+	 * Returns the blue color component of this color as an int from 0 to 255
 	 *
 	 * @since 25.0.0
 	 */
@@ -76,65 +78,99 @@ public function blue(): int {
 	 * @since 25.0.0
 	 */
 	public function blueF(): float {
-		return $this->g / 255;
+		return $this->b / 255;
 	}
 
 	/**
-	 * Returns the name of the color in the format "#RRGGBB"; i.e. a "#" character followed by three two-digit hexadecimal numbers.
+	 * Returns the hex triplet color value as a string ("#RRGGBB")
 	 *
 	 * @since 25.0.0
 	 */
 	public function name(): string {
 		return sprintf('#%02x%02x%02x', $this->r, $this->g, $this->b);
 	}
 
+	// Utility Functions
+
 	/**
-	 * Mix two colors
+	 * Generate a progression of colors starting with $color1 and moving toward $color2.
 	 *
-	 * @param int $steps the number of intermediate colors that should be generated for the palette
-	 * @param Color $color1 the first color
-	 * @param Color $color2 the second color
-	 * @return list<Color>
+	 * @param int $steps Total number of colors to return (including $color1, but excluding $color2); should be at least 2
+	 * @param Color $color1 The starting color (index 0 of the returned list)
+	 * @param Color $color2 The target color used to calculate the transition
+	 * @return list<Color> The list of colors starting with $color1 up to but not including $color2
 	 * @since 25.0.0
 	 */
 	public static function mixPalette(int $steps, Color $color1, Color $color2): array {
+		if ($steps < 1) {
+			// 1 is a hard requirement; 2 is a practical requirement
+			throw new \InvalidArgumentException('Palette steps must be at least 1 (and should be at least 2).');
+		}
+
 		$palette = [$color1];
-		$step = self::stepCalc($steps, [$color1, $color2]);
+		[$rDelta, $gDelta, $bDelta] = self::calculateDeltas($steps, $color1, $color2);
+
 		for ($i = 1; $i < $steps; $i++) {
-			$r = intval($color1->red() + ($step[0] * $i));
-			$g = intval($color1->green() + ($step[1] * $i));
-			$b = intval($color1->blue() + ($step[2] * $i));
-			$palette[] = new Color($r, $g, $b);
+			$palette[] = new Color(
+				// TODO: Consider using round() instead of (int) truncation for more accurate color transitions.
+				(int)($color1->red() + ($rDelta * $i)),
+				(int)($color1->green() + ($gDelta * $i)),
+				(int)($color1->blue() + ($bDelta * $i)),
+			);
 		}
+
 		return $palette;
 	}
 
 	/**
-	 * Alpha blend another color with a given opacity to this color
+	 * Blend this color over a source color.
+	 *
+	 * An opacity of 0 returns $source, and 1 returns this color.
 	 *
-	 * @return Color The new color
+	 * @param float $opacity Opacity of this color, expected in the range 0.0 to 1.0
+	 * @param Color $source The source/background color
+	 * @return Color The blended color
 	 * @since 25.0.0
 	 */
 	public function alphaBlending(float $opacity, Color $source): Color {
+		if ($opacity < 0.0 || $opacity > 1.0) {
+			throw new \InvalidArgumentException('Opacity must be between 0.0 and 1.0.');
+		}
+
 		return new Color(
+			// TODO: Consider using round() instead of (int) truncation for more accurate color transitions.
 			(int)((1 - $opacity) * $source->red() + $opacity * $this->red()),
 			(int)((1 - $opacity) * $source->green() + $opacity * $this->green()),
 			(int)((1 - $opacity) * $source->blue() + $opacity * $this->blue())
 		);
 	}
 
 	/**
-	 * Calculate steps between two Colors
-	 * @param int $steps start color
-	 * @param Color[] $ends end color
-	 * @return array{0: float, 1: float, 2: float} [r,g,b] steps for each color to go from $steps to $ends
+	 * Calculate the per-channel change (RGB deltas) required to transition between two colors.
+	 *
+	 * @param int $count The number of intervals to divide the transition into >0
+	 * @param Color $start The starting color
+	 * @param Color $end The target color
+	 * @return array{0: float, 1: float, 2: float} The per-channel [r, g, b] increment required for each interval
 	 * @since 25.0.0
 	 */
-	private static function stepCalc(int $steps, array $ends): array {
-		$step = [];
-		$step[0] = ($ends[1]->red() - $ends[0]->red()) / $steps;
-		$step[1] = ($ends[1]->green() - $ends[0]->green()) / $steps;
-		$step[2] = ($ends[1]->blue() - $ends[0]->blue()) / $steps;
-		return $step;
+	private static function calculateDeltas(int $count, Color $start, Color $end): array {
+		$deltas = [];
+
+		$deltas[0] = ($end->red() - $start->red()) / $count;
+		$deltas[1] = ($end->green() - $start->green()) / $count;
+		$deltas[2] = ($end->blue() - $start->blue()) / $count;
+
+		return $deltas;
+	}
+
+	private static function assertChannelInRange(int $value, string $channel): void {
+		if ($value < 0 || $value > 255) {
+			throw new \InvalidArgumentException(sprintf(
+				'Color channel "%s" must be between 0 and 255, got %d.',
+				$channel,
+				$value,
+			));
+		}
 	}
 }

lib/public/Color.php

@@ -263,9 +263,9 @@ public function testMixPalette(): void {
 		$palette = Color::mixPalette($steps, $colorFrom, $colorTo);
 		foreach ($palette as $j => $color) {
 			// calc increment
-			$incR = $colorTo->red() / $steps * $j;
-			$incG = $colorTo->green() / $steps * $j;
-			$incB = $colorTo->blue() / $steps * $j;
+			$incR = (int)($colorTo->red() / $steps * $j);
+			$incG = (int)($colorTo->green() / $steps * $j);
+			$incB = (int)($colorTo->blue() / $steps * $j);
 			// ensure everything is equal
 			$this->assertEquals($color, new Color($incR, $incG, $incB));
 		}