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 build of generated C++ code when using transitions on int properties with esp-idf #4822

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

tronical
Copy link
Member

The energy monitor declares a transition on an animated int property, for which Property::set_animated_binding_for_transition is called, which in turn calls slint_property_set_animated_binding_helper. The latter is overloaded for various property types, such as float, Color, or Brush, and then calls specialized functions from ffi, such as

slint_property_set_animated_binding_(int|float|etc.).

slint_property_set_animated_binding_int uses i32 in Rust, which cbindgen maps to int32_t, so the
slint_property_set_animated_binding_helper overload also uses int32_t.

Unfortunately, with esp-idf, int32_t is a distinct type from int, and the overload resolution fails.

As remedy, this change uses c_int instead of i32 in the Rust ffi, which maps to int.

This seems easier than changing Property to Property<int32_t> :-)

…ies with esp-idf

The energy monitor declares a transition on an animated int property,
for which Property<int>::set_animated_binding_for_transition is called,
which in turn calls slint_property_set_animated_binding_helper. The
latter is overloaded for various property types, such as float, Color,
or Brush, and then calls specialized functions from ffi, such as

slint_property_set_animated_binding_(int|float|etc.).

slint_property_set_animated_binding_int uses i32 in Rust, which cbindgen
maps to int32_t, so the
slint_property_set_animated_binding_helper overload also uses int32_t.

Unfortunately, with esp-idf, int32_t is a distinct type from int, and
the overload resolution fails.

As remedy, this change uses c_int instead of i32 in the Rust ffi, which
maps to int.

This seems easier than changing Property<int> to Property<int32_t> :-)
@tronical tronical requested a review from ogoffart March 12, 2024 11:44
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there going to be a test?
Would it be possible to run the test-driver-cpp on such platform?

@tronical
Copy link
Member Author

Is there going to be a test?

Best I can think of right now is implicit - getting the energy monitor for the esp32p4. (it renders one frame, fwiw)

Would it be possible to run the test-driver-cpp on such platform?

Perhaps a compile-only test would be possible, but linking may require going all the way idf.py. Not sure it's worth it - in this case riscv32-esp-elf-g++ is gcc and this is a bit of an edge case.

@tronical tronical merged commit b1f408f into master Mar 12, 2024
36 checks passed
@tronical tronical deleted the simon/esp-idf-int branch March 12, 2024 19:25
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.

2 participants