Record group selection in transformed coordinates to support panning/zooming while group selecting #7088
Conversation
…zooming while group selecting
| ex: pointer.x, | ||
| ey: pointer.y, | ||
| ex: this._absolutePointer.x, | ||
| ey: this._absolutePointer.y, |
There was a problem hiding this comment.
i dream a day in which the local pointer goes away entirely and fabric uses only the absolute values, and we remove most of the conversions.
src/canvas.class.js
Outdated
| fabric.util.drawDashedLine(ctx, minX, minY, maxX, minY, this.selectionDashArray); | ||
| fabric.util.drawDashedLine(ctx, minX, maxY, maxX, maxY, this.selectionDashArray); | ||
| fabric.util.drawDashedLine(ctx, minX, minY, minX, maxY, this.selectionDashArray); | ||
| fabric.util.drawDashedLine(ctx, maxX, minY, maxX, maxY, this.selectionDashArray); |
There was a problem hiding this comment.
would you check if IE11 supports dashed stroke? if it does, we can finally get rid of the block (!supportedLineDash)
There was a problem hiding this comment.
Here is the support link. Looks like IE 11 supports but I'm not sure if the support is good enough on other browsers like chrome? https://caniuse.com/mdn-api_canvasrenderingcontext2d_setlinedash
There was a problem hiding this comment.
yes setLineDash is an old feature that was missing on ie10 only.
Since we do not support ie10 anymore, this is safe to go.
|
I think this PR is fine. |
| supportLineDash = fabric.StaticCanvas.supports('setLineDash'), | ||
| isTouchEvent = fabric.util.isTouchEvent, | ||
| STROKE_OFFSET = 0.5; |
There was a problem hiding this comment.
i think STROKE_OFFSET is still needed. it is used because mouse events are always round numbers, while we want to draw single pixels line at 0.5.
There was a problem hiding this comment.
If I remember correctly, I think this actually to account for canvas drawing strokes down the center of the line. The hardcoded stroke offset of 0.5 works well for the default selectionLineWidth of 1, but doesn't look right for larger values. I think what I did accomplishes the same thing but allows for selectionLineWidth != 1
|
@SLKnutson any chance we can move this forward. |
|
@asturur You can make changes as needed. I will eventually have time for this, but I'm not sure how far out in the future that would be. Thanks! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Fixes this issue: #7086
There is still a slight issue because the group select controls don't get immediately rerendered when transforming, but I didn't want to shove too much into a single PR. I thought about making a feature toggle on fabric.Canvas, but I couldn't think of any use cases for the way it behaved before. I didn't see any existing methods of testing the contents of _drawSelection(). I'm glad to make changes as needed.
I changed _drawSelection to accommodate varying selectionLineWidth, since the previous implementation was essentially hardcoded to 1, and would draw outside the box for values > 1, now it should always draw within the box.
Thank you!