Skip to content

Commit 9a13f52

Browse files
oleibmanoleibman
authored
Redo Calculation of Color Tinting (#3580)
* Redo Calculation of Color Tinting Fix #3550. Some colors are specified in Excel by specifying a theme color to which a tint is applied. The original PHPExcel algorithm for doing this was developed by trial and error, and is good enough a lot of the time. However, for the issue at hand, the resulting color is detectably different from the calculation that Excel makes. Searching the web, I found https://gist.github.com/Mike-Honey/b36e651e9a7f1d2e1d60ce1c63b9b633 which comes much closer for the case in hand, and for all the other cases that I've looked at. That code depends on Python colorsys package; I have adapted the code from the Python gist and package into a new Php class. This doesn't agree perfectly with Excel. However, if each of the red, green, and blue components (each a value between 0 and 255 inclusive) agree within plus or minus 3 (arbitrary choice) of Excel's result, I think that is good enough. I have added a new test member which reads from a spreadsheet with Xml altered by hand to set up several theme/tint cells. These tests use the plus-or-minus-3 criterion. They result in 100% code coverage of the new class. Unsuprisingly, some existing tests failed with the new code. Issue2387Test reads a theme/tint font color, and is changed to use the plus-or-minus-3 criterion, comparing against the color as Excel shows it. ColorChangeBrightness showed 9 failures with the new code. It consists of calculations not involving a spreadsheet. For that reason, I felt it was sufficient to just do an exact match test, changing the 9 old results for new results confirmed with the Python code. I also added one new test case, the one that kicked off this entire PR. * Scrutinizer Being Stupid It strikes again.
1 parent 407479f commit 9a13f52

File tree

6 files changed

+245
-26
lines changed

6 files changed

+245
-26
lines changed

‎src/PhpSpreadsheet/Style/Color.php‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -362,23 +362,8 @@ public static function changeBrightness($hexColourValue, $adjustPercentage)
362362
$green = self::getGreen($hexColourValue, false);
363363
/** @var int $blue */
364364
$blue = self::getBlue($hexColourValue, false);
365-
if ($adjustPercentage > 0) {
366-
$red += (255 - $red) * $adjustPercentage;
367-
$green += (255 - $green) * $adjustPercentage;
368-
$blue += (255 - $blue) * $adjustPercentage;
369-
} else {
370-
$red += $red * $adjustPercentage;
371-
$green += $green * $adjustPercentage;
372-
$blue += $blue * $adjustPercentage;
373-
}
374-
375-
$rgb = strtoupper(
376-
str_pad(dechex((int) $red), 2, '0', 0) .
377-
str_pad(dechex((int) $green), 2, '0', 0) .
378-
str_pad(dechex((int) $blue), 2, '0', 0)
379-
);
380365

381-
return (($rgba) ? 'FF' : '') . $rgb;
366+
return (($rgba) ? 'FF' : '') . RgbTint::rgbAndTintToRgb($red, $green, $blue, $adjustPercentage);
382367
}
383368

384369
/**

‎src/PhpSpreadsheet/Style/RgbTint.php‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheet\Style;
4+
5+
/**
6+
* Class to handle tint applied to color.
7+
* Code borrows heavily from some Python projects.
8+
*
9+
* @see https://docs.python.org/3/library/colorsys.html
10+
* @see https://gist.github.com/Mike-Honey/b36e651e9a7f1d2e1d60ce1c63b9b633
11+
*/
12+
class RgbTint
13+
{
14+
private const ONE_THIRD = 1.0 / 3.0;
15+
private const ONE_SIXTH = 1.0 / 6.0;
16+
private const TWO_THIRD = 2.0 / 3.0;
17+
private const RGBMAX = 255.0;
18+
/**
19+
* MS excel's tint function expects that HLS is base 240.
20+
*
21+
* @see https://social.msdn.microsoft.com/Forums/en-US/e9d8c136-6d62-4098-9b1b-dac786149f43/excel-color-tint-algorithm-incorrect?forum=os_binaryfile#d3c2ac95-52e0-476b-86f1-e2a697f24969
22+
*/
23+
private const HLSMAX = 240.0;
24+
25+
/**
26+
* Convert red/green/blue to hue/luminance/saturation.
27+
*
28+
* @param float $red 0.0 through 1.0
29+
* @param float $green 0.0 through 1.0
30+
* @param float $blue 0.0 through 1.0
31+
*
32+
* @return float[]
33+
*/
34+
private static function rgbToHls(float $red, float $green, float $blue): array
35+
{
36+
$maxc = max($red, $green, $blue);
37+
$minc = min($red, $green, $blue);
38+
$luminance = ($minc + $maxc) / 2.0;
39+
if ($minc === $maxc) {
40+
return [0.0, $luminance, 0.0];
41+
}
42+
$maxMinusMin = $maxc - $minc;
43+
if ($luminance <= 0.5) {
44+
$s = $maxMinusMin / ($maxc + $minc);
45+
} else {
46+
$s = $maxMinusMin / (2.0 - $maxc - $minc);
47+
}
48+
$rc = ($maxc - $red) / $maxMinusMin;
49+
$gc = ($maxc - $green) / $maxMinusMin;
50+
$bc = ($maxc - $blue) / $maxMinusMin;
51+
if ($red === $maxc) {
52+
$h = $bc - $gc;
53+
} elseif ($green === $maxc) {
54+
$h = 2.0 + $rc - $bc;
55+
} else {
56+
$h = 4.0 + $gc - $rc;
57+
}
58+
$h = self::positiveDecimalPart($h / 6.0);
59+
60+
return [$h, $luminance, $s];
61+
}
62+
63+
/** @var mixed */
64+
private static $scrutinizerZeroPointZero = 0.0;
65+
66+
/**
67+
* Convert hue/luminance/saturation to red/green/blue.
68+
*
69+
* @param float $hue 0.0 through 1.0
70+
* @param float $luminance 0.0 through 1.0
71+
* @param float $saturation 0.0 through 1.0
72+
*
73+
* @return float[]
74+
*/
75+
private static function hlsToRgb($hue, $luminance, $saturation): array
76+
{
77+
if ($saturation === self::$scrutinizerZeroPointZero) {
78+
return [$luminance, $luminance, $luminance];
79+
}
80+
if ($luminance <= 0.5) {
81+
$m2 = $luminance * (1.0 + $saturation);
82+
} else {
83+
$m2 = $luminance + $saturation - ($luminance * $saturation);
84+
}
85+
$m1 = 2.0 * $luminance - $m2;
86+
87+
return [
88+
self::vFunction($m1, $m2, $hue + self::ONE_THIRD),
89+
self::vFunction($m1, $m2, $hue),
90+
self::vFunction($m1, $m2, $hue - self::ONE_THIRD),
91+
];
92+
}
93+
94+
private static function vFunction(float $m1, float $m2, float $hue): float
95+
{
96+
$hue = self::positiveDecimalPart($hue);
97+
if ($hue < self::ONE_SIXTH) {
98+
return $m1 + ($m2 - $m1) * $hue * 6.0;
99+
}
100+
if ($hue < 0.5) {
101+
return $m2;
102+
}
103+
if ($hue < self::TWO_THIRD) {
104+
return $m1 + ($m2 - $m1) * (self::TWO_THIRD - $hue) * 6.0;
105+
}
106+
107+
return $m1;
108+
}
109+
110+
private static function positiveDecimalPart(float $hue): float
111+
{
112+
$hue = fmod($hue, 1.0);
113+
114+
return ($hue >= 0.0) ? $hue : (1.0 + $hue);
115+
}
116+
117+
/**
118+
* Convert red/green/blue to HLSMAX-based hue/luminance/saturation.
119+
*
120+
* @return int[]
121+
*/
122+
private static function rgbToMsHls(int $red, int $green, int $blue): array
123+
{
124+
$red01 = $red / self::RGBMAX;
125+
$green01 = $green / self::RGBMAX;
126+
$blue01 = $blue / self::RGBMAX;
127+
[$hue, $luminance, $saturation] = self::rgbToHls($red01, $green01, $blue01);
128+
129+
return [
130+
(int) round($hue * self::HLSMAX),
131+
(int) round($luminance * self::HLSMAX),
132+
(int) round($saturation * self::HLSMAX),
133+
];
134+
}
135+
136+
/**
137+
* Converts HLSMAX based HLS values to rgb values in the range (0,1).
138+
*
139+
* @return float[]
140+
*/
141+
private static function msHlsToRgb(int $hue, int $lightness, int $saturation): array
142+
{
143+
return self::hlsToRgb($hue / self::HLSMAX, $lightness / self::HLSMAX, $saturation / self::HLSMAX);
144+
}
145+
146+
/**
147+
* Tints HLSMAX based luminance.
148+
*
149+
* @see http://ciintelligence.blogspot.co.uk/2012/02/converting-excel-theme-color-and-tint.html
150+
*/
151+
private static function tintLuminance(float $tint, float $luminance): int
152+
{
153+
if ($tint < 0) {
154+
return (int) round($luminance * (1.0 + $tint));
155+
}
156+
157+
return (int) round($luminance * (1.0 - $tint) + (self::HLSMAX - self::HLSMAX * (1.0 - $tint)));
158+
}
159+
160+
/**
161+
* Return result of tinting supplied rgb as 6 hex digits.
162+
*/
163+
public static function rgbAndTintToRgb(int $red, int $green, int $blue, float $tint): string
164+
{
165+
[$hue, $luminance, $saturation] = self::rgbToMsHls($red, $green, $blue);
166+
[$red, $green, $blue] = self::msHlsToRgb($hue, self::tintLuminance($tint, $luminance), $saturation);
167+
168+
return sprintf(
169+
'%02X%02X%02X',
170+
(int) round($red * self::RGBMAX),
171+
(int) round($green * self::RGBMAX),
172+
(int) round($blue * self::RGBMAX)
173+
);
174+
}
175+
}

‎tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2387Test.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ public function testIssue2387(): void
1515
$reader = IOFactory::createReader('Xlsx');
1616
$spreadsheet = $reader->load($filename);
1717
$sheet = $spreadsheet->getActiveSheet();
18-
self::assertSame('335593', $sheet->getCell('B2')->getStyle()->getFont()->getColor()->getRgb());
18+
// Font color being tested uses theme color with tint.
19+
// Excel shows final color as 305496.
20+
$expectedColor = '305496';
21+
$calculatedColor = $sheet->getCell('B2')->getStyle()->getFont()->getColor()->getRgb();
22+
self::assertSame($expectedColor, RgbTintTest::compareColors($calculatedColor, $expectedColor));
1923
self::assertSame(Fill::FILL_NONE, $sheet->getCell('B2')->getStyle()->getFill()->getFillType());
2024
self::assertSame('FFFFFF', $sheet->getCell('C2')->getStyle()->getFont()->getColor()->getRgb());
2125
self::assertSame('000000', $sheet->getCell('C2')->getStyle()->getFill()->getStartColor()->getRgb());

‎tests/PhpSpreadsheetTests/Reader/Xlsx/RgbTintTest.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
4+
5+
use PhpOffice\PhpSpreadsheet\IOFactory;
6+
use PHPUnit\Framework\TestCase;
7+
8+
class RgbTintTest extends TestCase
9+
{
10+
public static function compareColors(string $style, string $text): string
11+
{
12+
$styleRed = hexdec(substr($style, 0, 2));
13+
$styleGreen = hexdec(substr($style, 2, 2));
14+
$styleBlue = hexdec(substr($style, 4, 2));
15+
$textRed = hexdec(substr($text, 0, 2));
16+
$textGreen = hexdec(substr($text, 2, 2));
17+
$textBlue = hexdec(substr($text, 4, 2));
18+
$maxDiff = 3;
19+
if (abs($styleRed - $textRed) > $maxDiff) {
20+
return $style;
21+
}
22+
if (abs($styleGreen - $textGreen) > $maxDiff) {
23+
return $style;
24+
}
25+
if (abs($styleBlue - $textBlue) > $maxDiff) {
26+
return $style;
27+
}
28+
29+
return $text;
30+
}
31+
32+
public function testRgbTint(): void
33+
{
34+
$filename = 'tests/data/Reader/XLSX/RgbTint.xlsx';
35+
$reader = IOFactory::createReader('Xlsx');
36+
$spreadsheet = $reader->load($filename);
37+
$sheet = $spreadsheet->getActiveSheet();
38+
$row = 0;
39+
while (true) {
40+
++$row;
41+
$text = (string) $sheet->getCell("B$row");
42+
if ($text === '') {
43+
break;
44+
}
45+
$style = $sheet->getStyle("A$row")->getFill()->getStartColor()->getRgb();
46+
self::assertSame($text, self::compareColors($style, $text), "row $row");
47+
}
48+
$spreadsheet->disconnectWorksheets();
49+
}
50+
}

‎tests/data/Reader/XLSX/RgbTint.xlsx‎

10.8 KB
Binary file not shown.

‎tests/data/Style/Color/ColorChangeBrightness.php‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
],
1010
// RGBA
1111
[
12-
'FF99A8B7',
12+
'FF92A8BE',
1313
'FFAABBCC',
1414
-0.1,
1515
],
@@ -20,17 +20,17 @@
2020
0.1,
2121
],
2222
[
23-
'99A8B7',
23+
'92A8BE',
2424
'AABBCC',
2525
-0.1,
2626
],
2727
[
28-
'FF1919',
28+
'FF1A1A',
2929
'FF0000',
3030
0.1,
3131
],
3232
[
33-
'E50000',
33+
'E60000',
3434
'FF0000',
3535
-0.1,
3636
],
@@ -40,7 +40,7 @@
4040
0.1,
4141
],
4242
[
43-
'E57373',
43+
'FF5959',
4444
'FF8080',
4545
-0.1,
4646
],
@@ -50,7 +50,7 @@
5050
0.15,
5151
],
5252
[
53-
'D80000',
53+
'D90000',
5454
'FF0000',
5555
-0.15,
5656
],
@@ -60,18 +60,23 @@
6060
0.15,
6161
],
6262
[
63-
'D86C6C',
63+
'FF4646',
6464
'FF8080',
6565
-0.15,
6666
],
6767
[
68-
'FFF783',
68+
'FFF984',
6969
'FFF008',
7070
0.5,
7171
],
7272
[
73-
'7F7804',
73+
'847D00',
7474
'FFF008',
7575
-0.5,
7676
],
77+
'issue 3550' => [
78+
'558ED5',
79+
'1F497D',
80+
0.39997558519241921,
81+
],
7782
];

0 commit comments

Comments
 (0)