From 0b283b16cf1670118941d42a8864fbb78fa78741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:06:50 +0200 Subject: [PATCH] Fix resizing an instance with aspectRatio activated to 0, causing it to break (#9163) --- .../CompactInstancePropertiesSchema.js | 201 ++++++++++-------- .../CompactInstancePropertiesSchema.spec.js | 199 +++++++++++++++++ 2 files changed, 315 insertions(+), 85 deletions(-) create mode 100644 newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.spec.js diff --git a/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.js b/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.js index d677cdaf32..3844649a37 100644 --- a/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.js +++ b/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.js @@ -50,7 +50,67 @@ const applyRatio = ({ newReferenceValue: number, valueToApplyTo: number, |}) => { - return (newReferenceValue / oldReferenceValue) * valueToApplyTo; + // Rounding to 12 significant digits removes floating point errors + // (e.g. 104.01999999999998 instead of 104.02) that would otherwise + // accumulate when the size is changed step by step. + return Number( + ((newReferenceValue * valueToApplyTo) / oldReferenceValue).toPrecision(12) + ); +}; + +type Size = [number, number, number]; + +// Size of instances before it was set to 0 while keeping the ratio, to restore +// their proportions when the size is increased again. +const sizesBeforeCollapse: Map = new Map(); + +const setInstanceSize = ({ + instance, + dimension, + newValue, + getInstanceSize, + getInstanceDefaultSize, +}: {| + instance: gdInitialInstance, + dimension: 0 | 1 | 2, + newValue: number, + getInstanceSize: gdInitialInstance => Size, + getInstanceDefaultSize: gdInitialInstance => Size, +|}) => { + const shouldKeepRatio = instance.shouldKeepRatio(); + const currentSize = getInstanceSize(instance); + const newDimensionValue = Math.max(newValue, 0); + const isCollapsed = currentSize.every(value => value === 0); + const referenceSize = + shouldKeepRatio && isCollapsed + ? sizesBeforeCollapse.get(instance.ptr) || + getInstanceDefaultSize(instance) + : currentSize; + const referenceValue = referenceSize[dimension]; + const newSize = referenceSize.map((value, index) => + index === dimension + ? newDimensionValue + : !shouldKeepRatio || referenceValue === 0 + ? currentSize[index] + : applyRatio({ + oldReferenceValue: referenceValue, + newReferenceValue: newDimensionValue, + valueToApplyTo: value, + }) + ); + + if (newSize.every(value => value === 0)) { + if (!isCollapsed) sizesBeforeCollapse.set(instance.ptr, currentSize); + } else { + sizesBeforeCollapse.delete(instance.ptr); + } + + instance.setCustomWidth(newSize[0]); + instance.setCustomHeight(newSize[1]); + instance.setCustomDepth(newSize[2]); + // This must be done after reading the size. + instance.setHasCustomSize(true); + instance.setHasCustomDepth(true); }; const getEditObjectButton = ({ @@ -253,12 +313,14 @@ const getWidthField = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }: {| i18n: I18nType, getInstanceWidth: gdInitialInstance => number, getInstanceHeight: gdInitialInstance => number, getInstanceDepth: gdInitialInstance => number, + getInstanceDefaultSize: gdInitialInstance => Size, forceUpdate: () => void, |}): Field => ({ name: 'Width', @@ -266,34 +328,17 @@ const getWidthField = ({ valueType: 'number', getValue: getInstanceWidth, setValue: (instance: gdInitialInstance, newValue: number) => { - const shouldKeepRatio = instance.shouldKeepRatio(); - const newWidth = Math.max(newValue, 0); - if (shouldKeepRatio) { - const initialWidth = getInstanceWidth(instance) || 1; - instance.setCustomWidth(newWidth); - instance.setCustomHeight( - applyRatio({ - oldReferenceValue: initialWidth, - newReferenceValue: newWidth, - valueToApplyTo: getInstanceHeight(instance), - }) - ); - instance.setCustomDepth( - applyRatio({ - oldReferenceValue: initialWidth, - newReferenceValue: newWidth, - valueToApplyTo: getInstanceDepth(instance), - }) - ); - } else { - instance.setCustomWidth(newWidth); - instance.setCustomHeight(getInstanceHeight(instance)); - instance.setCustomDepth(getInstanceDepth(instance)); - } - - // This must be done after reading the size. - instance.setHasCustomSize(true); - instance.setHasCustomDepth(true); + setInstanceSize({ + instance, + dimension: 0, + newValue, + getInstanceSize: instance => [ + getInstanceWidth(instance), + getInstanceHeight(instance), + getInstanceDepth(instance), + ], + getInstanceDefaultSize, + }); forceUpdate(); }, renderLeftIcon: className => , @@ -314,12 +359,14 @@ const getHeightField = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }: {| i18n: I18nType, getInstanceWidth: gdInitialInstance => number, getInstanceHeight: gdInitialInstance => number, getInstanceDepth: gdInitialInstance => number, + getInstanceDefaultSize: gdInitialInstance => Size, forceUpdate: () => void, |}): Field => ({ name: 'Height', @@ -327,34 +374,17 @@ const getHeightField = ({ valueType: 'number', getValue: getInstanceHeight, setValue: (instance: gdInitialInstance, newValue: number) => { - const shouldKeepRatio = instance.shouldKeepRatio(); - const newHeight = Math.max(newValue, 0); - if (shouldKeepRatio) { - const initialHeight = getInstanceHeight(instance) || 1; - instance.setCustomWidth( - applyRatio({ - oldReferenceValue: initialHeight, - newReferenceValue: newHeight, - valueToApplyTo: getInstanceWidth(instance), - }) - ); - instance.setCustomHeight(newHeight); - instance.setCustomDepth( - applyRatio({ - oldReferenceValue: initialHeight, - newReferenceValue: newHeight, - valueToApplyTo: getInstanceDepth(instance), - }) - ); - } else { - instance.setCustomWidth(getInstanceWidth(instance)); - instance.setCustomHeight(newHeight); - instance.setCustomDepth(getInstanceDepth(instance)); - } - - // This must be done after reading the size. - instance.setHasCustomSize(true); - instance.setHasCustomDepth(true); + setInstanceSize({ + instance, + dimension: 1, + newValue, + getInstanceSize: instance => [ + getInstanceWidth(instance), + getInstanceHeight(instance), + getInstanceDepth(instance), + ], + getInstanceDefaultSize, + }); forceUpdate(); }, renderLeftIcon: className => , @@ -375,12 +405,14 @@ const getDepthField = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }: {| i18n: I18nType, getInstanceWidth: gdInitialInstance => number, getInstanceHeight: gdInitialInstance => number, getInstanceDepth: gdInitialInstance => number, + getInstanceDefaultSize: gdInitialInstance => Size, forceUpdate: () => void, |}): Field => ({ name: 'Depth', @@ -388,34 +420,17 @@ const getDepthField = ({ valueType: 'number', getValue: getInstanceDepth, setValue: (instance: gdInitialInstance, newValue: number) => { - const shouldKeepRatio = instance.shouldKeepRatio(); - const newDepth = Math.max(newValue, 0); - if (shouldKeepRatio) { - const initialDepth = getInstanceDepth(instance) || 1; - instance.setCustomWidth( - applyRatio({ - oldReferenceValue: initialDepth, - newReferenceValue: newDepth, - valueToApplyTo: getInstanceWidth(instance), - }) - ); - instance.setCustomHeight( - applyRatio({ - oldReferenceValue: initialDepth, - newReferenceValue: newDepth, - valueToApplyTo: getInstanceHeight(instance), - }) - ); - instance.setCustomDepth(newDepth); - } else { - instance.setCustomWidth(getInstanceWidth(instance)); - instance.setCustomHeight(getInstanceHeight(instance)); - instance.setCustomDepth(newDepth); - } - - // This must be done after reading the size. - instance.setHasCustomSize(true); - instance.setHasCustomDepth(true); + setInstanceSize({ + instance, + dimension: 2, + newValue, + getInstanceSize: instance => [ + getInstanceWidth(instance), + getInstanceHeight(instance), + getInstanceDepth(instance), + ], + getInstanceDefaultSize, + }); forceUpdate(); }, renderLeftIcon: className => , @@ -556,6 +571,17 @@ export const makeSchema = ({ ? instance.getCustomDepth() : onGetInstanceSize(instance)[2]; + const getInstanceDefaultSize = (instance: gdInitialInstance): Size => { + const hasCustomSize = instance.hasCustomSize(); + const hasCustomDepth = instance.hasCustomDepth(); + instance.setHasCustomSize(false); + instance.setHasCustomDepth(false); + const defaultSize = onGetInstanceSize(instance); + instance.setHasCustomSize(hasCustomSize); + instance.setHasCustomDepth(hasCustomDepth); + return defaultSize; + }; + if (is3DInstance) { // $FlowFixMe[incompatible-type] return [ @@ -582,6 +608,7 @@ export const makeSchema = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }), getHeightField({ @@ -589,6 +616,7 @@ export const makeSchema = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }), getDepthField({ @@ -596,6 +624,7 @@ export const makeSchema = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }), ], @@ -672,6 +701,7 @@ export const makeSchema = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }), getHeightField({ @@ -679,6 +709,7 @@ export const makeSchema = ({ getInstanceWidth, getInstanceHeight, getInstanceDepth, + getInstanceDefaultSize, forceUpdate, }), ], diff --git a/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.spec.js b/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.spec.js new file mode 100644 index 0000000000..4476f6ae7f --- /dev/null +++ b/newIDE/app/src/InstancesEditor/CompactInstancePropertiesEditor/CompactInstancePropertiesSchema.spec.js @@ -0,0 +1,199 @@ +// @flow +import { makeSchema } from './CompactInstancePropertiesSchema'; +const gd: libGDevelop = global.gd; + +// Avoid importing the whole properties panel (and its renderers). +jest.mock('./index', () => ({ styles: { icon: {} } })); + +const fakeI18n: any = { _: () => '' }; + +const findField = (schema: any, name: string): any => { + for (const field of schema) { + if (!field) continue; + if (field.name === name) return field; + const children = field.children || (field.child ? [field.child] : null); + if (children) { + const found = findField(children, name); + if (found) return found; + } + } + return null; +}; + +const makeSizeFields = ({ + is3DInstance, + defaultSize, +}: {| + is3DInstance: boolean, + defaultSize: [number, number, number], +|}) => { + const layout = new gd.Layout(); + const schema = makeSchema({ + is3DInstance, + hasOpacity: true, + canBeFlippedXY: true, + canBeFlippedZ: true, + i18n: fakeI18n, + forceUpdate: () => {}, + onEditObject: () => {}, + onGetInstanceSize: (instance: gdInitialInstance) => [ + instance.hasCustomSize() ? instance.getCustomWidth() : defaultSize[0], + instance.hasCustomSize() ? instance.getCustomHeight() : defaultSize[1], + instance.hasCustomDepth() ? instance.getCustomDepth() : defaultSize[2], + ], + layersContainer: layout.getLayers(), + }); + return { + widthField: findField(schema, 'Width'), + heightField: findField(schema, 'Height'), + depthField: findField(schema, 'Depth'), + layout, + }; +}; + +const getSize = (instance: gdInitialInstance) => [ + instance.getCustomWidth(), + instance.getCustomHeight(), + instance.getCustomDepth(), +]; + +describe('CompactInstancePropertiesSchema size fields', () => { + it('keeps the ratio when a 3D instance is resized to 0 and back', () => { + const { widthField, layout } = makeSizeFields({ + is3DInstance: true, + defaultSize: [40, 20, 10], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(true); + + widthField.setValue(instance, 80); + expect(getSize(instance)).toEqual([80, 40, 20]); + + widthField.setValue(instance, 0); + expect(getSize(instance)).toEqual([0, 0, 0]); + + widthField.setValue(instance, 8); + expect(getSize(instance)).toEqual([8, 4, 2]); + + instance.delete(); + layout.delete(); + }); + + it('keeps the ratio when a 2D instance is resized to 0 and back with another field', () => { + const { widthField, heightField, layout } = makeSizeFields({ + is3DInstance: false, + defaultSize: [40, 20, 0], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(true); + + heightField.setValue(instance, 0); + expect(getSize(instance)).toEqual([0, 0, 0]); + + widthField.setValue(instance, 10); + expect(getSize(instance)).toEqual([10, 5, 0]); + + instance.delete(); + layout.delete(); + }); + + it('uses the object default size ratio when the collapsed size was not remembered', () => { + const { depthField, layout } = makeSizeFields({ + is3DInstance: true, + defaultSize: [40, 20, 10], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(true); + // As if the project was saved with a size of 0. + instance.setHasCustomSize(true); + instance.setHasCustomDepth(true); + instance.setCustomWidth(0); + instance.setCustomHeight(0); + instance.setCustomDepth(0); + + depthField.setValue(instance, 5); + expect(getSize(instance)).toEqual([20, 10, 5]); + + instance.delete(); + layout.delete(); + }); + + it('keeps integer sizes exact when scaling a square step by step', () => { + const { widthField, layout } = makeSizeFields({ + is3DInstance: false, + defaultSize: [64, 64, 0], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(true); + + for (let width = 65; width <= 200; width++) { + widthField.setValue(instance, width); + expect(getSize(instance)).toEqual([width, width, 0]); + } + + instance.delete(); + layout.delete(); + }); + + it('does not accumulate rounding errors when scaling a decimal cube step by step', () => { + const { widthField, layout } = makeSizeFields({ + is3DInstance: true, + defaultSize: [64.02, 64.02, 32.01], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(true); + + let width = 64.02; + for (let i = 0; i < 200; i++) { + width = Math.round((width + 1) * 100) / 100; + widthField.setValue(instance, width); + expect(getSize(instance)).toEqual([ + width, + width, + Math.round(width * 50) / 100, + ]); + } + + instance.delete(); + layout.delete(); + }); + + it('only changes the edited dimension when the ratio is not kept', () => { + const { widthField, heightField, layout } = makeSizeFields({ + is3DInstance: true, + defaultSize: [40, 20, 10], + }); + const instance = new gd.InitialInstance(); + instance.setShouldKeepRatio(false); + + widthField.setValue(instance, 0); + expect(getSize(instance)).toEqual([0, 20, 10]); + + heightField.setValue(instance, 0); + widthField.setValue(instance, 15); + expect(getSize(instance)).toEqual([15, 0, 10]); + + instance.delete(); + layout.delete(); + }); + + it('only changes the edited dimension when the others are not 0', () => { + const { widthField, layout } = makeSizeFields({ + is3DInstance: true, + defaultSize: [40, 20, 10], + }); + const instance = new gd.InitialInstance(); + instance.setHasCustomSize(true); + instance.setHasCustomDepth(true); + instance.setCustomWidth(0); + instance.setCustomHeight(30); + instance.setCustomDepth(10); + instance.setShouldKeepRatio(true); + + widthField.setValue(instance, 5); + expect(getSize(instance)).toEqual([5, 30, 10]); + + instance.delete(); + layout.delete(); + }); +});