praeclarum / NGraphics

NGraphics is a cross platform library for rendering vector graphics on .NET. It provides a unified API for both immediate and retained mode graphics using high quality native renderers.
MIT License
706 stars 133 forks source link

Fix for the SVG SubPath Issue. #62

Closed Emasoft closed 7 years ago

Emasoft commented 7 years ago

Fix for the SVG SubPath Issue. Now subpaths are rendered correctly.

Emasoft commented 7 years ago

Subpath BEFORE (bug): subpath bug before fix

Subpath AFTER (fixed) subpath bug fixed after fix

The above SVG file for testing: assignement_icon.svg.zip

BlueRaja commented 7 years ago

Diff without whitespace changes: https://github.com/praeclarum/NGraphics/pull/62/files?w=1

Emasoft commented 7 years ago

@praeclarum Frank can you merge this, please? I need to share a project on GitHub that uses those fixes and I don't want to create a fork with the fix and a separate Nuget.

praeclarum commented 7 years ago

I don't like accepting changes with all these whitespace breaks - ruins history and blame.

I'm sorry, I really want these fixes but this PR is too messy.

BlueRaja commented 7 years ago

To be fair, the official Microsoft C# coding guidelines, followed by the vast majority of C# developers, recommends using 4-space indents.

@praeclarum is right that it doesn't make sense to update the whitespace along with other changes, though. Or to update it in some files and not others.

praeclarum commented 7 years ago

Contribution Etiquette

Anyway, these are good changes. If @Emasoft doesn't want to fix this PR then I can use the diffs to hand merge this puppy. I just don't like that @Emasoft would lose the credit.

Emasoft commented 7 years ago

I would love to fix it, but I don't know how. I just loaded the project in Xamarin Studio, and it uses the Visual Studio code formatting settings by default:

schermata 2016-11-30 alle 20 26 25 schermata 2016-11-30 alle 20 25 48

Is there a unix/osx shell script I can use to revert the .cs file to your formatting preferences?

What formatting standard are you using? Is there a specific formatting policy file to use for this project? A mandatory policy file to use would be great for avoiding messing up the formatting.

If you use a specific formatting policy, you can export them from Xamarin Studio -> Policies.. -> Export , like this:

schermata 2016-11-30 alle 20 35 35

If you export it, I can load it in Xamarin Studio and apply it to the source code. Or you can do this yourself as an additional commit of this pull request. If it is impossible to do, then hand merge it. I'm not happy to lose the credit but at least it will be merged. Thank you!

praeclarum commented 7 years ago

The problem isn't that your formatting is wrong, it's that you reformatted the whole file. Simply don't do that.

You must have a setting somewhere that reformats the whole document on change. Just turn that off or don't hit that command.

I am not using a formatting policy. Please just look at the file and match what is already being done.

Emasoft commented 7 years ago

I have the option "Xamarin Studio -> Preferences -> Behaviour -> Format document on save.. " on, because I like my code to be always formatted. I will switch this off in the future. But for now is there a way to fix this applying a new formatting scheme like the one you use?

praeclarum commented 7 years ago

Yeah, sorry this project should have a policy in the solution so you can have your auto format be safe. I'll look into this...

I'm afraid the only way to fix this is to rebase from master and apply the changes by hand again.

Sorry for trouble. I do appreciate the work. If you're unwilling to do it, I can merge myself.

BlueRaja commented 7 years ago

@praeclarum I don't disagree with you, I was just playing Devil's Advocate