-
Notifications
You must be signed in to change notification settings - Fork 690
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
Skia shapes hittesting #16201
base: master
Are you sure you want to change the base?
Skia shapes hittesting #16201
Conversation
this builds on top of #15875 |
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16201/index.html |
internal SKPath? LastDrawnStrokePath { get; private set; } | ||
internal SKPath? LastDrawnFillPath { get; private set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if there is an unmanaged memory leak here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I noticed that too. According to the documentation, the unmanaged memory will be freed in the managed object's destructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Especially this they are no longer disposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not disposing and relying on the GC may have performance impacts, I think. Finalizers aren't cheap, and an explicit Dispose call should be calling GC.SuppressFinalize
(inside SkiaSharp)
What errors did you get when disposing?
|
||
stroke.UpdatePaint(fillPaint, strokeGeometry.Bounds); | ||
|
||
LastDrawnStrokePath = strokeGeometry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we dispose the previous one here?
internal SKPath? LastDrawnStrokePath { get; private set; } | ||
internal SKPath? LastDrawnFillPath { get; private set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Especially this they are no longer disposed.
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16201/index.html |
f9c77d1
to
0219d0c
Compare
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-16201/index.html |
🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-16201/index.html |
GitHub Issue (If applicable): closes #15855
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Run
results.Other information
Internal Issue (If applicable):