Skip to content

[gui] fix context menu for member with multiple toggles choise - #23782

Merged
linev merged 4 commits into
root-project:masterfrom
linev:fix_toggle
Oct 26, 2026
Merged

linev merged 4 commits into
root-project:masterfrom
linev:fix_toggle

Conversation

@linev

@linev linev commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Like fImageQuality of the TASImage - see for instance galaxy_image.C tutorial.
ROOT crashes when trying to select not the last value in the submenu.

contextmenu

Problem that TToggle object rememeber pointers on setter and getter which
then used to select one of the values. But such setter/getter are transient objects
which are deleted by next call to TDataMember::GetterMethod() / TDataMember::SetterMethod()

Therefore in TTogle extract getter and setter callers right before usage. Also cleanup TToggle code to avoid plain string usage and simplify
several methods.

Also fix TDataMember::SetterMethod() - when as method argument enum is used.
In such case one has to cast argument to actual data type - otherwise TMethodCall is not working.

Cleanup cde in TRootCanvas and TQt6Canvas where correspondent TToggle instances are created

@linev linev self-assigned this Oct 21, 2026
@linev linev added in:Core Libraries in:GUI clean build Ask CI to do non-incremental build on PR labels Oct 21, 2026

@bellenot bellenot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I confirm it works, for what concerns the GUI part. For what concerns meta, I'll let the other reviewers decide

Comment thread core/gui/src/TToggle.cxx Outdated
Comment thread core/gui/src/TToggle.cxx Outdated
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Test Results

    24 files      24 suites   4d 4h 7m 27s ⏱️
 3 893 tests  3 892 ✅ 0 💤 1 ❌
83 373 runs  83 372 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 8a4746c.

♻️ This comment has been updated with latest results.

Comment thread core/meta/src/TDataMember.cxx Outdated
Comment thread core/meta/src/TDataMember.cxx
@pcanal

pcanal commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Sorry ... I inadvertently hit "close with comment" rather than "cancel comment"

@pcanal pcanal closed this Oct 21, 2026
@pcanal pcanal reopened this Oct 21, 2026
@pcanal

pcanal commented Oct 21, 2026

Copy link
Copy Markdown
Member

But such setter/getter are transient objects
which are deleted by next call to TDataMember::GetterMethod() / TDataMember::SetterMethod()

This is because those need to be recalculated if the actual object's type is of a derived class.

Comment thread core/gui/src/TToggle.cxx
Comment thread core/gui/src/TToggle.cxx
linev added 4 commits October 22, 2026 07:23
If data member is enum, setter method may accept enum data type
and therefore TMethodCall(class,method_name,"1") is not working.
One have to cast integer value to the type of setter method argument.
Do it only for enum, for other types more complex condition may apply
When `TDataMember::GetterMethod()` or `TDataMember::SetterMethod()`
are used to request `TMethodCall` instance - such instance is transient object
and pointer cannot be preserved for a long time.

Therefore in `TTogle` extract getter and setter callers right before usage.

Solves crash in `TASImage`` context menu where `fImageQuality` member
uses multiple toggles for selection of attribute.

Also cleanup `TToggle` code to avoid plain string usage and simplify
several methods.
Directly specify TDataMember pointer in SetToggledObject call. It has special handling in toggle object, therefore
better use clear syntax here
Directly specify TDataMember pointer in SetToggledObject call. It has special handling in toggle object, therefore
better use clear syntax here
@linev
linev requested a review from pcanal October 22, 2026 05:53
@linev linev closed this Oct 23, 2026
@linev linev reopened this Oct 23, 2026

@pcanal pcanal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@linev
linev merged commit fd1fbf8 into root-project:master Oct 26, 2026
46 of 71 checks passed
@linev
linev deleted the fix_toggle branch October 26, 2026 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR in:Core Libraries in:GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants