MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0027058Open CASCADE[OCCT] OCCT:Visualizationpublic2016-01-04 09:502016-04-20 15:50
Reporterabv 
Assigned Toabv 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version[OCCT] 7.0.0 
Target Version[OCCT] 7.0.0Fixed in Version[OCCT] 7.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
Attached Files

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

-  Notes
(0049754)
git (administrator)
2016-01-09 14:18

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
(0049755)
abv (manager)
2016-01-09 14:36

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?

(0049760)
git (administrator)
2016-01-09 20:13

Branch CR27058 has been updated forcibly by abv.

SHA-1: b22db77b571d363aae035c5423c8182e89519e11
(0049762)
kgv (developer)
2016-01-09 20:48

Please test the patch.
(0053008)
git (administrator)
2016-04-17 13:54

Branch CR27058 has been deleted by kgv.

SHA-1: b22db77b571d363aae035c5423c8182e89519e11

- Related Changesets
occt: master 180f89a2
Timestamp: 2016-01-09 10:53:31
Author: 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
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


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker