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(Path): bbox stroke projection #8040

Closed
wants to merge 49 commits into from
Closed

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Jun 30, 2022

closes #8025
closes #8036

And tests fail because Path has broken it's positioning logic somehow... I don't yet understand how or why.

TODO

  • fix positioning

if (!options.fromSVG) {
origin = this.translateToGivenOrigin(
// this looks bad, but is one way to keep it optional for now.
new fabric.Point(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like this. Because it doesn't take into account the translation caused by stroke width

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved I believe

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

Seems ready apart from annoying semantics on unit tests

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

Not bad

var v1 = pointBefore.subtract(point);
var v2 = pointAfter.subtract(point);
var angle = fabric.util.calcAngleBetweenVectors(v1, v2);
if (Math.abs(angle) < Math.PI / 2) {
Copy link
Contributor Author

@ShaMan123 ShaMan123 Jul 2, 2022

Choose a reason for hiding this comment

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

If we project all angles some bboxes of visual tests fix while other visual tests break

  • clipping13
    image

  • pathWithGradientSvg
    image

  • arc3 image

}
},

_setPositionDimensions: function (options) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a wretched function

// calculate bevel, round projections
// incorrect approximation
scalar = -s * Math.SQRT2;
return bisectorVector.scalarMultiply(scalar);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesn't cover bevel/round cases properly

@@ -369,7 +369,7 @@
});

