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 error in function change_transmat #34

Merged
merged 1 commit into from
May 20, 2021

Conversation

cajfisher
Copy link
Contributor

Matrix entry update function "change_transmat" should only change one value at a time, instead of all at once, otherwise the requested matrix data will be overwritten after the first value with values already in the boxes.

Matrix entry update function "change_transmat" should only change one value at a time, instead of all at once, otherwise the requested matrix data will be overwritten after the first value with values already in the boxes.
@ovhpa
Copy link
Collaborator

ovhpa commented May 20, 2021

Hello, and thank you for your suggestion.

Have you see the pull request #21 ?
I wonder if the code with your modification is properly optimizing with clang?
There use to be some trouble is using a pointer to represent an interger.

Maybe we should implement the suggestion in #21

Sincerely,

@cajfisher
Copy link
Contributor Author

cajfisher commented May 20, 2021 via email

@ovhpa
Copy link
Collaborator

ovhpa commented May 20, 2021

Hello,

Please give me a moment to test the change with clang and I will merge the PR ;)
clang was giving this problem both on Mac and Linux BTW.
It was quite a challenge to find the culprit at the time, this is why I am cautious ;)

Best,

@ovhpa
Copy link
Collaborator

ovhpa commented May 20, 2021

Hello again and sorry for the wait...

It seems that the latest version of clang does not complain with your change.
Oddly enough, it also no longer complains with the original version of change_transmat before PR #21.
I'm gonna merge your PR, but I hope some version / machine combination will not break again (on ARM, clang already complained that gpointer aka void * is taken from smaller integer type gint aka int).

Maybe when I have the time I will implement the small array solution, which does not cast an integer from a pointer.

Anyway, thank you for pointing that out, and thank you for your contribution.

Best,


for future references:
https://developer.gnome.org/glib/stable/glib-Type-Conversion-Macros.html#GINT-TO-POINTER:CAPS
#define GINT_TO_POINTER(i) ((gpointer) (glong) (i))

Original call to change_transmat function:
GTK_SIGNAL_FUNC(change_transmat), (gpointer) i);
Now:
GTK_SIGNAL_FUNC(change_transmat), GINT_TO_POINTER(i));

@ovhpa ovhpa merged commit e5421a4 into arohl:master May 20, 2021
@cajfisher
Copy link
Contributor Author

cajfisher commented May 20, 2021 via email

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