View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0030697 | Open CASCADE | OCCT:DRAW | public | 2019-05-06 23:09 | 2020-12-02 17:11 |
Reporter | kgv | Assigned To | bugmaster | ||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Product Version | 6.8.0 | ||||
Target Version | 7.5.0 | Fixed in Version | 7.5.0 | ||
Summary | 0030697: Draw Harness - Draw_Printer should not be set to Message::DefaultMessenger() by default | ||||
Description | Currently Draw Harness removes std::cout printer from Message::DefaultMessenger() and appends Draw_Printer instead. Draw_Printer redirects output to Tcl command result, which results in the following issues: - DefaultMessenger() is a global callback which can be used by any OCCT code, not exactly designed for Tcl output of particular command. This might include errors, failures and warnings, so that command output might become unexpectedly broken. Example - vreadpixel command which uses a workaround clearing Tcl output to avoid breaking tests (as result, messages from TKOpenGl are discarded and might remain unnoticed). - Feeding Tcl interpreter is not thread safe, while DefaultMessenger() can be used for emitting messages from working threads resulting in Draw Harness crash. - Draw_Printer accumulates messages internally and prints them into console only after command execution completion. This is annoying behavior for a long process reporting some intermediate messages - so that important problems can be seen only at the very end of command. It is proposed reverted patch making Draw_Printer as default printer ad Draw Harness start and reconsider its usage in particular commands. The origin of Draw_Printer are Data Exchange plugins, so that them should be checked carefully. | ||||
Steps To Reproduce | Not required | ||||
Tags | No tags attached. | ||||
Test case number | |||||
related to | 0024644 | closed | bugmaster | Draw_Printer - provide the way to control messages gravity filter |
related to | 0025748 | closed | Foundation Classes - Parallel version of progress indicator | |
related to | 0031036 | closed | apn | Foundation Classes, Message_PrinterOStream - add option printing colored text into console |
|
Branch CR30697 has been created by kgv. SHA-1: 80145f0110178da4585ea764914f24bfef7abbac Detailed log of new commits: Author: kgv Date: Mon May 6 23:05:13 2019 +0300 0030697: Draw Harness - Draw_Printer should not be set to Message::DefaultMessenger() by default |
|
Branch CR30697 has been updated by mzernova. SHA-1: 9f6e5d82deb44c9cafd530d15775fb30aaf641cc Detailed log of new commits: Author: mzernova Date: Thu Oct 31 16:03:29 2019 +0300 Fixed some bugs that occurred when using the default std::cout from Message::DefaultMessenger() instead of Draw_Printer |
|
@@ -147,9 +147,9 @@ Standard_Boolean Draw_ProgressIndicator::Show(const Standard_Boolean force) } // Print textual progress info - if ( myTextMode ) - Message::DefaultMessenger()->Send (text, Message_Info); - + if (myTextMode && myDraw) + (*(Draw_Interpretor*)myDraw) << text << "\n"; This should be a new dedicated option - only for test cases of progress indicator itself (their is a couple of them). Default implementation of Text Mode should print into std::cout (not Messenger) so that progress will be seen during task. |
|
+ const Handle(Message_Messenger)& aMsgMgr = Message::DefaultMessenger(); + if (!aMsgMgr.IsNull()) + { + aMsgMgr->RemovePrinters (STANDARD_TYPE (Message_PrinterOStream)); + aMsgMgr->RemovePrinters (STANDARD_TYPE (Draw_Printer)); + aMsgMgr->AddPrinter (new Draw_Printer (di)); + } It is better copying the whole list of printers and adding them back. |
|
Branch CR30697 has been updated by mzernova. SHA-1: 72bc28711fb2e2ede6204eb84a8ae4b79c7abc2e Detailed log of new commits: Author: mzernova Date: Thu Nov 7 14:12:31 2019 +0300 remarks from kgv |
|
Branch CR30697_1 has been created by mzernova. SHA-1: b1b14ad2d234606961b0f8757c1e586ad5aedf64 Detailed log of new commits: Author: kgv Date: Mon May 6 23:05:13 2019 +0300 0030697: Draw Harness - Draw_Printer should not be set to Message::DefaultMessenger() by default Fixed bugs that occurred when using the default std::cout from Message::DefaultMessenger() instead of Draw_Printer |
|
Branch CR30697 has been updated by mzernova. SHA-1: 14286674ea365a12d3ac873639dba2d7d243b5b5 Detailed log of new commits: Author: mzernova Date: Fri Nov 8 14:10:14 2019 +0300 fixed some problems caused by changes in Draw_ProgressIndicator::Show method |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 2fdf1fe2aac438c62fc4c6d2f29c5c7a023e8942 |
|
+ aProgress->SetTclMode (Standard_True); I think "SetTclOutput" would be better than "SetTclMode". > fixed some problems caused by changes in Draw_ProgressIndicator::Show method > @@ -454,6 +454,7 @@ static Standard_Integer sewing (Draw_Interpretor& theDi, > @@ -484,6 +484,7 @@ static Standard_Integer fixshape (Draw_Interpretor& di, > @@ -312,6 +312,7 @@ static Standard_Integer createmesh Could you please refer to test cases where this modification is necessary? The location of these modifications looks suspicion to me. |
|
Branch CR30697 has been updated by mzernova. SHA-1: 1342dab2e895ec545aff0fcf48753d90d167079c Detailed log of new commits: Author: mzernova Date: Fri Nov 8 20:18:54 2019 +0300 A dedicated option was added to Draw_ProgressIndicator, for outputting data to the tcl when performing tests |
|
test bugs modalg_5 bug22747 test bugs moddata_2 bug22572 test bugs moddata_2 bug22746_1 test bugs moddata_2 bug22746_2 test bugs moddata_2 bug22746_3 |
|
These are tests for progress indicator used within the algorithm. Expected change is modifying test case to use "XProgress -tclOutput" rather than modifying "sewing" command. # Progress indicator in sewing algorithm vinit XProgress -t set List1 [sewing result 0.1 a] if { [string compare $List1 ""] != 0 } { |
|
Branch CR30697 has been updated by mzernova. SHA-1: b0eb3f1777d43168d763c7f455580a4d43c2914a Detailed log of new commits: Author: mzernova Date: Thu Nov 14 15:15:10 2019 +0300 Added -tclOutput parameter to XProgress command |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 05f1e2245b32b4fc69eb4bf8a6673b0b61d44fa0 |
|
The patch CR30697 is ready to review |
|
http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR30697_1-master-mzernova |
|
+ if (!strcmp (argv[i], "-tclOutput")) It is preferred used case-insensitive comparison of Tcl command arguments. > TCollection_AsciiString anArgCase (argv[i]); > anArgCase.LowerCase(); > if (anArgCase == "-tcloutput") +Standard_Boolean &Draw_ProgressIndicator::DefaultTclOutput () Standard_Boolean& Draw_ProgressIndicator::DefaultTclOutput() +void Draw_ProgressIndicator::SetTclOutput (const Standard_Boolean theTclOutput) +{ ... +Standard_Boolean Draw_ProgressIndicator::GetTclOutput() const +{ + return myTclOutput; Inline definition would be better. + (*(Draw_Interpretor*) myDraw) << aText.str().c_str() << "\n"; myDraw declaration can be improved to avoid type cast. |
|
@@ -136,7 +138,9 @@ void BOPTest::ReportAlerts(const Handle(Message_Report)& theReport) } // output message with list of shapes - Message::DefaultMessenger()->Send (aText, anAlertTypes[iGravity]); + Draw_Interpretor& aDrawInterpretor = Draw::GetInterpretor(); + Handle(Message_Messenger) aMessenger = new Message_Messenger (new Draw_Printer (aDrawInterpretor)); + aMessenger->Send (aText, anAlertTypes[iGravity]); Could be just "aDrawInterpretor << aText" in this context. --- a/src/XSDRAWIGES/XSDRAWIGES.cxx +++ b/src/XSDRAWIGES/XSDRAWIGES.cxx @@ -106,6 +106,7 @@ static Standard_Integer igesbrep (Draw_Interpretor& di, Standard_Integer argc, c // Progress indicator Handle(Draw_ProgressIndicator) progress = new Draw_ProgressIndicator ( di, 1 ); + progress->SetTclOutput (Standard_True); ... --- a/src/XSDRAWSTEP/XSDRAWSTEP.cxx +++ b/src/XSDRAWSTEP/XSDRAWSTEP.cxx @@ -96,6 +96,7 @@ static Standard_Integer stepread (Draw_Interpretor& di/*theCommands*/, Standard_ // Progress indicator Handle(Draw_ProgressIndicator) progress = new Draw_ProgressIndicator ( di, 1 ); + progress->SetTclOutput (Standard_True); These are unexpected - I suppose that only specific test cases should be "XProgress -tclOutput" if there are any. |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: c68f6ba996451f5aa9455669ca4f920d6acf8f31 |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 7f81e983b549ff9019d3778d8dc6641c30f68a2e |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 64845d8b74c4df6f70770ce3400d932b5452c6ac |
|
--- a/src/Draw/Draw_VariableCommands.cxx +++ b/src/Draw/Draw_VariableCommands.cxx @@ -156,6 +156,7 @@ static Standard_Integer save(Draw_Interpretor& di, + progress->SetTclOutput (Standard_True); @@ -221,6 +222,7 @@ static Standard_Integer restore(Draw_Interpretor& di, + progress->SetTclOutput (Standard_True); Same remark for these two commands. |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 9d15f4d254c4e676cd3b455506dd92e2f7c2bf95 |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 9a09616c58ea76a9944c6b565155ba71f837e767 |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: aaf2ac08860940f0d0972e78646feb406d9527a5 |
|
+ //! Sets tcl output mode (on/off) + Standard_EXPORT inline void SetTclOutput (const Standard_Boolean theTclOutput) + { + myTclOutput = theTclOutput; + } + + //! Gets tcl output mode (on/off) + Standard_EXPORT inline Standard_Boolean GetTclOutput() const + { + return myTclOutput; + } Unexpected Standard_EXPORT within inline functions. + aPrinters.Append (aMsgMgr->ChangePrinters()); + for (Message_SequenceOfPrinters::Iterator aPrinterIter (aMsgMgr->Printers()); + aPrinterIter.More(); aPrinterIter.Next()) + { + aMsgMgr->RemovePrinter (aPrinterIter.Value()); + } It would be more natural iterating through copied aPrinters instead of modified sequence. + const Handle(Message_Messenger)& aMsgMgr = Message::DefaultMessenger(); + Message_SequenceOfPrinters aPrinters; + if (!aMsgMgr.IsNull()) NULL checks are redundant here - in no event Message::DefaultMessenger() should return a NULL pointer. |
|
Branch CR30697 has been updated forcibly by mzernova. SHA-1: db9c3d101b998c8f4d8fd2acb27651bddf5ef747 |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: d50855e5b762553bc6b049e5ff7b1b0e1fb54784 |
|
Branch CR30697 has been updated by mzernova. SHA-1: a7a15df3c0b7736fdfa60d4304e08c6745e851d7 Detailed log of new commits: Author: mzernova Date: Wed Nov 20 10:56:42 2019 +0300 remarks from osa |
|
Branch CR30697_1 has been updated forcibly by mzernova. SHA-1: 1a75e584fac58893bd88c9678a16041db7050b7a |
|
The patch CR30697 is ready to review |
|
The patch was reviewed |
|
Combination - OCCT branch : WEEK-47 master SHA - 51ee6a7dbb3a740ba0cfc693ce5e7a83663f05f4 5f5b1aed1c6e139bbd34314eca77ae7abcd8895c Products branch : WEEK-47 SHA - 1fe84f5f3e15bca9d1c6f8e9646341c4df675aec was compiled on Linux, MacOS and Windows platforms and tested in optimize mode. Number of compiler warnings: No new/fixed warnings Regressions/Differences/Improvements: No regressions/differences CPU differences: Debian80-64: OCCT Total CPU difference: 16799.850000000122 / 16823.120000000046 [-0.14%] Products Total CPU difference: 10810.050000000052 / 10787.080000000053 [+0.21%] Windows-64-VC14: OCCT Total CPU difference: 18294.203125 / 18299.59375 [-0.03%] Products Total CPU difference: 12837.546875 / 12853.21875 [-0.12%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Branch CR30697_1 has been deleted by inv. SHA-1: 1a75e584fac58893bd88c9678a16041db7050b7a |
|
Branch CR30697 has been deleted by inv. SHA-1: a7a15df3c0b7736fdfa60d4304e08c6745e851d7 |
occt: master caee80f3 2019-05-06 20:05:13 Committer: bugmaster Details Diff |
0030697: Draw Harness - Draw_Printer should not be set to Message::DefaultMessenger() by default Fixed bugs that occurred when using the default std::cout from Message::DefaultMessenger() instead of Draw_Printer A dedicated option was added to Draw_ProgressIndicator, for outputting data to the tcl when performing tests Added -tclOutput parameter to XProgress command |
Affected Issues 0030697 |
|
mod - src/BOPTest/BOPTest.cxx | Diff File | ||
mod - src/DBRep/DBRep.cxx | Diff File | ||
mod - src/Draw/Draw_Commands.cxx | Diff File | ||
mod - src/Draw/Draw_ProgressIndicator.cxx | Diff File | ||
mod - src/Draw/Draw_ProgressIndicator.hxx | Diff File | ||
mod - src/QABugs/QABugs_11.cxx | Diff File | ||
mod - src/QABugs/QABugs_19.cxx | Diff File | ||
mod - src/XSDRAW/XSDRAW.cxx | Diff File | ||
mod - tests/bugs/modalg_5/bug22747 | Diff File | ||
mod - tests/bugs/moddata_2/bug22572 | Diff File | ||
mod - tests/bugs/moddata_2/bug22746_1 | Diff File | ||
mod - tests/bugs/moddata_2/bug22746_2 | Diff File | ||
mod - tests/bugs/moddata_2/bug22746_3 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2019-05-06 23:09 | kgv | New Issue | |
2019-05-06 23:09 | kgv | Assigned To | => kgv |
2019-05-06 23:12 | git | Note Added: 0084205 | |
2019-05-06 23:14 | kgv | Relationship added | related to 0024644 |
2019-05-06 23:14 | kgv | Product Version | => 6.8.0 |
2019-05-12 16:46 | kgv | Relationship added | related to 0025748 |
2019-09-04 15:43 |
|
Target Version | 7.4.0 => 7.5.0 |
2019-10-05 20:16 | kgv | Assigned To | kgv => user897 |
2019-10-05 20:16 | kgv | Status | new => assigned |
2019-10-05 22:16 | kgv | Relationship added | related to 0031036 |
2019-11-06 11:30 | git | Note Added: 0088747 | |
2019-11-06 11:47 | kgv | Note Added: 0088748 | |
2019-11-06 11:49 | kgv | Note Added: 0088749 | |
2019-11-06 12:24 |
|
Assigned To | user897 => mzernova |
2019-11-07 16:22 | git | Note Added: 0088777 | |
2019-11-07 16:22 | git | Note Added: 0088778 | |
2019-11-08 20:12 | git | Note Added: 0088816 | |
2019-11-08 20:12 | git | Note Added: 0088817 | |
2019-11-08 20:15 | kgv | Note Added: 0088818 | |
2019-11-08 20:21 | git | Note Added: 0088819 | |
2019-11-14 10:41 | mzernova | Note Added: 0088953 | |
2019-11-14 10:55 | kgv | Note Added: 0088954 | |
2019-11-14 15:41 | git | Note Added: 0088962 | |
2019-11-14 15:41 | git | Note Added: 0088963 | |
2019-11-14 17:39 | mzernova | Note Added: 0088966 | |
2019-11-14 17:39 | mzernova | Assigned To | mzernova => osa |
2019-11-14 17:39 | mzernova | Status | assigned => resolved |
2019-11-14 17:39 | mzernova | Steps to Reproduce Updated | |
2019-11-14 17:44 | mzernova | Note Added: 0088969 | |
2019-11-14 17:46 | kgv | Note Added: 0088970 | |
2019-11-14 17:54 | kgv | Note Added: 0088971 | |
2019-11-15 11:06 | git | Note Added: 0088983 | |
2019-11-15 11:48 | git | Note Added: 0088987 | |
2019-11-15 11:53 | git | Note Added: 0088988 | |
2019-11-15 12:01 | kgv | Note Added: 0088989 | |
2019-11-15 12:53 | git | Note Added: 0088993 | |
2019-11-15 19:09 | git | Note Added: 0089016 | |
2019-11-18 10:20 | git | Note Added: 0089039 | |
2019-11-18 11:58 | kgv | Note Added: 0089045 | |
2019-11-18 13:43 | git | Note Added: 0089048 | |
2019-11-18 13:44 | git | Note Added: 0089049 | |
2019-11-19 18:33 |
|
Assigned To | osa => mzernova |
2019-11-19 18:33 |
|
Status | resolved => assigned |
2019-11-20 11:06 | git | Note Added: 0089091 | |
2019-11-20 11:06 | git | Note Added: 0089092 | |
2019-11-21 11:21 | mzernova | Note Added: 0089116 | |
2019-11-21 11:21 | mzernova | Assigned To | mzernova => osa |
2019-11-21 11:21 | mzernova | Status | assigned => resolved |
2019-11-21 12:02 |
|
Note Added: 0089121 | |
2019-11-21 12:02 |
|
Assigned To | osa => bugmaster |
2019-11-21 12:02 |
|
Status | resolved => reviewed |
2019-11-22 10:20 | bugmaster | Note Added: 0089144 | |
2019-11-22 10:20 | bugmaster | Status | reviewed => tested |
2019-11-24 11:21 | bugmaster | Changeset attached | => occt master caee80f3 |
2019-11-24 11:21 | bugmaster | Status | tested => verified |
2019-11-24 11:21 | bugmaster | Resolution | open => fixed |
2019-11-24 11:35 | git | Note Added: 0089175 | |
2019-11-24 11:35 | git | Note Added: 0089176 | |
2020-12-02 16:40 |
|
Fixed in Version | => 7.5.0 |
2020-12-02 17:11 |
|
Status | verified => closed |