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

Signals, Cairo, Events + General Cleanup #323

Merged
merged 43 commits into from
Aug 9, 2021
Merged

Signals, Cairo, Events + General Cleanup #323

merged 43 commits into from
Aug 9, 2021

Conversation

mjakeman
Copy link
Member

@mjakeman mjakeman commented May 19, 2021

This restores our old support for signals now that delegates work as expected.

This is technically done, but needs some further testing.

It also includes:

  • Basic implementation of Cairo
  • Support for GdkEvent
  • A complex sample demonstrating custom drawing code
  • Fixes to union and bitfield generation

TODO (before merge):

  • Value.Extract must work for arbitrary records
  • Fix bitfield size (long vs uint vs int?)
  • Fix issue where DrawingArea occasionally stops drawing on low memory (GC?)
  • Handle unions (e.g. GdkEvent / GdkEventKey)
    • Special case Gdk.Event / Gst.MiniObject / etc (MiniObject is for another PR)
    • (funnily enough, Gst.MiniObject is the example of what not to do when writing bindable APIs)
  • We need a solution for cross-platform cairo (handwritten bindings)
  • Tests (Next PR: Unit Test all of GLib/GObject)
  • Port a subset of text editor as a sample project
    • Either fix deletion or remove it from the text editor demo (currently poorly implemented, prone to crashing)

Delegates work now (yay!) so let's use the
closure marshal directly. This simplifies the
code quite a bit, and gets us closer to working
signals.
@mjakeman mjakeman changed the base branch from develop to wip/mjake/delegates May 19, 2021 10:43
Base automatically changed from wip/mjake/delegates to develop May 20, 2021 13:51
@badcel
Copy link
Member

badcel commented May 23, 2021

We need to check the documentation generation as the old union generation behavior is restored which could break the documentation again.

@mjakeman mjakeman changed the title Draft: Signals Signals and Other Cleanup Jun 7, 2021
mjakeman and others added 9 commits June 7, 2021 16:58
 - Avoid sizing errors by ensuring enums are always
   int-based (32-bits)
 - Determine whether to use a signed or unsigned int
   depending on the presence of a negative value
 - Perhaps clarify this with upstream? Seems to work
   reliably for now.
 - Adds a (mostly) functional text editor demo that
   reimplements the basic feature set of GtkTextView
   using a piece table.
 - Demonstrates custom drawing with cairo and event
   handling using GDK/Signals.
 - Adds a (mostly) functional text editor demo that
   reimplements the basic feature set of GtkTextView
   using a piece table.
 - Demonstrates custom drawing with cairo and event
   handling using GDK/Signals.
@mjakeman
Copy link
Member Author

mjakeman commented Jul 15, 2021

Had this error when doing key input after the drawing area stopped drawing:

(dotnet.exe:17948): GLib-GObject-CRITICAL **: 18:25:20.068: g_object_remove_toggle_ref: assertion 'G_IS_OBJECT (object)' failed

So maybe it isn't signal related and in fact a sign some underlying object is being freed.

Edit: Closure is being freed too:

(dotnet.exe:16444): GLib-GObject-CRITICAL **: 18:32:16.389: g_closure_unref: assertion 'closure->ref_count > 0' failed

Edit 2: Another error:

(dotnet.exe:5284): GLib-GObject-CRITICAL **: 18:56:59.389: g_object_remove_toggle_ref: assertion 'G_IS_OBJECT (object)' failed
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Repeat 2 times:
--------------------------------
   at GObject.Native.Object+Instance+Methods.RemoveToggleRef(IntPtr, GObject.Native.ToggleNotifyCallback, IntPtr)
--------------------------------
   at GObject.Native.ObjectMapper+ToggleRef.Dispose()
   at GObject.Native.ObjectMapper.Unmap(IntPtr)
   at GObject.Native.ObjectHandle.ReleaseHandle()
   at System.Runtime.InteropServices.SafeHandle.InternalRelease(Boolean)
   at System.Runtime.InteropServices.SafeHandle.Dispose(Boolean)
   at System.Runtime.InteropServices.SafeHandle.Finalize()

Edit 3: Some more findings, this is very bizarre.

I'm getting an output of "<NULL-class>" when checking the TypeNameFromInstance of the object that is having its toggle ref removed.

From glib's gtype.c:

const gchar *
g_type_name_from_instance (GTypeInstance *instance)
{
  if (!instance)
    return "<NULL-instance>";
  else
    return g_type_name_from_class (instance->g_class);
}

const gchar *
g_type_name_from_class (GTypeClass *g_class)
{
  if (!g_class)
    return "<NULL-class>";
  else
    return g_type_name (g_class->g_type);
}

This means the type class (g_class) is somehow NULL. Which means either the object is half-finalized or very badly corrupted.

Edit 4: The rest of the fields are all NULL as well. Looks like a simple case of use-after-free. Probably means the ToggleRef implementation is broken.

@mjakeman mjakeman changed the title Signals and Other Cleanup Signals, Cairo, Events + General Cleanup Jul 15, 2021
badcel added 3 commits August 7, 2021 11:40
- Remove unused projects
- Remove launchSettings.json
It can not be avoided completely, du to data structures of GDK.Event. Hopefully the workaround gets obsolete once we support GDK4.
@badcel badcel linked an issue Aug 8, 2021 that may be closed by this pull request
badcel added 2 commits August 8, 2021 17:47
`gtk_im_context_simple_new` returns an owned reference according to the documentation. This information must be given to the object instantiation process to avoid leaking memory.

See: https://docs.gtk.org/gtk3/ctor.IMContextSimple.new.html
@badcel badcel force-pushed the wip/mjake/signals branch from bfdf1af to 9aee619 Compare August 8, 2021 16:57
This adds some basic tests using GdkPixbuf which allows to run them as integration tests. Integration tests can be automatically run during continous integration on the server.

- Add test for basic property reading
- Add test for object disposal by garbage collector
- Add test for object disposal by "Dispose" method.
@badcel badcel force-pushed the wip/mjake/signals branch from 9aee619 to 8775b87 Compare August 8, 2021 16:58
- Add Quickstart
- Add DrawingArea
- Add TextEditor
@badcel badcel mentioned this pull request Aug 8, 2021
14 tasks
@badcel badcel linked an issue Aug 8, 2021 that may be closed by this pull request
14 tasks
@badcel badcel merged commit 897d945 into develop Aug 9, 2021
@badcel badcel deleted the wip/mjake/signals branch August 9, 2021 19:48
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.

Clean-up VS Solution / Delete Legacy Projects Closures Handling
2 participants