Skip to content

Commit

Permalink
Fix crashes in Preview due to ill-formed color values (#6980)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
AlexandreSi authored Sep 23, 2024
1 parent 0d36a27 commit 364ec2e
Show file tree
Hide file tree
Showing 17 changed files with 142 additions and 81 deletions.
4 changes: 1 addition & 3 deletions Extensions/3D/AmbientLight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ namespace gdjs {
}
updateStringParameter(parameterName: string, value: string): void {
if (parameterName === 'color') {
this.light.color.setHex(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
);
this.light.color.setHex(gdjs.rgbOrHexStringToNumber(value));
}
}
updateColorParameter(parameterName: string, value: number): void {
Expand Down
2 changes: 1 addition & 1 deletion Extensions/3D/DirectionalLight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ namespace gdjs {
updateStringParameter(parameterName: string, value: string): void {
if (parameterName === 'color') {
this.light.color = new THREE.Color(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
gdjs.rgbOrHexStringToNumber(value)
);
}
if (parameterName === 'top') {
Expand Down
2 changes: 1 addition & 1 deletion Extensions/3D/ExponentialFog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace gdjs {
updateStringParameter(parameterName: string, value: string): void {
if (parameterName === 'color') {
this.fog.color = new THREE.Color(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
gdjs.rgbOrHexStringToNumber(value)
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions Extensions/3D/HemisphereLight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ namespace gdjs {
updateStringParameter(parameterName: string, value: string): void {
if (parameterName === 'skyColor') {
this.light.color = new THREE.Color(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
gdjs.rgbOrHexStringToNumber(value)
);
}
if (parameterName === 'groundColor') {
this.light.groundColor = new THREE.Color(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
gdjs.rgbOrHexStringToNumber(value)
);
}
if (parameterName === 'top') {
Expand Down
2 changes: 1 addition & 1 deletion Extensions/3D/LinearFog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ namespace gdjs {
updateStringParameter(parameterName: string, value: string): void {
if (parameterName === 'color') {
this.fog.color = new THREE.Color(
gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value)
gdjs.rgbOrHexStringToNumber(value)
);
}
}
Expand Down
8 changes: 2 additions & 6 deletions Extensions/Effects/bevel-pixi-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,10 @@ namespace gdjs {
const bevelFilter = (filter as unknown) as PIXI.filters.BevelFilter &
BevelFilterExtra;
if (parameterName === 'lightColor') {
bevelFilter.lightColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
bevelFilter.lightColor = gdjs.rgbOrHexStringToNumber(value);
}
if (parameterName === 'shadowColor') {
bevelFilter.shadowColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
bevelFilter.shadowColor = gdjs.rgbOrHexStringToNumber(value);
}
}
updateColorParameter(
Expand Down
8 changes: 2 additions & 6 deletions Extensions/Effects/color-replace-pixi-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,9 @@ namespace gdjs {
const colorReplaceFilter = (filter as unknown) as PIXI.filters.ColorReplaceFilter &
ColorReplaceFilterExtra;
if (parameterName === 'originalColor') {
colorReplaceFilter.originalColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
colorReplaceFilter.originalColor = gdjs.rgbOrHexStringToNumber(value);
} else if (parameterName === 'newColor') {
colorReplaceFilter.newColor = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
colorReplaceFilter.newColor = gdjs.rgbOrHexStringToNumber(value);
}
}
updateColorParameter(
Expand Down
4 changes: 1 addition & 3 deletions Extensions/Effects/drop-shadow-pixi-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ namespace gdjs {
) {
const dropShadowFilter = (filter as unknown) as PIXI.filters.DropShadowFilter;
if (parameterName === 'color') {
dropShadowFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
dropShadowFilter.color = gdjs.rgbOrHexStringToNumber(value);
}
}
updateColorParameter(
Expand Down
2 changes: 1 addition & 1 deletion Extensions/Effects/glow-pixi-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace gdjs {
const glowFilter = (filter as unknown) as PIXI.filters.GlowFilter &
GlowFilterExtra;
if (parameterName === 'color') {
glowFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(value);
glowFilter.color = gdjs.rgbOrHexStringToNumber(value);
}
}
updateColorParameter(
Expand Down
4 changes: 1 addition & 3 deletions Extensions/Effects/outline-pixi-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ namespace gdjs {
) {
const outlineFilter = (filter as unknown) as PIXI.filters.OutlineFilter;
if (parameterName === 'color') {
outlineFilter.color = gdjs.PixiFiltersTools.rgbOrHexToHexNumber(
value
);
outlineFilter.color = gdjs.rgbOrHexStringToNumber(value);
}
}
updateColorParameter(
Expand Down
22 changes: 8 additions & 14 deletions Extensions/ParticleSystem/particleemitterobject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -922,23 +922,17 @@ namespace gdjs {
}

setParticleColor1(rgbColor: string): void {
const colors = rgbColor.split(';');
if (colors.length < 3) {
return;
}
this.setParticleRed1(parseInt(colors[0], 10));
this.setParticleGreen1(parseInt(colors[1], 10));
this.setParticleBlue1(parseInt(colors[2], 10));
const colors = gdjs.rgbOrHexToRGBColor(rgbColor);
this.setParticleRed1(colors[0]);
this.setParticleGreen1(colors[1]);
this.setParticleBlue1(colors[2]);
}

setParticleColor2(rgbColor: string): void {
const colors = rgbColor.split(';');
if (colors.length < 3) {
return;
}
this.setParticleRed2(parseInt(colors[0], 10));
this.setParticleGreen2(parseInt(colors[1], 10));
this.setParticleBlue2(parseInt(colors[2], 10));
const colors = gdjs.rgbOrHexToRGBColor(rgbColor);
this.setParticleRed2(colors[0]);
this.setParticleGreen2(colors[1]);
this.setParticleBlue2(colors[2]);
}

getParticleSize1(): float {
Expand Down
71 changes: 60 additions & 11 deletions GDJS/Runtime/gd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
*/
namespace gdjs {
const logger = new gdjs.Logger('Engine runtime');
const hexStringRegex = /^(#{0,1}[A-Fa-f0-9]{6})$/;
const shorthandHexStringRegex = /^(#{0,1}[A-Fa-f0-9]{3})$/;
const rgbStringRegex = /^(\d{1,3};\d{1,3};\d{1,3})/;

/**
* Contains functions used by events (this is a convention only, functions can actually
Expand Down Expand Up @@ -61,7 +64,7 @@ namespace gdjs {
};

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

/**
* Convert a shorthand Hex string (#1F4) to an RGB color array [r, g, b], where each component is in the range [0, 255].
*
* @param {string} hex Color hexadecimal
*/
export const shorthandHexToRGBColor = function (
hexString: string
): [number, number, number] {
const hexNumber = parseInt(hexString.replace('#', ''), 16);
return Number.isFinite(hexNumber)
? [
17 * ((hexNumber >> 8) & 0xf),
17 * ((hexNumber >> 4) & 0xf),
17 * (hexNumber & 0xf),
]
: [0, 0, 0];
};

/**
* 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).
* @param value The color as a RGB string or Hex string
*/
export const rgbOrHexToRGBColor = function (
value: string
): [number, number, number] {
const splitValue = value.split(';');
// If a RGB string is provided, return the RGB object.
if (splitValue.length === 3) {
return [
parseInt(splitValue[0], 10),
parseInt(splitValue[1], 10),
parseInt(splitValue[2], 10),
];
const rgbColor = extractRGBString(value);
if (rgbColor) {
const splitValue = rgbColor.split(';');
// If a RGB string is provided, return the RGB object.
if (splitValue.length === 3) {
return [
Math.min(255, parseInt(splitValue[0], 10)),
Math.min(255, parseInt(splitValue[1], 10)),
Math.min(255, parseInt(splitValue[2], 10)),
];
}
}

const hexColor = extractHexString(value);
if (hexColor) {
return hexToRGBColor(hexColor);
}
const shorthandHexColor = extractShorthandHexString(value);
if (shorthandHexColor) {
return shorthandHexToRGBColor(shorthandHexColor);
}
// Otherwise, convert the Hex to RGB.
return hexToRGBColor(value);
return [0, 0, 0];
};

/**
Expand Down Expand Up @@ -146,6 +178,23 @@ namespace gdjs {
];
};

export const extractHexString = (str: string): string | null => {
const matches = str.match(hexStringRegex);
if (!matches) return null;
return matches[0];
};
export const extractShorthandHexString = (str: string): string | null => {
const matches = str.match(shorthandHexStringRegex);
if (!matches) return null;
return matches[0];
};

export const extractRGBString = (str: string): string | null => {
const matches = str.match(rgbStringRegex);
if (!matches) return null;
return matches[0];
};

/**
* Get a random integer between 0 and max.
* @param max The maximum value (inclusive).
Expand Down
16 changes: 0 additions & 16 deletions GDJS/Runtime/pixi-renderers/pixi-filters-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,6 @@ namespace gdjs {
_filterCreators[filterName] = filterCreator;
};

/**
* Convert a string RGB color ("rrr;ggg;bbb") or a hex string (#rrggbb) to a hex number.
* @param value The color as a RGB string or hex string
*/
export const rgbOrHexToHexNumber = function (value: string): number {
const splitValue = value.split(';');
if (splitValue.length === 3) {
return gdjs.rgbToHexNumber(
parseInt(splitValue[0], 10),
parseInt(splitValue[1], 10),
parseInt(splitValue[2], 10)
);
}
return parseInt(value.replace('#', '0x'), 16);
};

/** A wrapper allowing to create an effect. */
export interface FilterCreator {
/** Function to call to create the filter */
Expand Down
12 changes: 2 additions & 10 deletions GDJS/Runtime/pixi-renderers/spriteruntimeobject-pixi-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,8 @@ namespace gdjs {
this._sprite.visible = !this._object.hidden;
}

setColor(rgbColor): void {
const colors = rgbColor.split(';');
if (colors.length < 3) {
return;
}
this._sprite.tint = gdjs.rgbToHexNumber(
parseInt(colors[0], 10),
parseInt(colors[1], 10),
parseInt(colors[2], 10)
);
setColor(rgbOrHexColor): void {
this._sprite.tint = gdjs.rgbOrHexStringToNumber(rgbOrHexColor);
}

getColor() {
Expand Down
6 changes: 3 additions & 3 deletions GDJS/Runtime/spriteruntimeobject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -759,10 +759,10 @@ namespace gdjs {
/**
* Change the tint of the sprite object.
*
* @param rgbColor The color, in RGB format ("128;200;255").
* @param rgbOrHexColor The color as a string, in RGB format ("128;200;255") or Hex format.
*/
setColor(rgbColor: string): void {
this._renderer.setColor(rgbColor);
setColor(rgbOrHexColor: string): void {
this._renderer.setColor(rgbOrHexColor);
}

/**
Expand Down
55 changes: 55 additions & 0 deletions GDJS/tests/tests/colors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
describe('gdjs', function () {
it('should define gdjs', function () {
expect(gdjs).to.be.ok();
});

describe('Color conversion', function () {
describe('Hex strings to RGB components', () => {
it('should convert hex strings', function () {
expect(gdjs.hexToRGBColor('#FFFFfF')).to.eql([255, 255, 255]);
expect(gdjs.hexToRGBColor('#000000')).to.eql([0, 0, 0]);
expect(gdjs.hexToRGBColor('#1245F5')).to.eql([18, 69, 245]);
});
it('should convert hex strings without hashtag', function () {
expect(gdjs.hexToRGBColor('FFFFfF')).to.eql([255, 255, 255]);
expect(gdjs.hexToRGBColor('000000')).to.eql([0, 0, 0]);
expect(gdjs.hexToRGBColor('1245F5')).to.eql([18, 69, 245]);
});
it('should convert shorthand hex strings', function () {
expect(gdjs.shorthandHexToRGBColor('#FfF')).to.eql([255, 255, 255]);
expect(gdjs.shorthandHexToRGBColor('#000')).to.eql([0, 0, 0]);
expect(gdjs.shorthandHexToRGBColor('#F3a')).to.eql([255, 51, 170]);
});
it('should convert shorthand hex strings without hashtag', function () {
expect(gdjs.shorthandHexToRGBColor('FFF')).to.eql([255, 255, 255]);
expect(gdjs.shorthandHexToRGBColor('000')).to.eql([0, 0, 0]);
expect(gdjs.shorthandHexToRGBColor('F3a')).to.eql([255, 51, 170]);
});
});
describe.only('RGB strings to RGB components', () => {
it('should convert rgb strings', function () {
expect(gdjs.rgbOrHexToRGBColor('0;0;0')).to.eql([0, 0, 0]);
expect(gdjs.rgbOrHexToRGBColor('255;255;255')).to.eql([255, 255, 255]);
expect(gdjs.rgbOrHexToRGBColor('120;12;6')).to.eql([120, 12, 6]);
});
it('should max rgb values', function () {
expect(gdjs.rgbOrHexToRGBColor('255;255;300')).to.eql([255, 255, 255]);
expect(gdjs.rgbOrHexToRGBColor('999;12;6')).to.eql([255, 12, 6]);
});
it('should cut rgb values if string too long', function () {
expect(gdjs.rgbOrHexToRGBColor('255;255;200456')).to.eql([
255,
255,
200,
]);
});
it('should return components for black if unrecognized input', function () {
expect(gdjs.rgbOrHexToRGBColor('NaN')).to.eql([0, 0, 0]);
expect(gdjs.rgbOrHexToRGBColor('19819830803')).to.eql([0, 0, 0]);
expect(gdjs.rgbOrHexToRGBColor('Infinity')).to.eql([0, 0, 0]);
expect(gdjs.rgbOrHexToRGBColor('-4564')).to.eql([0, 0, 0]);
expect(gdjs.rgbOrHexToRGBColor('9999;12;6')).to.eql([0, 0, 0]);
});
});
});
});
1 change: 1 addition & 0 deletions newIDE/app/src/EffectsList/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ const Effect = React.forwardRef(
</BackgroundText>
</Line>
<PropertiesEditor
key={effect.getEffectType()}
instances={[effect]}
schema={effectMetadata.parametersSchema}
project={project}
Expand Down

0 comments on commit 364ec2e

Please sign in to comment.