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

Quadratic path support #157

Closed
wants to merge 3 commits into from

Conversation

rFlex
Copy link

@rFlex rFlex commented Sep 6, 2017

Support for Q path. Unclear whether q will properly work with this change, I don't have an example that uses it unfortunately :(.

@kayuri
Copy link
Collaborator

kayuri commented Sep 8, 2017

Nope, doesn't work even on simple examples:

<?xml version="1.0"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
         "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink"
     height="800">
 <g>
<path d="M 10 35 q 15 -30 30 0" stroke="blue"
  stroke-width="5" fill="none" />
</g>
</svg>

I will try to have a closer look but seems like the TODO in code needs to be addressed :)

@rFlex
Copy link
Author

rFlex commented Sep 8, 2017

@kayuri I just updated the example and SVGView to allow setting an external url (easier to test local files from the example and it's a useful feature anyway). Beside the incorrect inferred bounds, it seems to work properly on your example. I can look at why the bounds are incorrect in the first place, unclear whether this is related to my change though.

screen shot 2017-09-08 at 11 56 31 am

@rFlex
Copy link
Author

rFlex commented Sep 8, 2017

@kayuri Example with bounding box working. In the latest commit I'm using CGPath boundingBox instead of the custom calculation code. You may have good reasons why you made that code in the first place, let me know if you want me to revert that change.

screen shot 2017-09-08 at 12 04 47 pm

@kayuri
Copy link
Collaborator

kayuri commented Sep 8, 2017

@rFlex yep, I got similar views, though that bounds issue was caused by the the change though. Will give it some more thought. Thanks!

@rFlex
Copy link
Author

rFlex commented Sep 8, 2017

@kayuri yes it's because it was missing the implementation to return the bounds from a Q/q Path, which causes problems when trying to scale the view.

@ystrot
Copy link
Member

ystrot commented Nov 28, 2017

Hi Simon,

Generally your patch looks good. Could you please update it to the latest version and avoid merge conflicts?

@ystrot
Copy link
Member

ystrot commented Dec 28, 2017

Hi Simon,

Unfortunately we were unable to apply your patch, because it was outdated. However we implemented quadratic segments (including q/Q/t/T types) and it's already in the master and will be part of 0.9.2 release.

@ystrot ystrot closed this Dec 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants