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 parsing pathgroup dimensions from inner paths #2348

Merged
merged 1 commit into from
Jul 17, 2015
Merged

Fix parsing pathgroup dimensions from inner paths #2348

merged 1 commit into from
Jul 17, 2015

Conversation

asturur
Copy link
Member

@asturur asturur commented Jul 17, 2015

now:
image

we cannot get the shape dimensions, declared as 100% we are not able to parse the dimensions correctly.

after fix ( i colored white space to show that the padding is actually a defined shape )
image

@kangax
Copy link
Member

kangax commented Jul 17, 2015

Nice. Do we have that shape in tests?

@asturur
Copy link
Member Author

asturur commented Jul 17, 2015

no but we can do a simple test if i found a pathgroup loading test.

@asturur
Copy link
Member Author

asturur commented Jul 17, 2015

I want to clarify something.
PathGroup is essentially a structure made to contain SVGs.
Or at least now is that.

Path inside pathgroup are not supposed and not supported to have own scaleX, scaleY, angle. Only transformMatrix that normally comes from parsing SVG transform(s) property.

I dream a version in wich group and pathgroup become one class and svg are handled as a normal group and can be degrouped on the fly. But we are still far away.

@asturur
Copy link
Member Author

asturur commented Jul 17, 2015

@kangax added tests for the parsing dimension and parsing dimensions with transform Matrix. You can merge.

kangax added a commit that referenced this pull request Jul 17, 2015
Fix parsing pathgroup dimensions from inner paths
@kangax kangax merged commit 1499d34 into fabricjs:master Jul 17, 2015
@asturur asturur deleted the fix-pathgroup-parsing branch July 20, 2015 22:43
This pull request was closed.
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.

2 participants