View Issue Details

IDProjectCategoryView StatusLast Update
0027058Open CASCADEOCCT:Visualizationpublic2016-04-20 15:50
ReporterabvAssigned Toabv 
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.0.0 
Target Version7.0.0Fixed in Version7.0.0 
Summary0027058: AIS_ColorScale defines methods SetColor and SetWidth hiding inherited methods
DescriptionAs 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 ReproduceCompile with CLang
TagsNo tags attached.
Test case number

Relationships

child of 0025076 closedabv Community Hidden overloaded virtual functions 
child of 0025785 closedski Open CASCADE Visualization - introduce AIS_ColorScale presentation for Color Scale 

Activities

git

2016-01-09 14:18

administrator   ~0049754

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

abv

2016-01-09 14:36

manager   ~0049755

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?

git

2016-01-09 20:13

administrator   ~0049760

Branch CR27058 has been updated forcibly by abv.

SHA-1: b22db77b571d363aae035c5423c8182e89519e11

kgv

2016-01-09 20:48

developer   ~0049762

Please test the patch.

git

2016-04-17 13:54

administrator   ~0053008

Branch CR27058 has been deleted by kgv.

SHA-1: b22db77b571d363aae035c5423c8182e89519e11

Related Changesets

occt: master 180f89a2

2016-01-09 10:53:31

abv


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

Issue History

Date Modified Username Field Change
2016-01-04 09:50 abv New Issue
2016-01-04 09:50 abv Assigned To => kgv
2016-01-08 19:12 abv Relationship added child of 0025076
2016-01-09 14:18 git Note Added: 0049754
2016-01-09 14:36 abv Note Added: 0049755
2016-01-09 14:36 abv 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 abv Changeset attached => occt master 180f89a2
2016-01-15 16:57 abv Assigned To bugmaster => abv
2016-01-15 16:57 abv Status reviewed => verified
2016-01-15 16:57 abv Resolution open => fixed
2016-04-17 13:54 git Note Added: 0053008
2016-04-20 15:42 aiv Fixed in Version => 7.0.0
2016-04-20 15:50 aiv Status verified => closed