freegroup / draw2d

Create Visio like drawings, diagrams or workflows with JavaScript and HTML5
https://freegroup.github.io/draw2d/#/examples
MIT License
738 stars 227 forks source link

Problems with zoom #54

Open Demak opened 5 years ago

Demak commented 5 years ago

Hi, found a few problems with zoom policy: https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/policy/canvas/WheelZoomPolicy.js#L114 This line should use height() instead of width()

https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/policy/canvas/WheelZoomPolicy.js#L149 This is always true when canvas dimensions are changed via setDimension here: https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/Canvas.js#L683 Because of that currently if you call canvas.setDimension when canvas is zoomed, zoom is not applied back after resize, which results in zoom being visually reset, but still applied on canvas coordinates calculation producing wrong results.

freegroup commented 4 years ago

I understand the first part of the ticket (width instead of Height)...but the second part is not clear to me. Can you please explain it in more detail?

Demak commented 4 years ago

Hi, it was quite a while ago, but as far as I can remember the problem in second part is that in setDimension we are calling this.setZoom with the first argument being zoomFactor of canvas, which after being passed to WheelZoomPolicy's zoom handler is checked for equality with itself in that condition and thus condition will always be true. Here is a flow: https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/Canvas.js#L683 https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/Canvas.js#L623 https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/policy/canvas/WheelZoomPolicy.js#L134 https://github.com/freegroup/draw2d/blob/f22835898ba3c1b091ee04ece91859f09b9ac45c/src/policy/canvas/WheelZoomPolicy.js#L149 in the end zoom and canvas.zoomFactor is the same variable. Thus there is no actions from WheelZoomPolicy's zoom handler on changing dimensions and as far as I can remember the problem was that zoom is reset to 1:1 in the start of setDimension but because of that condition was not applied back in the end.

Unfortunately I don't remember how I thought to fix it, haven't worked with draw2d for a while.

freegroup commented 4 years ago

I got it. Try to fix it

acrimu commented 3 years ago

I'm trying to use WeelZoomPolicy but I have some problems.

  1. If I try to use
    canvas.installEditPolicy(new draw2d.policy.canvas.WheelZoomPolicy({
    onMouseWheel: function(){
      // This is a <span>
      $("#draw2d_canvas_zoom").text(canvas.getZoom());
    }
    }));

    I get in console

    WheelZoomPolicy.js:159 Uncaught TypeError: Cannot read property 'zoomFactor' of null
    at Class._zoom (WheelZoomPolicy.js:159)
    at step (WheelZoomPolicy.js:137)
    at t.value (shifty.js:2)
    at shifty.js:2
phiferd commented 3 years ago

Not sure if this is the same issue, but there is a bug in the wheel zoom policy.

This line calls _zoom with three parameters (zoom, centerX, centerY):

https://github.com/freegroup/draw2d/blob/master/src/policy/canvas/WheelZoomPolicy.js#L127

But _zoom expects two parameters (zoom, centerPoint).