function clipping13(canvas, callback) {
var jsonData = '{"version":"2.4.6","objects":[{"type":"path","version":"2.4.6","left":84.63385601266276,"top":385.11623376730074,"width":3.14,"height":10.44,"fill":{"type":"linear","coords":{"x1":481.066,"y1":785.465,"x2":480.953,"y2":793.102},"colorStops":[{"offset":1,"color":"rgb(160,137,44)","opacity":1},{"offset":0.9,"color":"rgb(85,68,0)","opacity":1},{"offset":0.78,"color":"rgb(80,68,22)","opacity":1},{"offset":0.607,"color":"rgb(160,137,44)","opacity":1},{"offset":0.467,"color":"rgb(255,255,255)","opacity":1},{"offset":0.299,"color":"rgb(200,171,55)","opacity":1},{"offset":0.24,"color":"rgb(160,137,44)","opacity":1},{"offset":0.096,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(211,188,95)","opacity":1}],"offsetX":-439.1113425994523,"offsetY":-783.951,"gradientTransform":[-1,0,0,1,921.58,0]},"scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",442.142,794.394],["l",-2.583,-1.46],["c",-0.644,-2.39,-0.512,-5.004,-0.113,-7.693],["l",2.808,-1.29]]},{"type":"path","version":"2.4.6","left":78.08737427855635,"top":315.5127636903678,"width":2.42,"height":11.51,"fill":{"type":"linear","coords":{"x1":473.934,"y1":792.821,"x2":473.822,"y2":784.005},"colorStops":[{"offset":1,"color":"rgb(211,188,95)","opacity":1},{"offset":0.904,"color":"rgb(120,103,33)","opacity":1},{"offset":0.76,"color":"rgb(160,137,44)","opacity":1},{"offset":0.701,"color":"rgb(200,171,55)","opacity":1},{"offset":0.533,"color":"rgb(255,255,255)","opacity":1},{"offset":0.393,"color":"rgb(160,137,44)","opacity":1},{"offset":0.22,"color":"rgb(80,68,22)","opacity":1},{"offset":0.1,"color":"rgb(85,68,0)","opacity":1},{"offset":0,"color":"rgb(160,137,44)","opacity":1}],"offsetX":-446.578,"offsetY":-783.332,"gradientTransform":[-1,0,0,1,921.58,0]},"scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",448.32,783.332],["l",0.673,2.02],["c",-0.885,1.242,-0.83,2.952,-0.075,7.92],["l",-1.16,1.57],["l",-1.18,-4.49],["l",0.17,-5.673],["z"]]},{"type":"path","version":"2.4.6","left":92.34368668174653,"top":366.5275069557187,"width":6.57,"height":5.67,"fill":{"type":"linear","coords":{"x1":475.081,"y1":785.381,"x2":479.3,"y2":788.975},"colorStops":[{"offset":1,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(200,171,55)","opacity":1}],"offsetX":-441.1054336190674,"offsetY":-784.68,"gradientTransform":[-1,0,0,1,921.58,0]},"scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.365,784.68],["h",-5.616],["c",-0.592,1.87,-0.812,3.743,-0.506,5.615],["l",5.952,0.056],["c",0.658,-1.976,0.554,-3.843,0.17,-5.67],["z"]]},{"type":"path","version":"2.4.6","left":151.23029459047214,"top":365.75783100618594,"width":6.58,"height":4.48,"fill":{"type":"linear","coords":{"x1":476.181,"y1":791.235,"x2":477.099,"y2":794.257},"colorStops":[{"offset":1,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(200,171,55)","opacity":1}],"offsetX":-441.18800000000005,"offsetY":-790.248,"gradientTransform":[-1,0,0,1,921.58,0]},"scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.196,790.248],["c",0.397,1.492,0.613,3.14,0.562,4.483],["l",-5.503,-0.056],["c",-0.974,-1.853,-0.957,-3.128,-1.067,-4.406],["z"]]},{"type":"path","version":"2.4.6","left":78.67962464545468,"top":360.50959855742764,"width":6.4,"height":1.35,"fill":{"type":"linear","coords":{"x1":473.32,"y1":784.06,"x2":479.896,"y2":784.06},"colorStops":[{"offset":1,"color":"rgb(200,171,55)","opacity":1},{"offset":0,"color":"rgb(120,103,33)","opacity":1}],"offsetX":-441.751,"offsetY":-783.3879999999999,"gradientTransform":[-1,0,0,1,921.58,0]},"scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.196,784.68],["c",0.783,-0.43,0.715,-0.862,0.955,-1.292],["l",-5.67,0.224],["c",-0.308,0.343,-0.704,0.64,-0.73,1.123],["z"]]},{"type":"path","version":"2.4.6","left":195.70884317712697,"top":264.26532049868615,"width":2.99,"height":8.78,"fill":{"type":"linear","coords":{"x1":481.066,"y1":785.465,"x2":480.953,"y2":793.102},"colorStops":[{"offset":1,"color":"rgb(160,137,44)","opacity":1},{"offset":0.9,"color":"rgb(85,68,0)","opacity":1},{"offset":0.78,"color":"rgb(80,68,22)","opacity":1},{"offset":0.607,"color":"rgb(160,137,44)","opacity":1},{"offset":0.467,"color":"rgb(255,255,255)","opacity":1},{"offset":0.299,"color":"rgb(200,171,55)","opacity":1},{"offset":0.24,"color":"rgb(160,137,44)","opacity":1},{"offset":0.096,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(211,188,95)","opacity":1}],"offsetX":-228.298,"offsetY":-835.244,"gradientTransform":[0.93343,0,0,0.85628,-219.064,163.965]},"scaleX":9.32,"scaleY":10.58,"angle":90,"flipY":true,"path":[["M",228.298,844.02],["l",2.57,-1.083],["c",0.6,-2.048,0.477,-4.285,0.104,-6.587],["l",-2.62,-1.106]]},{"type":"circle","version":"2.4.6","left":-28.49,"top":-28.49,"width":4.64,"height":4.64,"fill":{"type":"radial","coords":{"x1":193.676,"y1":141.252,"x2":193.676,"y2":141.252,"r1":0,"r2":4.082},"colorStops":[{"offset":1,"color":"rgb(0,0,0)","opacity":1},{"offset":0.969,"color":"rgb(0,0,0)","opacity":1},{"offset":0.904,"color":"rgb(236,236,236)","opacity":1},{"offset":0.874,"color":"rgb(77,77,77)","opacity":1},{"offset":0.837,"color":"rgb(237,237,237)","opacity":1},{"offset":0.817,"color":"rgb(0,0,0)","opacity":1},{"offset":0,"color":"rgb(0,0,0)","opacity":1}],"offsetX":-116.293,"offsetY":-166.167,"gradientTransform":[0.3487,0.40483,-0.40345,0.34752,108.054,40.97]},"scaleX":59.8,"scaleY":59.8,"radius":2.321,"startAngle":0,"endAngle":360},{"type":"circle","version":"2.4.6","left":32.24,"top":30.43,"width":3.58,"height":3.58,"fill":{"type":"linear","coords":{"x1":195.171,"y1":143.461,"x2":191.574,"y2":138.568},"colorStops":[{"offset":1,"color":"rgb(204,204,204)","opacity":1},{"offset":0.687,"color":"rgb(255,255,255)","opacity":1},{"offset":0,"color":"rgb(255,255,255)","opacity":1}],"offsetX":-116.817,"offsetY":-166.661,"gradientTransform":[0.52872,0,0,0.52872,16.3,93.714]},"stroke":"#b3b3b3","strokeWidth":0.02,"strokeLineCap":"round","scaleX":59.8,"scaleY":59.8,"radius":1.789,"startAngle":0,"endAngle":360},{"type":"circle","version":"2.4.6","left":125.91,"top":124.11,"width":0.46,"height":0.46,"fill":{"type":"linear","coords":{"x1":656.429,"y1":320.934,"x2":506.429,"y2":131.648},"colorStops":[{"offset":1,"color":"rgb(242,242,242)","opacity":1},{"offset":0,"color":"rgb(102,102,102)","opacity":1}],"offsetX":-118.37599999999999,"offsetY":-168.22,"gradientTransform":[0.0017,0,0,0.0017,117.64,168.082]},"stroke":"#999","strokeWidth":0,"strokeLineCap":"round","scaleX":59.8,"scaleY":59.8,"radius":0.23,"startAngle":0,"endAngle":360}]}';
var jsonData = '{"version":"5.1.0","objects":[{"type":"group","version":"5.1.0","left":-28.49,"top":-28.49,"width":337.3916,"height":413.6066,"objects":[{"type":"path","version":"5.1.0","left":-55.572,"top":206.8033,"width":3.1427,"height":10.443,"fill":{"type":"linear","coords":{"x1":481.066,"y1":785.465,"x2":480.953,"y2":793.102},"colorStops":[{"offset":1,"color":"rgb(160,137,44)","opacity":1},{"offset":0.9,"color":"rgb(85,68,0)","opacity":1},{"offset":0.78,"color":"rgb(80,68,22)","opacity":1},{"offset":0.607,"color":"rgb(160,137,44)","opacity":1},{"offset":0.467,"color":"rgb(255,255,255)","opacity":1},{"offset":0.299,"color":"rgb(200,171,55)","opacity":1},{"offset":0.24,"color":"rgb(160,137,44)","opacity":1},{"offset":0.096,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(211,188,95)","opacity":1}],"offsetX":-439.1113425994523,"offsetY":-783.951,"gradientUnits":"pixels","gradientTransform":[-1,0,0,1,921.58,0]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",442.142,794.394],["L",439.55899999999997,792.934],["C",438.91499999999996,790.544,439.04699999999997,787.93,439.44599999999997,785.241],["L",442.25399999999996,783.951]]},{"type":"path","version":"5.1.0","left":-64.4041,"top":137.1995,"width":2.7626,"height":12.3327,"fill":{"type":"linear","coords":{"x1":473.934,"y1":792.821,"x2":473.822,"y2":784.005},"colorStops":[{"offset":1,"color":"rgb(211,188,95)","opacity":1},{"offset":0.904,"color":"rgb(120,103,33)","opacity":1},{"offset":0.76,"color":"rgb(160,137,44)","opacity":1},{"offset":0.701,"color":"rgb(200,171,55)","opacity":1},{"offset":0.533,"color":"rgb(255,255,255)","opacity":1},{"offset":0.393,"color":"rgb(160,137,44)","opacity":1},{"offset":0.22,"color":"rgb(80,68,22)","opacity":1},{"offset":0.1,"color":"rgb(85,68,0)","opacity":1},{"offset":0,"color":"rgb(160,137,44)","opacity":1}],"offsetX":-446.578,"offsetY":-783.115958749821,"gradientUnits":"pixels","gradientTransform":[-1,0,0,1,921.58,0]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",448.32,783.332],["L",448.993,785.352],["C",448.108,786.5939999999999,448.163,788.304,448.918,793.2719999999999],["L",447.758,794.842],["L",446.578,790.352],["L",446.748,784.679],["z"]]},{"type":"path","version":"5.1.0","left":-59.9006,"top":191.9376,"width":6.9652,"height":7.8907,"fill":{"type":"linear","coords":{"x1":475.081,"y1":785.381,"x2":479.3,"y2":788.975},"colorStops":[{"offset":1,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(200,171,55)","opacity":1}],"offsetX":-440.70594607384373,"offsetY":-783.5421461913462,"gradientUnits":"pixels","gradientTransform":[-1,0,0,1,921.58,0]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.365,784.68],["L",441.749,784.68],["C",441.15700000000004,786.55,440.937,788.423,441.24300000000005,790.295],["L",447.19500000000005,790.351],["C",447.85300000000007,788.375,447.749,786.508,447.36500000000007,784.681],["z"]]},{"type":"path","version":"5.1.0","left":1.4031,"top":190.1394,"width":7.4814,"height":6.2775,"fill":{"type":"linear","coords":{"x1":476.181,"y1":791.235,"x2":477.099,"y2":794.257},"colorStops":[{"offset":1,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(200,171,55)","opacity":1}],"offsetX":-440.8988611577768,"offsetY":-789.3386038208589,"gradientUnits":"pixels","gradientTransform":[-1,0,0,1,921.58,0]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.196,790.248],["C",447.593,791.74,447.809,793.388,447.75800000000004,794.731],["L",442.25500000000005,794.675],["C",441.28100000000006,792.822,441.29800000000006,791.5469999999999,441.18800000000005,790.269],["z"]]},{"type":"path","version":"5.1.0","left":-67.4486,"top":184.3337,"width":6.8373,"height":2.4714,"fill":{"type":"linear","coords":{"x1":473.32,"y1":784.06,"x2":479.896,"y2":784.06},"colorStops":[{"offset":1,"color":"rgb(200,171,55)","opacity":1},{"offset":0,"color":"rgb(120,103,33)","opacity":1}],"offsetX":-441.5216681728531,"offsetY":-782.828220356729,"gradientUnits":"pixels","gradientTransform":[-1,0,0,1,921.58,0]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":-90,"path":[["M",447.196,784.68],["C",447.97900000000004,784.25,447.911,783.818,448.151,783.3879999999999],["L",442.481,783.612],["C",442.173,783.9549999999999,441.777,784.252,441.751,784.735],["z"]]},{"type":"path","version":"5.1.0","left":55.503,"top":85.9516,"width":2.9869,"height":8.776,"fill":{"type":"linear","coords":{"x1":481.066,"y1":785.465,"x2":480.953,"y2":793.102},"colorStops":[{"offset":1,"color":"rgb(160,137,44)","opacity":1},{"offset":0.9,"color":"rgb(85,68,0)","opacity":1},{"offset":0.78,"color":"rgb(80,68,22)","opacity":1},{"offset":0.607,"color":"rgb(160,137,44)","opacity":1},{"offset":0.467,"color":"rgb(255,255,255)","opacity":1},{"offset":0.299,"color":"rgb(200,171,55)","opacity":1},{"offset":0.24,"color":"rgb(160,137,44)","opacity":1},{"offset":0.096,"color":"rgb(120,103,33)","opacity":1},{"offset":0,"color":"rgb(211,188,95)","opacity":1}],"offsetX":-228.298,"offsetY":-835.244,"gradientUnits":"pixels","gradientTransform":[0.9334,0,0,0.8563,-219.064,163.965]},"stroke":"","scaleX":9.32,"scaleY":10.58,"angle":90,"flipY":true,"path":[["M",228.298,844.02],["L",230.868,842.937],["C",231.468,840.889,231.345,838.652,230.972,836.35],["L",228.352,835.244]]},{"type":"circle","version":"5.1.0","left":-168.6958,"top":-206.8033,"width":4.642,"height":4.642,"fill":{"type":"radial","coords":{"x1":193.676,"y1":141.252,"x2":193.676,"y2":141.252,"r1":0,"r2":4.082},"colorStops":[{"offset":1,"color":"rgb(0,0,0)","opacity":1},{"offset":0.9689999999999999,"color":"rgb(0,0,0)","opacity":1},{"offset":0.904,"color":"rgb(236,236,236)","opacity":1},{"offset":0.8740000000000001,"color":"rgb(77,77,77)","opacity":1},{"offset":0.8370000000000001,"color":"rgb(237,237,237)","opacity":1},{"offset":0.8169999999999998,"color":"rgb(0,0,0)","opacity":1},{"offset":0,"color":"rgb(0,0,0)","opacity":1}],"offsetX":2.321,"offsetY":2.321,"gradientUnits":"pixels","gradientTransform":[0.3487,0.4048,-0.4034,0.3475,-10.56,-127.518]},"stroke":"","scaleX":59.8,"scaleY":59.8,"radius":2.321},{"type":"circle","version":"5.1.0","left":-107.9658,"top":-147.8833,"width":3.578,"height":3.578,"fill":{"type":"linear","coords":{"x1":195.171,"y1":143.461,"x2":191.574,"y2":138.568},"colorStops":[{"offset":1,"color":"rgb(204,204,204)","opacity":1},{"offset":0.687,"color":"rgb(255,255,255)","opacity":1},{"offset":0,"color":"rgb(255,255,255)","opacity":1}],"offsetX":1.789,"offsetY":1.789,"gradientUnits":"pixels","gradientTransform":[0.5287,0,0,0.5287,-102.306,-74.736]},"stroke":"rgb(179,179,179)","strokeWidth":0.02,"strokeLineCap":"round","scaleX":59.8,"scaleY":59.8,"radius":1.789},{"type":"circle","version":"5.1.0","left":-14.2958,"top":-54.2033,"width":0.46,"height":0.46,"fill":{"type":"linear","coords":{"x1":656.429,"y1":320.934,"x2":506.429,"y2":131.648},"colorStops":[{"offset":1,"color":"rgb(242,242,242)","opacity":1},{"offset":0,"color":"rgb(102,102,102)","opacity":1}],"offsetX":0.23,"offsetY":0.23,"gradientUnits":"pixels","gradientTransform":[0.0017,0,0,0.0017,-0.966,-0.368]},"stroke":"rgb(153,153,153)","strokeWidth":0,"strokeLineCap":"round","scaleX":59.8,"scaleY":59.8,"radius":0.23}]}]}';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR is breaking
So loading json from prev versions will not do. However exporting to svg from one version and importing to another works.
This is how I fixed this test

* @param {Point} A
* @param {Point} B
* @param {Point} C
* @param {Point} A the vertex of the bisector
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrong desc

@stale
Copy link

stale bot commented Aug 12, 2022

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.

@stale stale bot added the stale Issue marked as stale by the stale bot label Aug 12, 2022
@ShaMan123 ShaMan123 removed the stale Issue marked as stale by the stale bot label Aug 13, 2022
@stale
Copy link

stale bot commented Sep 21, 2022

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.

@stale stale bot added the stale Issue marked as stale by the stale bot label Sep 21, 2022
@ShaMan123 ShaMan123 removed the stale Issue marked as stale by the stale bot label Sep 21, 2022
@stale
Copy link

stale bot commented Oct 29, 2022

This pull request 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.

@stale stale bot added the stale Issue marked as stale by the stale bot label Oct 29, 2022
@ShaMan123 ShaMan123 added bug fix-next and removed stale Issue marked as stale by the stale bot bug labels Oct 29, 2022
@ShaMan123 ShaMan123 mentioned this pull request Oct 30, 2022
7 tasks
@ShaMan123 ShaMan123 linked an issue Oct 30, 2022 that may be closed by this pull request
7 tasks
@ShaMan123
Copy link
Contributor Author

Anyone looking to impl this I suggest using bezier-js

@ShaMan123 ShaMan123 closed this May 7, 2024
@ShaMan123 ShaMan123 deleted the path-bbox-stroke-projection branch May 7, 2024 19:35
@asturur asturur restored the path-bbox-stroke-projection branch May 7, 2024 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Path stroke bbox bug(Path): stroke projection on bbox Stroke for Path gets cut off
1 participant