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

Enable checking if PatternUnits or PatternContentUnits was actually set #669

Conversation

wieslawsoltes
Copy link
Contributor

Reference Issue

The #668 actually broke Svg.Skia pattern rendering, did not see this as breaking at first. In order to correctly render pattern, pattern property inheritance must be used to check for inherited values. As #668 removed invalid inherited enum value it broke Svg.Skia as it does not have access to internal fields and those are necessary to provide information if actually PatternUnits or PatternContentUnits was set.

What does this implement/fix? Explain your changes.

Adds HasPatternUnits() and HasPatternContentUnits() methods to check if field has actually a value.

Any other comments?

@wieslawsoltes
Copy link
Contributor Author

@wieslawsoltes
Copy link
Contributor Author

Hiding such internals neccessary for proper rendering will make #590 much harder to achieve.

@H1Gdev
Copy link
Contributor

H1Gdev commented Feb 11, 2020

I think this PR solution is ad hocery.
Inherit was returned...

https://github.com/vvvv/SVG/blob/3efa867fb69ee2c68a59f5c1e8b92c988e94e1c7/Source/Painting/SvgPatternServer.cs#L155-L183

I think we should think of a way to return linked value in properties.

@wieslawsoltes
Copy link
Contributor Author

I think we should think of a way to return linked value in properties.

Agree with that, returning correct linked value in properties is important and necessary in other place too.

@mrbean-bremen mrbean-bremen merged commit df64dd3 into svg-net:master Feb 16, 2020
@H1Gdev
Copy link
Contributor

H1Gdev commented Feb 16, 2020

@wieslawsoltes
@mrbean-bremen

Temporary methods that have been added in this PR do not know when to be deleted.
So I think it should be stated in comments etc.

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.

3 participants