-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Code Quality: Removed Vanara from ShellPreviewViewModel #16945
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
Code Quality: Removed Vanara from ShellPreviewViewModel #16945
Conversation
b7f4136 to
449f1dc
Compare
|
CC @hez2010 Ready, confirmed to be working well. |
src/Files.App/ViewModels/UserControls/Previews/ShellPreviewViewModel.cs
Outdated
Show resolved
Hide resolved
|
I'm OOF this week. Will take a look later. |
30cea65 to
decf3da
Compare
hez2010
left a comment
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.
Overall LGTM. Left some comments.
src/Files.App/ViewModels/UserControls/Previews/ShellPreviewViewModel.cs
Outdated
Show resolved
Hide resolved
| _windProc = new(WndProc); | ||
| var pWindProc = Marshal.GetFunctionPointerForDelegate(_windProc); | ||
| var pfnWndProc = (delegate* unmanaged[Stdcall]<HWND, uint, WPARAM, LPARAM, LRESULT>)pWindProc; |
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.
I would like to avoid using GetFunctionPointerForDelegate. Instead, marking WndProc as static and UnmanagedCallersOnly and taking a function pointer from it directly.
While we don't need to do it immediately in this PR as it may require some refactor to WndProc.
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. Since making proc-s static has huge impact like you said, i was hesitant to do this (while I understand using GetFunctionPointerForDelegate has performance impact likewise)
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.
Im curious, if you were to, how would you refactor (roughly)?
|
@0x5bfa can you please resolve the conflicts? |
…wModel.cs Co-authored-by: Steve <[email protected]> Signed-off-by: 0x5BFA <[email protected]>
0ca9b55 to
141bc82
Compare
Resolved / Related Issues
Steps used to test these changes