View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0024202 | Open CASCADE | OCCT:DRAW | public | 2013-09-24 18:28 | 2014-11-11 12:59 |
| Reporter | kgv | Assigned To | apn | ||
| Priority | normal | Severity | feature | ||
| Status | closed | Resolution | fixed | ||
| Platform | A | OS | L | ||
| Target Version | 6.8.0 | Fixed in Version | 6.8.0 | ||
| Summary | 0024202: Support class methods as callbacks for Draw Harness commands | ||||
| Description | Current Draw_Interpretor interface supports only global functions as command implementation. This results in dirty code in each command which wants to access global variables and group them in some class. | ||||
| Additional information and documentation updates | New method Draw_Interpretor::Add() accept class method as callback function for tcl-command definition:
template<typename theHandleType>
inline void Add (Standard_CString theCommandName,
Standard_CString theHelp,
Standard_CString theFileName,
const theHandleType& theObjPtr,
typename Draw_Interpretor::CallBackDataMethod<theHandleType>::methodType theMethod,
Standard_CString theGroup)
Class instance should passed by handle (smart pointer). | ||||
| Tags | No tags attached. | ||||
| Test case number | Not needed | ||||
|
|
Patch is ready for review in CR24202 branch. |
|
|
The patch seems to me potentially dangerous in that new (templated) method Draw_Interpretor::Add() accepts pointer to an object those lifetime must be longer than life of the command, while no protection is made against passing pointer to local variable, or destruction of the pointer while the command still exists. Please consider using smart pointer (Handle) to ensure sufficient life time of the object. If this is not suitable, at least please document this requirement clearly in comments. Other remarks: - name of the argument theClassPtr in the above mentioned method Add(), and similar field and argument in class Draw_CallBackDataFunc, is misleading: I suppose it should be rather called theObjectPtr - classes defined in Draw_CommandFunction.hxx should better be moved to the inside of the Draw_Interpretor class (as private nested classes), as they are made only for internal usage in this class (I do not think inheritance should be supported) - method Draw_Interpretor::add() should be made private for the same reason |
|
|
Updated patch is ready for review in CR24202_3 branch. Callback now requires Handle rather than object pointer. Classes moved to Draw_Interpretor. |
|
|
No remarks, please test |
|
|
Dear kgv, could you please rebase issue CR24202_3 on current master, there are conflict files. |
|
|
Rebased patch is ready for testing in CR24202_4 branch. |
|
|
Dear BugMaster, Branch CR24202_4 (and products from GIT master) was compiled on Linux and Windows platforms and tested. SHA-1: 8a3087db03790768c067eca733f08b1bd3e95b96 Number of compiler warnings: occt component : Linux: 44 (44 on master) Windows: 0 (0 on master) products component : Linux: 12 (12 on master) Windows: 2 (2 on master) Regressions/Differences: No regressions/differences Testing cases: Not needed Testing on Linux: Total MEMORY difference: 372819988 / 372443488 Total CPU difference: 43232.96 / 43920.73000000019 Testing on Windows: Total MEMORY difference: 419324916 / 409834624 Total CPU difference: 38269.640625 / 35405.109375 There are not differences in images found by testdiff. |
|
occt: master dda67c1c 2014-02-05 09:17:29 Committer: apn Details Diff |
0024202: Support class methods as callbacks for Draw Harness commands |
Affected Issues 0024202 |
|
| mod - src/Draw/Draw.cdl | Diff File | ||
| rm - src/Draw/Draw_CommandFunction.hxx | Diff File | ||
| rm - src/Draw/Draw_Interpretor.cdl | Diff File | ||
| mod - src/Draw/Draw_Interpretor.cxx | Diff File | ||
| add - src/Draw/Draw_Interpretor.hxx | Diff File | ||
| mod - src/Draw/FILES | Diff File | ||
| mod - src/NCollection/NCollection_Handle.hxx | Diff File | ||
| mod - src/QABugs/QABugs_19.cxx | Diff File | ||
| mod - src/Standard/Standard_DefineHandle.hxx | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-09-24 18:28 | kgv | New Issue | |
| 2013-09-24 18:28 | kgv | Assigned To | => mkv |
| 2013-09-24 18:28 | kgv | Assigned To | mkv => kgv |
| 2013-09-24 18:32 | kgv | Note Added: 0025715 | |
| 2013-09-24 18:32 | kgv | Assigned To | kgv => abv |
| 2013-09-24 18:32 | kgv | Status | new => resolved |
| 2013-12-21 10:13 |
|
Target Version | 6.7.0 => 6.7.1 |
| 2014-01-15 10:58 |
|
Note Added: 0027487 | |
| 2014-01-15 10:58 |
|
Assigned To | abv => kgv |
| 2014-01-15 10:58 |
|
Status | resolved => assigned |
| 2014-01-22 18:35 | kgv | Note Added: 0027613 | |
| 2014-01-22 18:35 | kgv | Assigned To | kgv => abv |
| 2014-01-22 18:35 | kgv | Status | assigned => resolved |
| 2014-02-05 12:51 |
|
Note Added: 0027761 | |
| 2014-02-05 12:51 |
|
Assigned To | abv => bugmaster |
| 2014-02-05 12:51 |
|
Status | resolved => reviewed |
| 2014-02-05 13:08 |
|
Note Added: 0027762 | |
| 2014-02-05 13:08 |
|
Assigned To | bugmaster => kgv |
| 2014-02-05 13:08 |
|
Status | reviewed => feedback |
| 2014-02-05 13:18 | kgv | Note Added: 0027763 | |
| 2014-02-05 13:18 | kgv | Assigned To | kgv => mkv |
| 2014-02-05 13:18 | kgv | Status | feedback => reviewed |
| 2014-02-06 07:24 |
|
Note Added: 0027786 | |
| 2014-02-06 07:24 |
|
Test case number | => Not needed |
| 2014-02-06 07:24 |
|
Assigned To | mkv => bugmaster |
| 2014-02-06 07:24 |
|
Status | reviewed => tested |
| 2014-02-10 12:41 | apn | Changeset attached | => occt master dda67c1c |
| 2014-02-10 12:41 | apn | Assigned To | bugmaster => apn |
| 2014-02-10 12:41 | apn | Status | tested => verified |
| 2014-02-10 12:41 | apn | Resolution | open => fixed |
| 2014-04-04 12:36 |
|
Target Version | 6.7.1 => 6.8.0 |
| 2014-10-07 11:36 | kgv | Additional Information Updated | |
| 2014-11-11 12:44 |
|
Fixed in Version | => 6.8.0 |
| 2014-11-11 12:59 |
|
Status | verified => closed |
