search

LaserWeb / LaserWeb4

Collaborative effort on the next version of LaserWeb / CNCWeb
GNU Affero General Public License v3.0
713 stars 192 forks source link

Reenable viewBox coordinate transformation in the SVG parser #487

Closed florianfesti closed 6 years ago

florianfesti commented 6 years ago

I get users complaining that the SVGs I generate with Boxes.py (https://github.com/florianfesti/boxes) cannot be rendered with Laserweb. A quick look raised the suspicion that the viewBox calculation may be off. Well, ViewBox transformation was commented out.

Turns out the commented out code was actually wrong as it did scale the coordinates first and then move the starting corner to the origin. For this to work properly the starting corner needs to be move to the origin first and coordinates must only scaled after that to ensure the starting corner actually ends up on the origin.

jorgerobles commented 6 years ago

Hi Florian

Viewbox was disabled on the parser but recomputed on other part of the codebase, isn't it @tbfleming?

I use your generators often, but the issues i experience are related with open paths rather than viewbox.

El vie., 1 jun. 2018 17:18, Florian Festi notifications@github.com escribió:

I get users complaining that the SVGs I generate with Boxes.py ( https://github.com/florianfesti/boxes) cannot be rendered with Laserweb. A quick look raised the suspicion that the viewBox calculation may be off. Well, ViewBox transformation was commented out.

Turns out the commented out code was actually wrong as it did scale the coordinates first and then move the starting corner to the origin. For this to work properly the starting corner needs to be move to the origin first and coordinates must only scaled after that to ensure the starting corner actually ends up on the origin.

You can view, comment on, or merge this pull request online at:

https://github.com/LaserWeb/LaserWeb4/pull/487 Commit Summary

  • Reenable viewBox coordinate transformation in the SVG parser

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/LaserWeb4/pull/487, or mute the thread https://github.com/notifications/unsubscribe-auth/ABoIYC6K7O1JGJxgO74CsFYvJgCzxf0eks5t4VtEgaJpZM4UW4GR .

tbfleming commented 6 years ago

@jorgerobles yep. Plus it works with various viewbox transforms coming out of Inkscape.

florianfesti commented 6 years ago

OK, so there is another issue at play here. The two lines are still in the wrong order and should be swapped even if they remain commented out. I filed an issue with lw.svg-parser (actually even before this PR) which actually uses the code and mishandles the viewBox.

florianfesti commented 6 years ago

I am just closing this PR here as this is not going to solve the actual issue at hand.

The question what to do with open paths remains. Restrictions in the cairo lib used by Boxes.py make it unlikely that the problem is going away soon. Also I don't really see why paths need to be closed for cutting. New features like tabs will rather produce open paths on purpose.

jorgerobles commented 6 years ago

hi Florian,

There's no need of closed paths in LW, but we have found also issues with clipperjs in our codebase in certain conditions (not very common)

I usually end importing your svgs on AI, releasing compound paths and making join of coincident lines (making them closed polygons). Upon importing on LW, they run ok.

I've got press design background, and learnt a while ago that vector data needs to be tweaked in >80% of the cases, no matter what source nor destination (lw or other), so vector edition (freehand, ai...) Apps are my best friends.

Anyway, thanks for your effort.

El vie., 1 jun. 2018 18:19, Florian Festi notifications@github.com escribió:

I am just closing this PR here as this is not going to solve the actual issue at hand.

The question what to do with open paths remains. Restrictions in the cairo lib used by Boxes.py make it unlikely that the problem is going away soon. Also I don't really see why paths need to be closed for cutting. New features like tabs will rather produce open paths on purpose.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/LaserWeb/LaserWeb4/pull/487#issuecomment-393931363, or mute the thread https://github.com/notifications/unsubscribe-auth/ABoIYHqNAPOEuJL8XZPYDE1yrCIMMsIGks5t4Wl0gaJpZM4UW4GR .