View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0027058 | Open CASCADE | OCCT:Visualization | public | 2016-01-04 09:50 | 2016-04-20 15:50 |
| Reporter | Assigned To | ||||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Product Version | 7.0.0 | ||||
| Target Version | 7.0.0 | Fixed in Version | 7.0.0 | ||
| Summary | 0027058: AIS_ColorScale defines methods SetColor and SetWidth hiding inherited methods | ||||
| Description | As suggested by CLang compiler warning (see 0025076), AIS_ColorScale defines methods SetColor and SetWidth that have names corresponding to virtual methods of its base class, but have quite different meaning: - In base class (AIS_InteractiveObject), these methods define color and width of the lines (wireframe) - In AIS_ColorScale, these methods define colors of particular parts of the color scale, and breadth of the color scale (in pixels, I suppose) This is rather confusing, and perhaps AIS_ColorScale class can be revised to avoid this confusion. | ||||
| Steps To Reproduce | Compile with CLang | ||||
| Tags | No tags attached. | ||||
| Test case number | |||||
|
|
Branch CR27058 has been created by abv. SHA-1: ca9e30d23013ea278505a1fed054ff5d1abda159 Detailed log of new commits: Author: abv Date: Sat Jan 9 13:53:31 2016 +0300 0027058: AIS_ColorScale defines methods SetColor and SetWidth hiding inherited methods Interface of AIS_ColorScale is revised to make it more consistent: - Methods SetBgColor()/GetBGColor(), and corresponding field, are removed. It was used to select white or black color for the color bar frame (by contrast). That color can now be set explicitly by inherited method SetColor(). - Own methods Get/SetColor() are renamed to Get/SetIntervalColor(), to avoid confusion with inherited method SetColor() - Methods Get/SetWidth() are renamed to Get/SetBreadth(), to avoid confusion with inherited method SetWidth() - Method Get/Set for labels and colors, and DRAW command vcolorscale, now all accept index starting at 1 - Comments added to explain indexation rules |
|
|
Fix pushed to CR27058, please review. Note that the fix is focused on getting rid of confusion between own and inherited methods SetColor() and SetWidth(). I have tried to correct evident indexation errors (such as when method SetColor() is said to accept index starting at 1, but actually uzes 0-based index). However, the class obviously still has a number of design flaws that are worth to be addressed separately. At a minimum, the class must maintain its own integrity. This is not the case now; for instance, there is no checks for consistency between set number of intervals, "types" of label and color definitions, and lengths of corresponding sequences. It seems that any incautious setting may lead to inconsistent state of the class and hence unpredictable results. This OCAF-like style ("you must know how this class works inside to use it") must be avoided. Documentation should be improved, as well. For instance, it is not clear: - How enum Aspect_TypeOfColorScalePosition (with values LEFT, RIGHT, and CENTER) can define position of a title? - What is meaning of coordinates given to SetPosition()? Is it lower, upper, left, right, corner, center or whatever? |
|
|
Branch CR27058 has been updated forcibly by abv. SHA-1: b22db77b571d363aae035c5423c8182e89519e11 |
|
|
Please test the patch. |
|
|
Branch CR27058 has been deleted by kgv. SHA-1: b22db77b571d363aae035c5423c8182e89519e11 |
|
occt: master 180f89a2 2016-01-09 10:53:31
Committer: abv Details Diff |
0027058: AIS_ColorScale defines methods SetColor and SetWidth hiding inherited methods Interface of AIS_ColorScale is revised to make it more consistent: - Methods SetBgColor()/GetBGColor(), and corresponding field, are removed. It was used to select white or black color for the color bar frame (by contrast). That color can now be set explicitly by inherited method SetColor(). - Own methods Get/SetColor() are renamed to Get/SetIntervalColor(), to avoid confusion with inherited method SetColor() - Methods Get/SetWidth() are renamed to Get/SetBreadth(), to avoid confusion with inherited method SetWidth() - Method Get/Set for labels and colors, and DRAW command vcolorscale, now all accept index starting at 1 - Comments added to explain indexation rules |
Affected Issues 0027058 |
|
| mod - src/AIS/AIS_ColorScale.cxx | Diff File | ||
| mod - src/AIS/AIS_ColorScale.hxx | Diff File | ||
| mod - src/ViewerTest/ViewerTest_ViewerCommands.cxx | Diff File | ||
| mod - tests/bugs/vis/bug25136 | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-01-04 09:50 |
|
New Issue | |
| 2016-01-04 09:50 |
|
Assigned To | => kgv |
| 2016-01-08 19:12 |
|
Relationship added | child of 0025076 |
| 2016-01-09 14:18 | git | Note Added: 0049754 | |
| 2016-01-09 14:36 |
|
Note Added: 0049755 | |
| 2016-01-09 14:36 |
|
Status | new => resolved |
| 2016-01-09 20:13 | git | Note Added: 0049760 | |
| 2016-01-09 20:48 | kgv | Note Added: 0049762 | |
| 2016-01-09 20:48 | kgv | Assigned To | kgv => bugmaster |
| 2016-01-09 20:48 | kgv | Status | resolved => reviewed |
| 2016-01-09 20:49 | kgv | Relationship added | child of 0025785 |
| 2016-01-15 16:57 |
|
Changeset attached | => occt master 180f89a2 |
| 2016-01-15 16:57 |
|
Assigned To | bugmaster => abv |
| 2016-01-15 16:57 |
|
Status | reviewed => verified |
| 2016-01-15 16:57 |
|
Resolution | open => fixed |
| 2016-04-17 13:54 | git | Note Added: 0053008 | |
| 2016-04-20 15:42 |
|
Fixed in Version | => 7.0.0 |
| 2016-04-20 15:50 |
|
Status | verified => closed |
