search

dikalo / lienzo-core

Lienzo Structure Graphics GWT Toolkit
Apache License 2.0
51 stars 58 forks source link

Fix to make straight lines possible. #194

Closed Josephblt closed 6 years ago

Josephblt commented 6 years ago

Fix to make straight lines possible. @romartin.

romartin commented 6 years ago

Do NOT merge yet please - needs review

romartin commented 6 years ago

Hey @Josephblt , Tested and works fine, but on the other hand I think you're updating here the CPs for the OrthogonalPolyline, instead of changing the line drawing between CPs, as @mdproctor recommended. Isn't it? Can pleae @mdproctor confirm? PS: Probably the fix now it's just doing the points.copy() before your correctBreakDistance? (I mean in the parse method)

romartin commented 6 years ago

Yes @Josephblt , you're in fact changing the CPs of the line, which can cause trouble at some points... not sure if you're around, anyway don't worry lemme try to fix this and create a PR for it now.

SprocketNYC commented 6 years ago

No merge yet, right?

Josephblt commented 6 years ago

Hey Roger. The points collection is cloned inside the correctBreakDistance method to avoid changing Control Points position.

Em 23 de set de 2017 17:32, "Roger Martínez" notifications@github.com escreveu:

Yes @Josephblt https://github.com/josephblt , you're in facr changing the CPs of the line, which can cause trouble at some points... not sure if you're around, anyway don't worry lemme try to fix this and create a PR for it now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ahome-it/lienzo-core/pull/194#issuecomment-331668135, or mute the thread https://github.com/notifications/unsubscribe-auth/AYMakg7sBjgrvtkoBKYfeMAVNO2m06qZks5slWrGgaJpZM4PhYJX .

Josephblt commented 6 years ago

I'll be home in about an hour. To try and fix this.

Em 23 de set de 2017 17:37, "Wagner Scholl Lemos" josephblt@gmail.com escreveu:

Hey Roger. The points collection is cloned inside the correctBreakDistance method to avoid changing Control Points position.

Em 23 de set de 2017 17:32, "Roger Martínez" notifications@github.com escreveu:

Yes @Josephblt https://github.com/josephblt , you're in facr changing the CPs of the line, which can cause trouble at some points... not sure if you're around, anyway don't worry lemme try to fix this and create a PR for it now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ahome-it/lienzo-core/pull/194#issuecomment-331668135, or mute the thread https://github.com/notifications/unsubscribe-auth/AYMakg7sBjgrvtkoBKYfeMAVNO2m06qZks5slWrGgaJpZM4PhYJX .

romartin commented 6 years ago

ohhh @Josephblt sorry, yes you're doing it inside the method. In fact you're cloning the points twice at this point, you can just do it once at the begging of the parse method... but for me that's fine for now... we urgently need this fix in and a release! So please just provide a fix for this once possible next week and we can introduce it on next release, if you agree guys!

@SprocketNYC yeah for me all good!, can be merged if you agree too! @SprocketNYC BTW please we need a release with last fixes... just preparing a summary mail, will send you in a bit. Really thanks!

Thanks guys!!