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

Fix CefNet.Avalonia Compat #5763

Closed
wants to merge 6 commits into from
Closed

Conversation

AigioL
Copy link

@AigioL AigioL commented Apr 12, 2021

What does the pull request do?

ca408e5 Breaking the compatibility
CefNet.Avalonia required cursor handle.
https://github.com/CefNet/CefNet/blob/master/CefNet.Avalonia/Internal/CursorInteropHelper.cs#L18

What is the current behavior?

System.TypeInitializationException
  HResult=0x80131534
  Message=The type initializer for 'CefNet.Internal.CursorInteropHelper' threw an exception.
  Source=CefNet.Avalonia
  StackTrace:
   在 CefNet.Internal.CursorInteropHelper.Create(IntPtr cursorHandle)
   在 CefNet.Internal.AvaloniaWebViewGlue.OnCursorChange(CefBrowser browser, IntPtr cursorHandle, CefCursorType type, CefCursorInfo customCursorInfo)
   在 CefNet.Internal.CefDisplayHandlerGlue.OnCursorChange(CefBrowser browser, IntPtr cursor, CefCursorType type, CefCursorInfo customCursorInfo)
   在 CefNet.CefDisplayHandler.OnCursorChangeImpl(cef_display_handler_t* self, cef_browser_t* browser, IntPtr cursor, CefCursorType type, cef_cursor_info_t* custom_cursor_info)

内部异常 1:
MissingMethodException: Method not found: 'Avalonia.Platform.IPlatformHandle Avalonia.Input.Cursor.get_PlatformCursor()'.

@dnfadmin
Copy link

dnfadmin commented Apr 12, 2021

CLA assistant check
All CLA requirements met.

@AigioL AigioL closed this Apr 13, 2021
@AigioL AigioL reopened this Apr 13, 2021
@maxkatz6 maxkatz6 requested a review from grokys April 13, 2021 03:24
@grokys
Copy link
Member

grokys commented Apr 13, 2021

@kekekeks are we OK exposing IntPtr handles in *Impl interfaces or should IPlatformHandle be used?

@grokys grokys requested a review from kekekeks April 13, 2021 09:09
@Gillibald
Copy link
Contributor

Gillibald commented Apr 13, 2021

Looks like only the Win32 platform uses a handle and the rest sets the property to IntPtr.Zero so this change is wrong and redundant Handle properties should be removed.

On Win32 just cast to IPlatformHandle: https://github.com/AvaloniaUI/Avalonia/blob/master/src/Windows/Avalonia.Win32/CursorFactory.cs#L170

@AigioL
Copy link
Author

AigioL commented Apr 13, 2021

看起来只有 Win32 平台使用句柄, 其余的设置属性, 所以这种变化是错误的, 多余的属性应该删除。IntPtr.Zero``Handle

在Win32刚刚投到:https://github.com/AvaloniaUI/Avalonia/blob/master/src/Windows/Avalonia.Win32/CursorFactory.cs#L170IPlatformHandle

It also seems to have influenced macOS and Linux

https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.X11/X11CursorFactory.cs#L72
https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.X11/X11CursorFactory.cs#L141
https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.X11/X11CursorFactory.cs#L88
https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.X11/X11CursorFactory.cs#L77

Mobile platforms may not need
Maybe should let the desktop implementation classes inherit IPlatformHandle
Or set up two interfaces to distinguish between mobile platform (IMobileCursorImpl) and desktop platform (IDesktopCursorImpl)

@kekekeks
Copy link
Member

kekekeks commented Apr 13, 2021

All platform handles should be exposed as IPlatformHandle. In some cases platform handles might be non-representable as a plain IntPtr without additional info. For Win32 we can use PlatformHandle class with HCURSOR as description value. For X11 it should be XCursor.

@AigioL AigioL changed the title Added ICursorImpl.Handle Fix Compat Fix CefNet.Avalonia Compat Apr 15, 2021
@AigioL AigioL closed this Apr 17, 2021
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.

5 participants