graphene_rect_contains_point() is wrong about points above the rect

[flxzt]

Experienced behavior

Rect::contains_point(&self, p: &Point) returns true for points which are above the rectangle.

Expected behavior

It should return false when they are not inside it.

Steps to reproduce

I experienced this bug while using the graphene-rs rust bindings for graphene, so this is rust code. This is the code that can reproduce the issue:

use gtk4::graphene::{Point, Rect};

fn main() {
    let rect = Rect::new(100., 100., 100., 100.);

    let right = Point::new(250., 150.);
    let below = Point::new(150., 50.);
    let left = Point::new(50., 150.);
    let above = Point::new(150., 250.);

    assert!(!rect.contains_point(&right));
    assert!(!rect.contains_point(&below));
    assert!(!rect.contains_point(&left));
    assert!(!rect.contains_point(&above));
}

Backtrace:

thread 'main' panicked at 'assertion failed: !rect.contains_point(&above)', src/main.rs:14:5
stack backtrace:
   0:     0x55897d544380 - std::backtrace_rs::backtrace::libunwind::trace::ha5edb8ba5c6b7a6c
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
   1:     0x55897d544380 - std::backtrace_rs::backtrace::trace_unsynchronized::h0de86d320a827db2
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55897d544380 - std::sys_common::backtrace::_print_fmt::h97b9ad6f0a1380ff
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x55897d544380 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h14be7eb08f97fe80
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:46:22
   4:     0x55897d55e2cf - core::fmt::write::h2ca8877d3e0e52de
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/fmt/mod.rs:1094:17
   5:     0x55897d542e85 - std::io::Write::write_fmt::h64f5987220b618f4
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/io/mod.rs:1584:15
   6:     0x55897d545fcb - std::sys_common::backtrace::_print::h7f1a4097308f2e0a
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:49:5
   7:     0x55897d545fcb - std::sys_common::backtrace::print::h1f799fc2ca7f5035
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:36:9
   8:     0x55897d545fcb - std::panicking::default_hook::{{closure}}::hf38436e8a3ce1071
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:208:50
   9:     0x55897d545a9d - std::panicking::default_hook::he2f8f3fae11ed1dd
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:225:9
  10:     0x55897d5465dd - std::panicking::rust_panic_with_hook::h79a18548bd90c7d4
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:591:17
  11:     0x55897d546147 - std::panicking::begin_panic_handler::{{closure}}::h212a72cc08e25126
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:495:13
  12:     0x55897d54481c - std::sys_common::backtrace::__rust_end_short_backtrace::hbd6897dd42bc0fcd
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/sys_common/backtrace.rs:141:18
  13:     0x55897d5460d9 - rust_begin_unwind
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:493:5
  14:     0x55897d534d91 - core::panicking::panic_fmt::h77ecd04e9b1dd84d
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/panicking.rs:92:14
  15:     0x55897d534cdd - core::panicking::panic::h60569d8a39169222
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/panicking.rs:50:5
  16:     0x55897d5358f6 - graphenetest::main::hd472e6662594c6f2
                               at /run/media/felixz/Daten/source/rust/graphenetest/src/main.rs:14:5
  17:     0x55897d5355db - core::ops::function::FnOnce::call_once::h979fe99887ecba2d
                               at /home/felixz/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  18:     0x55897d535a3e - std::sys_common::backtrace::__rust_begin_short_backtrace::h0e90eea8e5c5bd6e
                               at /home/felixz/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  19:     0x55897d535641 - std::rt::lang_start::{{closure}}::hd476fe4938731590
                               at /home/felixz/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:49:18
  20:     0x55897d5469da - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc4354216bf39217c
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/ops/function.rs:259:13
  21:     0x55897d5469da - std::panicking::try::do_call::hb68eb312780385cf
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:379:40
  22:     0x55897d5469da - std::panicking::try::h22b8e08595060b8b
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:343:19
  23:     0x55897d5469da - std::panic::catch_unwind::hc64f1a6a0e71b1fc
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panic.rs:431:14
  24:     0x55897d5469da - std::rt::lang_start_internal::h4461fc58637f04f8
                               at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/rt.rs:34:21
  25:     0x55897d535620 - std::rt::lang_start::h2c890320d1935a21
                               at /home/felixz/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:48:5
  26:     0x55897d535a2c - main
  27:     0x7f92562d8b75 - <unknown>
  28:     0x55897d53548e - _start
  29:                0x0 - <unknown>

Operating system in use

Fedora 34, gtk4-rs on version 0.2

[ebassi]

There must be something wrong in the Rust bindings, as adding this:

 const graphene_point_t points[] = {
   GRAPHENE_POINT_INIT (250, 150),
   GRAPHENE_POINT_INIT (150, 50),
   GRAPHENE_POINT_INIT (50, 150),
   GRAPHENE_POINT_INIT (150, 250),
 };
 const unsigned n_points = sizeof (points) / sizeof (points[0]);
 graphene_rect_init (&r, 100, 100, 100, 100);
 for (unsigned i = 0; i < n_points; i++)
   {
     mutest_expect ("a rectangle to not contain a point outside its boundaries",
                    mutest_bool_value (graphene_rect_contains_point (&r, &(points[i]))),
                    mutest_to_be, false,
                    NULL);
   }

to the test suite still makes it pass.

[sdroege]

It's an ABI mismatch. The problem here is that graphene uses bool aka _Bool (1 byte here) instead of gboolean (int) for the return type.

        <return-value transfer-ownership="none">
          <doc xml:space="preserve">`true` if the rectangle contains the point</doc>
          <type name="gboolean" c:type="_Bool"/>
        </return-value>

Lists the type name as gboolean but also as C type _Bool.

How exactly is this supposed to be handled by bindings, and do other non-libgirepository bindings actually handle this at all?

[ebassi]

Okay, this is not really an issue in Graphene.

[sdroege]

Well, it's an issue in graphene in the sense that it generates wrong introspection data (and maybe should've used proper gboolean instead of C _Bool but that's too late now) but I guess the actual bug here would be in gobject-introspection

[ebassi]

Graphene is not a GLib library, so of course it can't use gboolean. :-)

I guess gobject-introspection should treat _Bool as a native type instead of coercing it into a gboolean, but then again: _Bool has no defined size and no GType representation, so there's a ton more things that need to be done there.

[sdroege]

Graphene is not a GLib library, so of course it can't use gboolean. :-)

Oh indeed, completely forgot about that :D Sorry