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

Support for dashed lines #47

Closed
Rungee opened this issue Mar 18, 2016 · 16 comments
Closed

Support for dashed lines #47

Rungee opened this issue Mar 18, 2016 · 16 comments
Assignees
Milestone

Comments

@Rungee
Copy link

Rungee commented Mar 18, 2016

No description provided.

@bholmes
Copy link
Contributor

bholmes commented Mar 22, 2016

I quickly found that this patch is more involved than adding a missing property or method. I have created a branch to discuss the best way to integrate PathEffects.

Ensuring that we wrap the return of the getter of the PathEffect property with the correct managed object is the point of discussion.

@petergolde
Copy link
Contributor

I don't think, in this particular case, that exposing subclasses of SkPathEffect serves any significant purpose, because the subclasses in the C++ API don't expose additional public members. I would just expose the single sealed class SKPathEffect, along with static Create function. E.g.:

static SKPathEffect CreateDashed(float[] intervals, float phase)

@petergolde
Copy link
Contributor

Obviously the other PathEffects would have their own CreateXXX function.

@hypeartist
Copy link

Could anyone, please, make a windows binary (dll) of SkiaSharp with SKPathEffect added?

@migueldeicaza
Copy link
Contributor

I agree with Peter's take. We do not need to bind the subclasses (and in fact, I am wondering if we need to surface all of those SKStream subclasses in the first place as well).

We should just provide convenient factory methods to create these.

@bholmes
Copy link
Contributor

bholmes commented Apr 7, 2016

@migueldeicaza First I agree that we should do this. I need to check the remaining path effect classes to see if there are any critical members that will be missing.

There are some members on these effects, but they feel advanced to me. Also I do not think creating factory methods would stop us from later surfacing these if needed.

I see two similar options. One add the factory methods to the paint object in C# which will handle creating the effect and assigning in C. The other option is to create a C# class (SkPathEffect) that only has static factory methods that take the paint as an argument to assign the new effect to. Again in C.

In both options I think we need a way to clear the effect from the paint. That could be as simple as a ResetEffect method on the paint class.

Thoughts?

@migueldeicaza
Copy link
Contributor

I like it.

@petergolde
Copy link
Contributor

I would have a read/write property on SKPaint of type SKPathEffect.

SKPathEffect would have only static creation methods, e.g.:

public static SKPathEffect CreateDashedPathEffect(...);

I think this is

  1. most like the skia C++ api
  2. discoverable
  3. doesn't overly pollute the SKPaint members
  4. could extend later if we allow user-written subclasses of SKPathEffect (I don't think we should be go here now though)

Peter

On Apr 7, 2016, at 2:12 PM, Bill Holmes notifications@github.com wrote:

@migueldeicaza First I agree that we should do this. I need to check the remaining path effect classes to see if there are any critical members that will be missing.

There are some members on these effects, but they feel advanced to me. Also I do not think creating factory methods would stop us from later surfacing these if needed.

I see two similar options. One add the factory methods to the paint object in C# which will handle creating the effect and assigning in C. The other option is to create a C# class (SkPathEffect) that only has static factory methods that take the paint as an argument to assign the new effect to. Again in C.

In both options I think we need a way to clear the effect from the paint. That could be as simple as a ResetEffect method on the paint class.

Thoughts?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

@hypeartist
Copy link

Is anything of SKPathEffect implemented in 1.49.2-beta?

@bholmes
Copy link
Contributor

bholmes commented Apr 12, 2016

@hypeartist no there is not

@derrickp
Copy link

derrickp commented May 4, 2016

Any idea when this might be implemented? Is it a fairly low priority thing?

@gentledepp
Copy link
Contributor

Is there any chance this will be implemented soon? At least in a branch so I can make a private nuget package? I would code it myself if I had any clue how to do so... :-\

@migueldeicaza
Copy link
Contributor

Mattew is on vacation this week, and we were hoping to focus on merging our changes with upstream Google after that. So shortly after I would hope we could work on this.

@mattleibow
Copy link
Contributor

mattleibow commented Jul 13, 2016

The C code for this is in commit mono/skia@e31d35b
DIFF: mono/skia@xamarin-mobile-bindings...mono:feature-patheffect

C#: #112

@mattleibow
Copy link
Contributor

This will be in the next release. It is currently merged into master.

@Rungee
Copy link
Author

Rungee commented Jul 29, 2016

Thanks a lot for all your efforts.

29 Июл 2016 г. 20:31 пользователь "Matthew Leibowitz" <
notifications@github.com> написал:

This will be in the next release. It is currently merged into master.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#47 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABlXL1r0vqzYPXKIOr2y_E9--gAath0pks5qakb_gaJpZM4Hz1wZ
.

@mattleibow mattleibow added this to the 1.53.0 milestone Aug 11, 2016
@mattleibow mattleibow added this to To Be Classified in Previous Releases Jul 26, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Aug 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Previous Releases
To Be Classified
Development

No branches or pull requests

8 participants