Skip to content

Commit 364ec2e

Browse files
AlexandreSiAlexandreSi
authored
Fix crashes in Preview due to ill-formed color values (#6980)
- Make color parsing from string more robust (issues when setting colors in Sprite, BBText, Particle emitter and effects with colors) - Allow use of hex strings and shorthand hex strings in color fields - Remove UI glitch when switching effect type and both effects have parameters with identical names
1 parent 0d36a27 commit 364ec2e

17 files changed

+142
-81
lines changed

‎Extensions/3D/AmbientLight.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,7 @@ namespace gdjs {
7373
}
7474
updateStringParameter(parameterName: string, value: string): void {
7575
if (parameterName === 'color') {
76-
this.light.color.setHex(
77-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
78-
);
76+
this.light.color.setHex(gdjs.rgbOrHexStringToNumber(value));
7977
}
8078
}
8179
updateColorParameter(parameterName: string, value: number): void {

‎Extensions/3D/DirectionalLight.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ namespace gdjs {
9494
updateStringParameter(parameterName: string, value: string): void {
9595
if (parameterName === 'color') {
9696
this.light.color = new THREE.Color(
97-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
97+
gdjs.rgbOrHexStringToNumber(value)
9898
);
9999
}
100100
if (parameterName === 'top') {

‎Extensions/3D/ExponentialFog.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ namespace gdjs {
7171
updateStringParameter(parameterName: string, value: string): void {
7272
if (parameterName === 'color') {
7373
this.fog.color = new THREE.Color(
74-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
74+
gdjs.rgbOrHexStringToNumber(value)
7575
);
7676
}
7777
}

‎Extensions/3D/HemisphereLight.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,12 @@ namespace gdjs {
9595
updateStringParameter(parameterName: string, value: string): void {
9696
if (parameterName === 'skyColor') {
9797
this.light.color = new THREE.Color(
98-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
98+
gdjs.rgbOrHexStringToNumber(value)
9999
);
100100
}
101101
if (parameterName === 'groundColor') {
102102
this.light.groundColor = new THREE.Color(
103-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
103+
gdjs.rgbOrHexStringToNumber(value)
104104
);
105105
}
106106
if (parameterName === 'top') {

‎Extensions/3D/LinearFog.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ namespace gdjs {
7676
updateStringParameter(parameterName: string, value: string): void {
7777
if (parameterName === 'color') {
7878
this.fog.color = new THREE.Color(
79-
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
79+
gdjs.rgbOrHexStringToNumber(value)
8080
);
8181
}
8282
}

‎Extensions/Effects/bevel-pixi-filter.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,10 @@ namespace gdjs {
6767
const bevelFilter = (filter as unknown) as PIXI.filters.BevelFilter &
6868
BevelFilterExtra;
6969
if (parameterName === 'lightColor') {
70-
bevelFilter.lightColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
71-
value
72-
);
70+
bevelFilter.lightColor = gdjs.rgbOrHexStringToNumber(value);
7371
}
7472
if (parameterName === 'shadowColor') {
75-
bevelFilter.shadowColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
76-
value
77-
);
73+
bevelFilter.shadowColor = gdjs.rgbOrHexStringToNumber(value);
7874
}
7975
}
8076
updateColorParameter(

‎Extensions/Effects/color-replace-pixi-filter.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,9 @@ namespace gdjs {
4545
const colorReplaceFilter = (filter as unknown) as PIXI.filters.ColorReplaceFilter &
4646
ColorReplaceFilterExtra;
4747
if (parameterName === 'originalColor') {
48-
colorReplaceFilter.originalColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
49-
value
50-
);
48+
colorReplaceFilter.originalColor = gdjs.rgbOrHexStringToNumber(value);
5149
} else if (parameterName === 'newColor') {
52-
colorReplaceFilter.newColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
53-
value
54-
);
50+
colorReplaceFilter.newColor = gdjs.rgbOrHexStringToNumber(value);
5551
}
5652
}
5753
updateColorParameter(

‎Extensions/Effects/drop-shadow-pixi-filter.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ namespace gdjs {
6666
) {
6767
const dropShadowFilter = (filter as unknown) as PIXI.filters.DropShadowFilter;
6868
if (parameterName === 'color') {
69-
dropShadowFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
70-
value
71-
);
69+
dropShadowFilter.color = gdjs.rgbOrHexStringToNumber(value);
7270
}
7371
}
7472
updateColorParameter(

‎Extensions/Effects/glow-pixi-filter.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ namespace gdjs {
5353
const glowFilter = (filter as unknown) as PIXI.filters.GlowFilter &
5454
GlowFilterExtra;
5555
if (parameterName === 'color') {
56-
glowFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value);
56+
glowFilter.color = gdjs.rgbOrHexStringToNumber(value);
5757
}
5858
}
5959
updateColorParameter(

‎Extensions/Effects/outline-pixi-filter.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ namespace gdjs {
4141
) {
4242
const outlineFilter = (filter as unknown) as PIXI.filters.OutlineFilter;
4343
if (parameterName === 'color') {
44-
outlineFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
45-
value
46-
);
44+
outlineFilter.color = gdjs.rgbOrHexStringToNumber(value);
4745
}
4846
}
4947
updateColorParameter(

‎Extensions/ParticleSystem/particleemitterobject.ts

+8-14
Original file line numberDiff line numberDiff line change
@@ -922,23 +922,17 @@ namespace gdjs {
922922
}
923923

924924
setParticleColor1(rgbColor: string): void {
925-
const colors = rgbColor.split(';');
926-
if (colors.length < 3) {
927-
return;
928-
}
929-
this.setParticleRed1(parseInt(colors[0], 10));
930-
this.setParticleGreen1(parseInt(colors[1], 10));
931-
this.setParticleBlue1(parseInt(colors[2], 10));
925+
const colors = gdjs.rgbOrHexToRGBColor(rgbColor);
926+
this.setParticleRed1(colors[0]);
927+
this.setParticleGreen1(colors[1]);
928+
this.setParticleBlue1(colors[2]);
932929
}
933930

934931
setParticleColor2(rgbColor: string): void {
935-
const colors = rgbColor.split(';');
936-
if (colors.length < 3) {
937-
return;
938-
}
939-
this.setParticleRed2(parseInt(colors[0], 10));
940-
this.setParticleGreen2(parseInt(colors[1], 10));
941-
this.setParticleBlue2(parseInt(colors[2], 10));
932+
const colors = gdjs.rgbOrHexToRGBColor(rgbColor);
933+
this.setParticleRed2(colors[0]);
934+
this.setParticleGreen2(colors[1]);
935+
this.setParticleBlue2(colors[2]);
942936
}
943937

944938
getParticleSize1(): float {

‎GDJS/Runtime/gd.ts

+60-11
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
*/
1111
namespace gdjs {
1212
const logger = new gdjs.Logger('Engine runtime');
13+
const hexStringRegex = /^(#{0,1}[A-Fa-f0-9]{6})$/;
14+
const shorthandHexStringRegex = /^(#{0,1}[A-Fa-f0-9]{3})$/;
15+
const rgbStringRegex = /^(\d{1,3};\d{1,3};\d{1,3})/;
1316

1417
/**
1518
* Contains functions used by events (this is a convention only, functions can actually
@@ -61,7 +64,7 @@ namespace gdjs {
6164
};
6265

6366
/**
64-
* Convert a Hex string to an RGB color array [r, g, b], where each component is in the range [0, 255].
67+
* Convert a Hex string (#124FE4) to an RGB color array [r, g, b], where each component is in the range [0, 255].
6568
*
6669
* @param {string} hex Color hexadecimal
6770
*/
@@ -74,24 +77,53 @@ namespace gdjs {
7477
: [0, 0, 0];
7578
};
7679

80+
/**
81+
* Convert a shorthand Hex string (#1F4) to an RGB color array [r, g, b], where each component is in the range [0, 255].
82+
*
83+
* @param {string} hex Color hexadecimal
84+
*/
85+
export const shorthandHexToRGBColor = function (
86+
hexString: string
87+
): [number, number, number] {
88+
const hexNumber = parseInt(hexString.replace('#', ''), 16);
89+
return Number.isFinite(hexNumber)
90+
? [
91+
17 * ((hexNumber >> 8) & 0xf),
92+
17 * ((hexNumber >> 4) & 0xf),
93+
17 * (hexNumber & 0xf),
94+
]
95+
: [0, 0, 0];
96+
};
97+
7798
/**
7899
* Convert a RGB string ("rrr;ggg;bbb") or a Hex string ("#rrggbb") to a RGB color array ([r,g,b] with each component going from 0 to 255).
79100
* @param value The color as a RGB string or Hex string
80101
*/
81102
export const rgbOrHexToRGBColor = function (
82103
value: string
83104
): [number, number, number] {
84-
const splitValue = value.split(';');
85-
// If a RGB string is provided, return the RGB object.
86-
if (splitValue.length === 3) {
87-
return [
88-
parseInt(splitValue[0], 10),
89-
parseInt(splitValue[1], 10),
90-
parseInt(splitValue[2], 10),
91-
];
105+
const rgbColor = extractRGBString(value);
106+
if (rgbColor) {
107+
const splitValue = rgbColor.split(';');
108+
// If a RGB string is provided, return the RGB object.
109+
if (splitValue.length === 3) {
110+
return [
111+
Math.min(255, parseInt(splitValue[0], 10)),
112+
Math.min(255, parseInt(splitValue[1], 10)),
113+
Math.min(255, parseInt(splitValue[2], 10)),
114+
];
115+
}
116+
}
117+
118+
const hexColor = extractHexString(value);
119+
if (hexColor) {
120+
return hexToRGBColor(hexColor);
121+
}
122+
const shorthandHexColor = extractShorthandHexString(value);
123+
if (shorthandHexColor) {
124+
return shorthandHexToRGBColor(shorthandHexColor);
92125
}
93-
// Otherwise, convert the Hex to RGB.
94-
return hexToRGBColor(value);
126+
return [0, 0, 0];
95127
};
96128

97129
/**
@@ -146,6 +178,23 @@ namespace gdjs {
146178
];
147179
};
148180

181+
export const extractHexString = (str: string): string | null => {
182+
const matches = str.match(hexStringRegex);
183+
if (!matches) return null;
184+
return matches[0];
185+
};
186+
export const extractShorthandHexString = (str: string): string | null => {
187+
const matches = str.match(shorthandHexStringRegex);
188+
if (!matches) return null;
189+
return matches[0];
190+
};
191+
192+
export const extractRGBString = (str: string): string | null => {
193+
const matches = str.match(rgbStringRegex);
194+
if (!matches) return null;
195+
return matches[0];
196+
};
197+
149198
/**
150199
* Get a random integer between 0 and max.
151200
* @param max The maximum value (inclusive).

‎GDJS/Runtime/pixi-renderers/pixi-filters-tools.ts

-16
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,6 @@ namespace gdjs {
5252
_filterCreators[filterName] = filterCreator;
5353
};
5454

55-
/**
56-
* Convert a string RGB color ("rrr;ggg;bbb") or a hex string (#rrggbb) to a hex number.
57-
* @param value The color as a RGB string or hex string
58-
*/
59-
export const rgbOrHexToHexNumber = function (value: string): number {
60-
const splitValue = value.split(';');
61-
if (splitValue.length === 3) {
62-
return gdjs.rgbToHexNumber(
63-
parseInt(splitValue[0], 10),
64-
parseInt(splitValue[1], 10),
65-
parseInt(splitValue[2], 10)
66-
);
67-
}
68-
return parseInt(value.replace('#', '0x'), 16);
69-
};
70-
7155
/** A wrapper allowing to create an effect. */
7256
export interface FilterCreator {
7357
/** Function to call to create the filter */

‎GDJS/Runtime/pixi-renderers/spriteruntimeobject-pixi-renderer.ts

+2-10
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,8 @@ namespace gdjs {
142142
this._sprite.visible = !this._object.hidden;
143143
}
144144

145-
setColor(rgbColor): void {
146-
const colors = rgbColor.split(';');
147-
if (colors.length < 3) {
148-
return;
149-
}
150-
this._sprite.tint = gdjs.rgbToHexNumber(
151-
parseInt(colors[0], 10),
152-
parseInt(colors[1], 10),
153-
parseInt(colors[2], 10)
154-
);
145+
setColor(rgbOrHexColor): void {
146+
this._sprite.tint = gdjs.rgbOrHexStringToNumber(rgbOrHexColor);
155147
}
156148

157149
getColor() {

‎GDJS/Runtime/spriteruntimeobject.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -759,10 +759,10 @@ namespace gdjs {
759759
/**
760760
* Change the tint of the sprite object.
761761
*
762-
* @param rgbColor The color, in RGB format ("128;200;255").
762+
* @param rgbOrHexColor The color as a string, in RGB format ("128;200;255") or Hex format.
763763
*/
764-
setColor(rgbColor: string): void {
765-
this._renderer.setColor(rgbColor);
764+
setColor(rgbOrHexColor: string): void {
765+
this._renderer.setColor(rgbOrHexColor);
766766
}
767767

768768
/**

‎GDJS/tests/tests/colors.js

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
describe('gdjs', function () {
2+
it('should define gdjs', function () {
3+
expect(gdjs).to.be.ok();
4+
});
5+
6+
describe('Color conversion', function () {
7+
describe('Hex strings to RGB components', () => {
8+
it('should convert hex strings', function () {
9+
expect(gdjs.hexToRGBColor('#FFFFfF')).to.eql([255, 255, 255]);
10+
expect(gdjs.hexToRGBColor('#000000')).to.eql([0, 0, 0]);
11+
expect(gdjs.hexToRGBColor('#1245F5')).to.eql([18, 69, 245]);
12+
});
13+
it('should convert hex strings without hashtag', function () {
14+
expect(gdjs.hexToRGBColor('FFFFfF')).to.eql([255, 255, 255]);
15+
expect(gdjs.hexToRGBColor('000000')).to.eql([0, 0, 0]);
16+
expect(gdjs.hexToRGBColor('1245F5')).to.eql([18, 69, 245]);
17+
});
18+
it('should convert shorthand hex strings', function () {
19+
expect(gdjs.shorthandHexToRGBColor('#FfF')).to.eql([255, 255, 255]);
20+
expect(gdjs.shorthandHexToRGBColor('#000')).to.eql([0, 0, 0]);
21+
expect(gdjs.shorthandHexToRGBColor('#F3a')).to.eql([255, 51, 170]);
22+
});
23+
it('should convert shorthand hex strings without hashtag', function () {
24+
expect(gdjs.shorthandHexToRGBColor('FFF')).to.eql([255, 255, 255]);
25+
expect(gdjs.shorthandHexToRGBColor('000')).to.eql([0, 0, 0]);
26+
expect(gdjs.shorthandHexToRGBColor('F3a')).to.eql([255, 51, 170]);
27+
});
28+
});
29+
describe.only('RGB strings to RGB components', () => {
30+
it('should convert rgb strings', function () {
31+
expect(gdjs.rgbOrHexToRGBColor('0;0;0')).to.eql([0, 0, 0]);
32+
expect(gdjs.rgbOrHexToRGBColor('255;255;255')).to.eql([255, 255, 255]);
33+
expect(gdjs.rgbOrHexToRGBColor('120;12;6')).to.eql([120, 12, 6]);
34+
});
35+
it('should max rgb values', function () {
36+
expect(gdjs.rgbOrHexToRGBColor('255;255;300')).to.eql([255, 255, 255]);
37+
expect(gdjs.rgbOrHexToRGBColor('999;12;6')).to.eql([255, 12, 6]);
38+
});
39+
it('should cut rgb values if string too long', function () {
40+
expect(gdjs.rgbOrHexToRGBColor('255;255;200456')).to.eql([
41+
255,
42+
255,
43+
200,
44+
]);
45+
});
46+
it('should return components for black if unrecognized input', function () {
47+
expect(gdjs.rgbOrHexToRGBColor('NaN')).to.eql([0, 0, 0]);
48+
expect(gdjs.rgbOrHexToRGBColor('19819830803')).to.eql([0, 0, 0]);
49+
expect(gdjs.rgbOrHexToRGBColor('Infinity')).to.eql([0, 0, 0]);
50+
expect(gdjs.rgbOrHexToRGBColor('-4564')).to.eql([0, 0, 0]);
51+
expect(gdjs.rgbOrHexToRGBColor('9999;12;6')).to.eql([0, 0, 0]);
52+
});
53+
});
54+
});
55+
});

‎newIDE/app/src/EffectsList/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ const Effect = React.forwardRef(
296296
</BackgroundText>
297297
</Line>
298298
<PropertiesEditor
299+
key={effect.getEffectType()}
299300
instances={[effect]}
300301
schema={effectMetadata.parametersSchema}
301302
project={project}

0 commit comments

Comments
 (0)