Skip to content

Commit

Permalink
Always round the result of canvas interactions to the nearest whole p…
Browse files Browse the repository at this point in the history
…ixel (#4306)

* fix(canvas) Always round the result of canvas interactions to the nearest whole pixel

* fix(tests) Fix aspect ratio resize test

* fix(canvas) Apply rounding to snapping

* chore(tests) Update tests broken by whole pixel rounding
  • Loading branch information
Rheeseyb authored Oct 9, 2023
1 parent 04ea105 commit 58fd4cb
Show file tree
Hide file tree
Showing 26 changed files with 185 additions and 226 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ async function getGuidelineRenderResult(scale: number) {
data-testid='first-div'
style={{
position: 'absolute',
left: 50.5,
top: 146.5,
left: 50,
top: 146,
width: 50,
height: 50,
backgroundColor: 'red',
Expand All @@ -74,8 +74,8 @@ async function getGuidelineRenderResult(scale: number) {
data-testid='second-div'
style={{
position: 'absolute',
left: 110.5,
top: 206.5,
left: 110,
top: 206,
width: 7,
height: 9,
backgroundColor: 'blue',
Expand Down Expand Up @@ -428,14 +428,14 @@ describe('Snapping guidelines for absolutely moved element', () => {
const renderResult = await getGuidelineRenderResult(1)

expect(await getGuidelineDimensions(renderResult, 'guideline-0')).toEqual({
left: '110px',
top: '156px',
left: '109.5px',
top: '155.5px',
width: '0px',
height: '59px',
})
expect(await getGuidelineDimensions(renderResult, 'guideline-1')).toEqual({
left: '60px',
top: '206px',
left: '59.5px',
top: '205.5px',
width: '57px',
height: '0px',
})
Expand All @@ -456,44 +456,44 @@ describe('Snapping guidelines for absolutely moved element', () => {
{
guideline: {
type: 'XAxisGuideline',
x: 110.5,
yBottom: 215.5,
yTop: 156.5,
x: 110,
yBottom: 215,
yTop: 156,
},
snappingVector: {
x: 0,
y: 0,
},
pointsOfRelevance: [
{
x: 110.5,
y: 206.5,
x: 110,
y: 206,
},
{
x: 110.5,
y: 215.5,
x: 110,
y: 215,
},
],
},
{
guideline: {
type: 'YAxisGuideline',
xLeft: 60.5,
xRight: 117.5,
y: 206.5,
xLeft: 60,
xRight: 117,
y: 206,
},
snappingVector: {
x: 0,
y: 0,
},
pointsOfRelevance: [
{
x: 110.5,
y: 206.5,
x: 110,
y: 206,
},
{
x: 117.5,
y: 206.5,
x: 117,
y: 206,
},
],
},
Expand All @@ -504,14 +504,14 @@ describe('Snapping guidelines for absolutely moved element', () => {
const renderResult = await getGuidelineRenderResult(4)

expect(await getGuidelineDimensions(renderResult, 'guideline-0')).toEqual({
left: '110.375px',
top: '156.375px',
left: '109.875px',
top: '155.875px',
width: '0px',
height: '59px',
})
expect(await getGuidelineDimensions(renderResult, 'guideline-1')).toEqual({
left: '60.375px',
top: '206.375px',
left: '59.875px',
top: '205.875px',
width: '57px',
height: '0px',
})
Expand All @@ -532,44 +532,44 @@ describe('Snapping guidelines for absolutely moved element', () => {
{
guideline: {
type: 'XAxisGuideline',
x: 110.5,
yBottom: 215.5,
yTop: 156.5,
x: 110,
yBottom: 215,
yTop: 156,
},
snappingVector: {
x: 0,
y: 0,
},
pointsOfRelevance: [
{
x: 110.5,
y: 206.5,
x: 110,
y: 206,
},
{
x: 110.5,
y: 215.5,
x: 110,
y: 215,
},
],
},
{
guideline: {
type: 'YAxisGuideline',
xLeft: 60.5,
xRight: 117.5,
y: 206.5,
xLeft: 60,
xRight: 117,
y: 206,
},
snappingVector: {
x: 0,
y: 0,
},
pointsOfRelevance: [
{
x: 110.5,
y: 206.5,
x: 110,
y: 206,
},
{
x: 117.5,
y: 206.5,
x: 117,
y: 206,
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
magnitude,
offsetPoint,
pointDifference,
roundPointTo,
roundPointToNearestWhole,
zeroCanvasPoint,
} from '../../../core/shared/math-utils'
import type { ElementPath } from '../../../core/shared/project-file-types'
Expand Down Expand Up @@ -244,9 +244,8 @@ export function updateInteractionViaDragDelta(
movement: CanvasVector,
): InteractionSessionWithoutMetadata {
if (currentState.interactionData.type === 'DRAG') {
const accumulatedMovement = roundPointTo(
const accumulatedMovement = roundPointToNearestWhole(
offsetPoint(currentState.interactionData._accumulatedMovement, movement),
0,
)
const dragThresholdPassed = dragExceededThreshold(accumulatedMovement)
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1252,9 +1252,9 @@ describe('Absolute Resize Bounding Box Strategy single select', () => {
backgroundColor: '#aaaaaa33',
position: 'absolute',
top: 65,
left: 94,
left: 93,
bottom: 246,
right: 225
right: 226
}}
data-uid='ccc'
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ describe('when aspect ratio is locked', () => {
edgePosition(0.5, 1),
canvasPoint({ x: 20, y: 15 }),
{ width: 100, height: 200 },
{ width: 107.5, height: 215 },
{ width: 108, height: 215 },
shiftModifier,
)
})
Expand All @@ -309,7 +309,7 @@ describe('when aspect ratio is locked', () => {
edgePosition(0.5, 0),
canvasPoint({ x: 20, y: 15 }),
{ width: 100, height: 200 },
{ width: 92.5, height: 185 },
{ width: 93, height: 185 },
shiftModifier,
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import * as EP from '../../../../core/shared/element-path'
import type { ElementInstanceMetadataMap } from '../../../../core/shared/element-template'
import { getJSXElementNameLastPart } from '../../../../core/shared/element-template'
import type { CanvasRectangle } from '../../../../core/shared/math-utils'
import { canvasRectangle, offsetPoint, zeroCanvasPoint } from '../../../../core/shared/math-utils'
import {
canvasRectangle,
offsetPoint,
roundPointToNearestWhole,
zeroCanvasPoint,
} from '../../../../core/shared/math-utils'
import type { ElementPath } from '../../../../core/shared/project-file-types'
import { CSSCursor, Utils } from '../../../../uuiui-deps'
import { CSSCursor } from '../../../../uuiui-deps'
import type { InsertionSubject } from '../../../editor/editor-modes'
import type { EditorState, EditorStatePatch } from '../../../editor/store/editor-state'
import { foldAndApplyCommandsInner } from '../../commands/commands'
Expand Down Expand Up @@ -142,9 +147,7 @@ function dragToInsertStrategyFactory(
return []
}

const pointOnCanvas = Utils.roundPointToNearestHalf(
interactionSession.interactionData.dragStart,
)
const pointOnCanvas = roundPointToNearestWhole(interactionSession.interactionData.dragStart)
return insertionSubjects.map((s) => ({
subject: s,
frame: canvasRectangle({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ describe('Flex Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flexchild1'
Expand Down Expand Up @@ -535,7 +535,7 @@ describe('Flex Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flexchild1'
Expand Down Expand Up @@ -720,7 +720,7 @@ describe('Flex Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flexchild1'
Expand Down Expand Up @@ -906,7 +906,7 @@ describe('Flex Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flexchild1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ describe('Flex Resize', () => {
edgePosition(0.5, 1),
canvasPoint({ x: 20, y: 15 }),
{ width: 100, height: 200 },
{ width: 107.5, height: 215 },
{ width: 108, height: 215 },
shiftModifier,
)
})
Expand All @@ -299,7 +299,7 @@ describe('Flex Resize', () => {
edgePosition(0.5, 0),
canvasPoint({ x: 20, y: 15 }),
{ width: 100, height: 200 },
{ width: 92.5, height: 185 },
{ width: 93, height: 185 },
shiftModifier,
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe('Flow Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flowchild1'
Expand Down Expand Up @@ -390,7 +390,7 @@ describe('Flow Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flowchild1'
Expand Down Expand Up @@ -571,7 +571,7 @@ describe('Flow Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flowchild1'
Expand Down Expand Up @@ -753,7 +753,7 @@ describe('Flow Reparent To Absolute Strategy', () => {
height: 100,
backgroundColor: 'teal',
position: 'absolute',
left: -0.5,
left: 0,
top: 0,
}}
data-uid='flowchild1'
Expand Down
Loading

0 comments on commit 58fd4cb

Please sign in to comment.