Skip to content
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.

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

fix(core): snap the text pos of cloneNode to grid(#1545) #1550

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

wbccb
Copy link
Contributor

@wbccb wbccb commented Apr 5, 2024

fix #1545
fix #1440 => related: transform相关问题汇总

问题发生的原因

menu鼠标右键点击复制时,会触发

// packages/core/src/LogicFlow.tsx
cloneNode(nodeId) {
    const Model = this.graphModel.getNodeModelById(nodeId);
    const data = Model.getData();
    const { guards } = this.options;
    const enabledClone = guards && guards.beforeClone ? guards.beforeClone(data) : true;
    if (enabledClone) {
      return this.graphModel.cloneNode(nodeId);
    }
}

最终执行逻辑也就是this.graphModel.cloneNode(nodeId)

// packages/core/src/model/GraphModel.ts
cloneNode(nodeId) {
  const Model = this.getNodeModelById(nodeId);
  const data = Model.getData();
  data.x += 30;
  data.y += 30;
  //...
  if (data.text) {
    data.text.x += 30;
    data.text.y += 30;
  }
  const nodeModel = this.addNode(data);
  //...
  return nodeModel.getData();
}

从上面代码可以知道,复制node时会增加30左右的偏移量,然后触发this.addNode()进行克隆节点的复制

// packages/core/src/model/GraphModel.ts
function addNode(nodeConfig, eventType, event) {
  const nodeOriginData = formatData(nodeConfig);
  // 添加节点的时候,如果这个节点Id已经存在,则采用新的id
  if (nodeOriginData.id && this.nodesMap[nodeConfig.id]) {
    delete nodeOriginData.id;
  }
  const Model = this.getModel(nodeOriginData.type);
  if (!Model) {
    throw new Error(
      `找不到${nodeOriginData.type}对应的节点,请确认是否已注册此类型节点。`
    );
  }
  nodeOriginData.x = snapToGrid(nodeOriginData.x, this.gridSize);
  nodeOriginData.y = snapToGrid(nodeOriginData.y, this.gridSize);
  const nodeModel = new Model(nodeOriginData, this);
  this.nodes.push(nodeModel);
  const nodeData = nodeModel.getData();
  //...
  return nodeModel;
}

从上面this.addNode()方法可以知道,会对坐标进行一定的修正,也就是下面代码所示

function snapToGrid(point, gridSize) {
  // 保证 x, y 的值为 gridSize 的整数倍
  return gridSize * Math.round(point / gridSize) || point;
}

因此问题就出在,当this.gridSize不为1时,node.xnode.y会根据this.gridSize进行一定的修正,但是node.text.xnode.text.y却没有进行根据this.gridSize进行一定的修正,导致了位置错误

而在GraphModel.ts的其它方法中,我们可以发现一些相似的代码,如下面代码所示

// packages/core/src/model/GraphModel.ts
function graphDataToModel(graphData) {
  //...
  if (graphData.nodes) {
    this.nodes = map(graphData.nodes, (node) => {
      const Model = this.getModel(node.type);
      if (!Model) {
        throw new Error(`找不到${node.type}对应的节点。`);
      }
      const { x: nodeX, y: nodeY } = node;
      // 根据 grid 修正节点的 x, y
      if (nodeX && nodeY) {
        node.x = snapToGrid(nodeX, this.gridSize);
        node.y = snapToGrid(nodeY, this.gridSize);
        if (typeof node.text === "object") {
          node.text.x -= getGridOffset(nodeX, this.gridSize);
          node.text.y -= getGridOffset(nodeY, this.gridSize);
        }
      }
      return new Model(node, this);
    });
  } else {
    this.nodes = [];
  }
  //...
}

在执行snapToGrid()之后,是有进行node.text.x -= getGridOffset(nodeX, this.gridSize),因此#1545的问题就在于,缺失了node.text.xnode.text.y根据this.gridSize进行一定坐标修正的逻辑

解决方法

node.text进行gridSize的处理

graphDataToModel()node.text.xnode.text.y的处理补充到addNode()

// packages/core/src/model/GraphModel.ts
function addNode(nodeConfig, eventType, event) {
  const nodeOriginData = formatData(nodeConfig);
  // 添加节点的时候,如果这个节点Id已经存在,则采用新的id
  if (nodeOriginData.id && this.nodesMap[nodeConfig.id]) {
    delete nodeOriginData.id;
  }
  const Model = this.getModel(nodeOriginData.type);
  if (!Model) {
    throw new Error(
      `找不到${nodeOriginData.type}对应的节点,请确认是否已注册此类型节点。`
    );
  }
  nodeOriginData.x = snapToGrid(nodeOriginData.x, this.gridSize);
  nodeOriginData.y = snapToGrid(nodeOriginData.y, this.gridSize);
  if (typeof node.text === "object") {
    node.text.x -= getGridOffset(nodeX, this.gridSize);
    node.text.y -= getGridOffset(nodeY, this.gridSize);
  }
  const nodeModel = new Model(nodeOriginData, this);
  this.nodes.push(nodeModel);
  const nodeData = nodeModel.getData();
  //...
  return nodeModel;
}

优化1

我们可以发现graphDataToModel()addNode()中创建new Model(nodeOriginData, this)的代码一模一样,因此可以抽离出一个公共方法

function getModelAfterSnapToGrid(node) {
  const Model = this.getModel(node.type);
  if (!Model) {
    throw new Error(
      `找不到${node.type}对应的节点,请确认是否已注册此类型节点。`
    );
  }
  const { x: nodeX, y: nodeY } = node;
  // 根据 grid 修正节点的 x, y
  if (nodeX && nodeY) {
    node.x = snapToGrid(nodeX, this.gridSize);
    node.y = snapToGrid(nodeY, this.gridSize);
    if (typeof node.text === "object") {
      node.text.x += node.x - nodeX;
      node.text.y += node.y - nodeY;
    }
  }
  return new Model(node, this);
}

优化2

getGridOffset()存在一定的问题,可能会导致整体坐标计算错误

node.x = snapToGrid(nodeX, this.gridSize);
node.y = snapToGrid(nodeY, this.gridSize);
if (typeof node.text === "object") {
  node.text.x -= getGridOffset(nodeX, this.gridSize);
  node.text.y -= getGridOffset(nodeY, this.gridSize);
}

function snapToGrid(point, gridSize) {
  // 保证 x, y 的值为 gridSize 的整数倍
  return gridSize * Math.round(point / gridSize) || point;
}
function getGridOffset(distance, gridSize) {
  return distance % gridSize;
}

我们发现一个问题,当初始化参数grid=true,此时this.gridSize=20

  • 但是当nodeX=29时,snapToGrid(29, 20)=20getGridOffset(29, 20)=9node坐标从29->20,减少了9node.text坐标也减少了9,这样结果是正确的
  • 如果nodeX=31时,snapToGrid(31, 20)=40getGridOffset(31, 20)=11node坐标从31->40,增加了9node.text坐标由于是node.text.x -= getGridOffset(nodeX, this.gridSize),因此减少了11,这样的结果是错的

因此使用Math.round()这种四舍五入的做法,我们无法判断到底该node.text.x -= getGridOffset(nodeX, this.gridSize)还是node.text.x += getGridOffset(nodeX, this.gridSize)

// 原来的处理是:node.text.x -= getGridOffset(nodeX, this.gridSize);
// 由于snapToGrid()使用了Math.round()四舍五入的做法,因此无法判断需要执行
// node.text.x = node.text.x + getGridOffset()
// 还是
// node.text.x = node.text.x - getGridOffset()
// 直接改为node.x - nodeX就可以满足上面的要求
node.text.x += (node.x - nodeX);
node.text.y += (node.y - nodeY);

单测

  • 增加了1545-spec.test.ts简单模拟了下bug
  • 增加graphmodel.test.ts对新增方法getModelAfterSnapToGrid()进行测试

未解决的问题

虽然单元测试能够Pass,但是最后会报错
graphmodel

测试了其它已经存在的单元测试,比如packages/core/__tests__/logicflow.test.ts,也存在这个问题
graphmodel

暂时还没找到解决方法,后面有空的时候可能要集中处理下

@DymoneLewis DymoneLewis merged commit 6c47f7d into didi:master Apr 9, 2024
@wbccb wbccb deleted the fix/1545 branch April 9, 2024 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants