MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0028478Community[OCCT] OCCT:Foundation Classespublic2017-02-20 13:232017-09-29 16:29
ReporterBenjaminBihler 
Assigned Tobugmaster 
PrioritynormalSeveritytrivial 
StatusclosedResolutionfixed 
PlatformMingw64OSWindowsOS Version7
Product Version[OCCT] 7.1.0 
Target Version[OCCT] 7.2.0Fixed in Version[OCCT] 7.2.0 
Summary0028478: Scope Names Are Swallowed in Message_ProgressSentry Constructors
DescriptionThe constructors call progress->SetName with the task name and afterwards they call progress->NewScope without passing the name. But NewScope(...) will overwrite the name with an empty string, so at the end the name is not set.

If I am correct, then task names have never been treated correctly in Message_ProgressSentry, except if someone has called progressIndicator->SetName(...) later on. This makes me wonder. Did I get something wrong?
Steps To ReproduceCreate a Message_ProgressIndicator, use a Message_ProgressSentry to initialize the task and then display the task name by calling GetScope(1).GetName() of Message_ProgressIndicator. The task name will be empty because of this issue.
TagsNo tags attached.
Test case numberbugs fclasses bug28478, perf fclasses progress
Attached Files

- Relationships

-  Notes
(0063913)
git (administrator)
2017-02-20 13:27

Branch CR28478 has been created by BenjaminBihler.

SHA-1: 6007f4b782fbb8b66fb0e0094c2c00dc8cdb9dd2


Detailed log of new commits:

Author: Benjamin Bihler
Date: Mon Feb 20 11:25:54 2017 +0100

    0028478: Scope Names Are Swallowed in Message_ProgressSentry Constructors
    
    Constructors pass names during scope creation now instead of setting the
    names before creating a new scope.
(0067613)
BenjaminBihler (developer)
2017-06-22 17:00

Reminder sent to: abv

Andrey, would you mind having a look at this issue? I have already created a solution for it.
(0069588)
git (administrator)
2017-08-18 16:12

Branch CR28478_1 has been created by abv.

SHA-1: 41345d408cd23a4e81f70ecd8b9efa41ac75343a


Detailed log of new commits:

Author: abv
Date: Fri Aug 18 15:05:34 2017 +0300

    0028478: Scope Names Are Swallowed in Message_ProgressSentry Constructors
    
    Setting name of top-level scope in Message_ProgressIndicator to "Step" by default is avoided, for consistency with default behavior for nested scopes.
    
    Tests are added to control output and performance of progress indicator (bugs fclasses bug28478 and perf fclasses progress, respectively).
    
    Implementation of class Draw_ProgressIndicator is improved to update indicator basing on achieved total progress (1% by default) instead of elapsed time since last update.
    
    Method OSD_Chronometer::Restart() is fixed to actually reset the counter.
    
    DRAW command readstl is improved to show progress indicator if configured (by command XProgress).
(0069591)
abv (manager)
2017-08-18 16:19

Hello Benjamin,

I beg your pardon for long delay with my reply -- alas, I was totally occupied by urgent tasks until this week, and progress indicator is too tricky subject to give fast answers on it.

Actually there seem to be no problem in the current implementation. I believe you have been confused by recursive logic of the progress indicator (I admit it is not well documented, will try to improve that).

The point is that the name is assigned not to the progress indicator as a whole, but to current scope. The method SetName() and similar provided by Message_ProgressIndicator class are just short-cuts to the current scope; this can be rather misleading, so we likely will remove them in the future.

On each level of recursion, the procedure that gets the scope allocated by its calling process configures it according to its nature: it sets name, range, step, etc. The name is intended to indicate what is being incremented on the current level (like "step 1 of 23"). Then the procedure increments the progress within its scope, and can (optionally) create sub-scopes for nested procedures.

Class Message_ProgressSentry is intended to provide convenient interface to this.

Consider this sample code:

<code>
  // Outer cycle
  Message_ProgressSentry anOuter (aProgress, "Outer", 0, nbOuter, 1);
  for (int i = 0; i < nbOuter && anOuter.More(); i++, anOuter.Next())
  {
    // Inner cycle
    Message_ProgressSentry anInner (aProgress, "Inner", 0, nbInner, 1);
    for (int j = 0; j < nbInner && anInner.More(); j++, anInner.Next())
    {
      // Cycle body
    }
  }
</code>

In the current implementation it will produce desired output (see implementation of class Draw_ProgressIndicator in the branch CR28478_1 -- I have had to modify it to get usable output):

Progress: 0% Outer: 1 / 3
Progress: 17% Outer: 1 / 3 Inner: 1 / 2
Progress: 33% Outer: 1 / 3 Inner: 2 / 2
Progress: 50% Outer: 2 / 3 Inner: 1 / 2
Progress: 67% Outer: 2 / 3 Inner: 2 / 2
Progress: 83% Outer: 3 / 3 Inner: 1 / 2
Progress: 100% Outer: 3 / 3 Inner: 2 / 2

With your change, the output becomes wrong:

Progress: 0% Step: 1 / 3 Outer: 0 / 100
Progress: 17% Step: 1 / 3 Outer: 1 / 2 Inner: 100 / 100
Progress: 33% Step: 1 / 3 Outer: 2 / 2 Inner: 100 / 100
Progress: 50% Step: 2 / 3 Outer: 1 / 2 Inner: 100 / 100
Progress: 67% Step: 2 / 3 Outer: 2 / 2 Inner: 100 / 100
Progress: 83% Step: 3 / 3 Outer: 1 / 2 Inner: 100 / 100
Progress: 100% Step: 3 / 3 Outer: 2 / 2 Inner: 100 / 100

You can see that names get assigned to sub-scopes and do not match the range.

Thus I consider current implementation as correct and going to keep it (apart of some corrections and new tests added, see branch CR28478_1).
Please let me know your opinion on this.
(0069594)
git (administrator)
2017-08-18 16:45

Branch CR28478_1 has been updated forcibly by abv.

SHA-1: 34f43b5fefd04883154321be4da0f653bd57c5e5
(0069694)
BenjaminBihler (developer)
2017-08-21 09:54

Okay, I see. I have indeed not completely understood how to deal with the scope hierarchy and I have not been aware of Draw_ProgressIndicator. Otherwise I could have learned from its implementation.

So I am actually happy with the progress indicator code (not with the documentation however). ;)

Another minor remark: the documentation of Message_ProgressIndicator states that after resetting, the scale has the name "Step", but you have commented out that line.
(0069709)
abv (manager)
2017-08-21 13:23

Yes sure, thank you for noting this! I am going to update this description, will take this remark into account.
(0069733)
git (administrator)
2017-08-22 10:42

Branch CR28478_1 has been updated by abv.

SHA-1: d935af5a77ba44535596b2cb058b996be88395fc


Detailed log of new commits:

Author: abv
Date: Tue Aug 22 10:41:59 2017 +0300

    Description of class Message_ProgressIndicator is updated.

(0070030)
git (administrator)
2017-08-29 10:37

Branch CR28478_2 has been created by abv.

SHA-1: de929c73591be20901d01e53ce521c96848f20a4


Detailed log of new commits:

Author: abv
Date: Fri Aug 18 15:05:34 2017 +0300

    0028478: Scope Names Are Swallowed in Message_ProgressSentry Constructors
    
    Tests are added to control output and performance of progress indicator (bugs fclasses bug28478 and perf fclasses progress, respectively).
    
    Implementation of class Draw_ProgressIndicator is improved to update indicator basing on achieved total progress (1% by default) instead of elapsed time since last update.
    
    Method OSD_Chronometer::Restart() is fixed to actually reset the counter.
    
    DRAW command readstl is improved to show progress indicator if configured (by command XProgress).
    
    Description of class Message_ProgressIndicator is updated; code example is added in description of Message_ProgressSentry.
(0070031)
abv (manager)
2017-08-29 10:39

Please consider branch CR28478_2 for integration. The patch has been tested in its previous state, different only by class comment; see Jenkins job CR28478-master-abv
(0070036)
bugmaster (administrator)
2017-08-29 12:04

Combination -
OCCT branch : CR28478_2 - SHA-1: de929c73591be20901d01e53ce521c96848f20a4
Products branch : master
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.

http://jenkins-test-10.nnov.opencascade.com/view/CR28478-master-abv/ [^]

Number of compiler warnings:

OCCT :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

Products :
Linux: 4 (4 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:

Windows:
OCCT
Total CPU difference: 17330.010689098726 / 17295.75286949859 [+0.20%]
Producst
Total CPU difference: 7688.758886599963 / 7720.302288799964 [-0.41%]

Linux:
OCCT
Total CPU difference: 19451.89000000023 / 19456.560000000376 [-0.02%]
Products
Total CPU difference: 7703.40000000009 / 7725.120000000072 [-0.28%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

New test case is OK
(0070183)
git (administrator)
2017-09-04 18:20

Branch CR28478 has been deleted by kgv.

SHA-1: 6007f4b782fbb8b66fb0e0094c2c00dc8cdb9dd2
(0070184)
git (administrator)
2017-09-04 18:20

Branch CR28478_1 has been deleted by kgv.

SHA-1: d935af5a77ba44535596b2cb058b996be88395fc
(0070185)
git (administrator)
2017-09-04 18:20

Branch CR28478_2 has been deleted by kgv.

SHA-1: de929c73591be20901d01e53ce521c96848f20a4

- Related Changesets
occt: master 6b55f8e3
Timestamp: 2017-08-18 12:05:34
Author: abv
Committer: bugmaster
Details ] Diff ]
0028478: Scope Names Are Swallowed in Message_ProgressSentry Constructors

Tests are added to control output and performance of progress indicator (bugs fclasses bug28478 and perf fclasses progress, respectively).

Implementation of class Draw_ProgressIndicator is improved to update indicator basing on achieved total progress (1% by default) instead of elapsed time since last update.

Method OSD_Chronometer::Restart() is fixed to actually reset the counter.

DRAW command readstl is improved to show progress indicator if configured (by command XProgress).

Description of class Message_ProgressIndicator is updated; code example is added in description of Message_ProgressSentry.
mod - src/Draw/Draw_ProgressIndicator.cxx Diff ] File ]
mod - src/Draw/Draw_ProgressIndicator.hxx Diff ] File ]
mod - src/Message/Message_ProgressIndicator.hxx Diff ] File ]
mod - src/Message/Message_ProgressSentry.hxx Diff ] File ]
mod - src/OSD/OSD_Chronometer.cxx Diff ] File ]
mod - src/QABugs/QABugs_11.cxx Diff ] File ]
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML.cxx Diff ] File ]
add - tests/bugs/fclasses/bug28478 Diff ] File ]
add - tests/perf/fclasses/progress Diff ] File ]

- Issue History
Date Modified Username Field Change
2017-02-20 13:23 BenjaminBihler New Issue
2017-02-20 13:23 BenjaminBihler Assigned To => BenjaminBihler
2017-02-20 13:27 git Note Added: 0063913
2017-02-20 13:27 BenjaminBihler Assigned To BenjaminBihler => mpv
2017-02-20 13:27 BenjaminBihler Status new => resolved
2017-02-20 13:46 kgv Assigned To mpv => abv
2017-02-20 13:46 kgv Category OCCT:Application Framework => OCCT:Foundation Classes
2017-06-22 17:00 BenjaminBihler Note Added: 0067613
2017-08-18 16:12 git Note Added: 0069588
2017-08-18 16:19 abv Note Added: 0069591
2017-08-18 16:45 git Note Added: 0069594
2017-08-21 08:41 abv Assigned To abv => BenjaminBihler
2017-08-21 08:41 abv Status resolved => feedback
2017-08-21 09:54 BenjaminBihler Note Added: 0069694
2017-08-21 10:55 BenjaminBihler Assigned To BenjaminBihler => abv
2017-08-21 13:23 abv Note Added: 0069709
2017-08-22 10:42 git Note Added: 0069733
2017-08-29 10:37 git Note Added: 0070030
2017-08-29 10:39 abv Note Added: 0070031
2017-08-29 10:39 abv Assigned To abv => bugmaster
2017-08-29 10:39 abv Status feedback => reviewed
2017-08-29 12:00 bugmaster Test case number => bugs fclasses bug28478
2017-08-29 12:04 bugmaster Note Added: 0070036
2017-08-29 12:04 bugmaster Status reviewed => tested
2017-08-29 15:30 bugmaster Test case number bugs fclasses bug28478 => bugs fclasses bug28478, perf fclasses progress
2017-08-31 18:37 bugmaster Changeset attached => occt master 6b55f8e3
2017-08-31 18:37 bugmaster Status tested => verified
2017-08-31 18:37 bugmaster Resolution open => fixed
2017-09-04 18:20 git Note Added: 0070183
2017-09-04 18:20 git Note Added: 0070184
2017-09-04 18:20 git Note Added: 0070185
2017-09-29 16:17 user533 Fixed in Version => 7.2.0
2017-09-29 16:29 user533 Status verified => closed


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker