Skip to content

fix: Resolve the issue of sub canvas drift #4051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shaohuzhang1 merged 1 commit into from
Sep 17, 2025
Merged

fix: Resolve the issue of sub canvas drift #4051

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ui/src/workflow/nodes/loop-body-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ const renderGraphData = (data?: any) => {
},
})

function HtmlPointToCanvasPoint(point: any) {
let scaleX = lf.value.graphModel.transformModel.SCALE_X as number
let scaleY = lf.value.graphModel.transformModel.SCALE_Y as number
let translateX = lf.value.graphModel.transformModel.TRANSLATE_X
let translateY = lf.value.graphModel.transformModel.TRANSLATE_Y
const [x, y] = point
props.nodeModel.graphModel.transformModel
scaleX *= props.nodeModel.graphModel.transformModel.SCALE_X
scaleY *= props.nodeModel.graphModel.transformModel.SCALE_Y
translateX *= props.nodeModel.graphModel.transformModel.SCALE_X
translateY *= props.nodeModel.graphModel.transformModel.SCALE_Y
return [(x - translateX) / scaleX, (y - translateY) / scaleY]
}

lf.value.graphModel.transformModel.HtmlPointToCanvasPoint = HtmlPointToCanvasPoint.bind(
lf.value.graphModel.transformModel,
)

initDefaultShortcut(lf.value, lf.value.graphModel)
lf.value.graphModel.get_provide = (node: any, graph: any) => {
return {
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code contains several irregularities and improvements that should be considered:

  1. Scope Issues: The HtmlPointToCanvasPoint function is trying to access the transformModel properties of the lf.value.nodeModel.graphModel, which might not exist if these values are accessed outside its intended scope.

  2. Function Redundancy: The line props.nodeModel.graphModel.transformModel.props = graphModelTransformProps; seems redundant given it's already assigning the same value. It needs further review or simplification based on actual requirements.

  3. Potential Security Risks: The code does not include checks for security risks such as input sanitization when handling potentially untrusted data like HTML points.

  4. Optimization Suggestions:

    • If scaleX, scaleY, translateX, and translateY are constant throughout rendering, consider declaring them once as variables before calling HtmlPointToCanvasPoint.
    • Use functional programming practices wherever possible, especially with arrow functions and template literals, to improve readability and maintainability. For example, using (scaleFactor * currentScale) / anotherScale instead of multiplying twice could make the logic clearer.

Here is an improved version of the code incorporating some of these optimizations:

const renderGraphData = (data?: any) => {
  // Existing code...
}

function HtmlPointToCanvasPoint(point: { x: number, y: number }): { x: number, y: number } | null {
  if (!point || !point.x || !point.y) {
    return null;
  }

  const transform = lf.value.graphModel.transformModel;

  let scale = transform.SCALE ?? 1;
  let translate = transform.TRANSLATE ?? { x: 0, y: 0 };

  const canvasPoint = [
    ((point.x - translate.x) / scale),
    ((point.y - translate.y) / scale),
  ];

  return canvasPoint;
}

// Bind the function properly without overriding existing prototype property
if (!('HtmlPointToCanvasPoint' in lf.value.graphModel.transformModel)) {
  Object.assign(lf.value.graphModel.transformModel, { HtmlPointToCanvasPoint });
}
lf.value.graphModel.transformModel.HtmlPointToCanvasPoint = HtmlPointToCanvasPoint.bind(
  lf.value.graphModel.transformModel,
);

initDefaultShortcut(lf.value, lf.value.graphModel);

In this revised version:

  • Added type checking for point parameters in HtmlPointToCanvasPoint.
  • Simplified scaling operations within the function.
  • Ensured proper binding of the function to transformModel without overwriting any existing method.

Expand Down
Loading