Skip to content

Improve SavePrimitive in TG classes#18038

Merged
linev merged 20 commits intoroot-project:masterfrom
linev:save_next
Mar 19, 2025
Merged

Improve SavePrimitive in TG classes#18038
linev merged 20 commits intoroot-project:masterfrom
linev:save_next

Conversation

@linev
Copy link
Member

@linev linev commented Mar 18, 2025

  1. Introduce TGFrame::SaveCrtrArgs method to prepare options and custom color for use in saved constructor. There are many places in code where such combination handled with many if statements
  2. Use new method where it make sence
  3. Replace std::endl by \n and quote by \"
  4. Fix several problems where with custom colors also font and uGC->GetGC() must be initialized. Otherwise when saved into the macro arguments will remain empty, not necessary
  5. Add to TGMainFrame::SavePrimitive several commands to popup window
  6. Always use TString::ReplaceSpecialCppChars when saving text or tooltips
  7. Simplify implementation of TGFrame::GetOptionString()

Tested with several macros from tutorials like calendar.C or listBox.C or statusBar.C

linev added 6 commits March 18, 2025 11:00
While it used to store string args in C++ macro,
replace these delimiter together with backslah \ and double quote " symbols
Use lambda to append produced string
Call MapSubwindows, Resize and MapWindow to produce
reasonable output at the end of macro
Simplify connected singals saving
Remap color signal
Use TString::ReplaceSpecialCppChars when store text
@github-actions
Copy link

github-actions bot commented Mar 18, 2025

Test Results

    20 files      20 suites   4d 17h 1m 32s ⏱️
 2 726 tests  2 726 ✅ 0 💤 0 ❌
52 591 runs  52 591 ✅ 0 💤 0 ❌

Results for commit 445cdb3.

♻️ This comment has been updated with latest results.

linev added 12 commits March 19, 2025 09:45
Save custom brackground color if necessary and
create extra args string which typically appears in the end
of the constructor args.

Will helps to simplify saving of constructor
Use new SaveCtorArgs to simpligy code
In TGSlider::GetTypeString ensure that at least 0 is returned
In some classes with custom color also font and pixmap must be initialized
Add additional checks in TGButton.cxx to init font and pixmap
parGC and parFont has default value
Create interim variables based on gROOT->ClassSaved(Class())
In several classes default background is WhitePixColor
Therefore this value should be checked when storing color
Use ReplaceSpecialCppChars, avoid quote, apply clang-format
Reduce dynamic vars, use local variables,
remove includes optimization
Simplify text conversion, avoid plain char* allocation
Do not try to save void* pointer stored in fUserData
Correctly store picture for all menu entry kinds
Use clear API to save items and list of items
Use more unique variable names for items and pictures to avoid clashes
Add images already with AddItem invocation
@linev linev requested a review from couet as a code owner March 19, 2025 10:34
Copy link
Member

@bellenot bellenot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. FYI, one way of testing if it works properly would be to run stressgui before and after these changes

@linev
Copy link
Member Author

linev commented Mar 19, 2025

FYI, one way of testing if it works properly would be to run stressgui before and after these changes

stressGUI creates macros but not execute them.
And created size is not checked.

I checked - this PR does not affect stressGUI

@bellenot
Copy link
Member

stressGUI creates macros but not execute them.
And created size is not checked.

One should execute stressgui -ref to generate the .ref file and then stressgui to run the test

@linev
Copy link
Member Author

linev commented Mar 19, 2025

One should execute stressgui -ref to generate the .ref file and then stressgui to run the test

Is all about PNG image sizes.
But created ROOT macro is not checked and not executed.
I already find more bugs and will fix them with following PR

@bellenot
Copy link
Member

Oh yes, sorry, you're right...

@linev linev merged commit cb10c61 into root-project:master Mar 19, 2025
22 of 23 checks passed
@linev linev deleted the save_next branch March 19, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants