View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0023501CommunityOCCT:Visualizationpublic2012-10-30 18:012013-12-19 13:55
ReporterPawel Assigned ToPawel  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2008 
Product Version6.5.3 
Target Version6.7.0Fixed in Version6.7.0 
Summary0023501: Redundant triangulation cleaning/generation (1) in AIS_Shape.cxx
DescriptionIn AIS_Shape.cxx, starting in line 241, the code:

      if (OwnDeviationAngle(newangle,prevangle) ||
          OwnDeviationCoefficient(newcoeff,prevcoeff))
        if (Abs (newangle - prevangle) > Precision::Angular() ||
            Abs (newcoeff - prevcoeff) > Precision::Confusion() ) {
          BRepTools::Clean(myshape);
        }

will clean the triangulation even if:
(1) OwnDeviationAngle is false, Abs (newangle - prevangle) > Precision::Angular()
(2) OwnDeviationCoefficient is false, Abs (newcoeff - prevcoeff) > Precision::Confusion()

which I think was not intended.

I guess it makes sense to differentiate between the two cases: (1)OwnDeviationAngle is true, (2)OwnDeviationCoefficient is true.
Steps To ReproduceDraw reproducer:

box b 10 20 30
vinit
vsetshading b 0.001 #!!! Take the standard coefficient value for the drawer so that no change takes effect !!!
vdisplay b
vsetdispmode b 1 #Triangulation cleared

=====================
or directly in C++:

TopoDS_Shape box = BRepPrimAPI_MakeBox(gp_Pnt(0, 0, 0), gp_Pnt(10, 30, 20));
Handle (AIS_InteractiveObject) AISshape = new AIS_Shape(box);
myAISContext->Display(AISshape, Standard_False);
myAISContext->SetDeviationCoefficient(AISshape, myAISContext->DeviationCoefficient(), Standard_False);
myAISContext->SetDisplayMode(AISshape, AIS_Shaded, Standard_False);
myAISContext->UpdateCurrentViewer();
TagsNo tags attached.
Test case numberNot needed

Relationships

related to 0023200 closedbugmaster Visualization - prevent multiple triangulating of a shape that already has been triangulated 

Activities

Pawel

2012-10-30 18:11

developer   ~0022022

Branch CR23501 pushed. Please review.

abv

2012-11-02 18:29

manager   ~0022091

It seems the fix is in branch CR23200, not CR23501

Pawel

2012-11-02 22:17

developer   ~0022092

No. Those are two separate problems/fixes.

Actually this one (CR23501) is a bugfix. It avoids cleaning and then recomputing the triangulation if not necessary.

CR23200 is a workaround (it is not a general solution and I would NOT recommend including it in OCCT). It simply kills the triangulation code because it gets called automatically through XDE.

I hope this clarifies the case. If not I can elaborate on these issues.

Pawel

2013-05-23 18:47

developer   ~0024499

I have found similar problems in AIS package and corrected them in branch CR23501_2. Ready to review.

san

2013-05-27 10:24

developer   ~0024518

Branch CR23501_2 reviewed without remarks, ready for testing.

Pawel

2013-05-28 14:07

developer   ~0024533

Again, I have found the same problem in XCAFPrs_AISObject.cxx and pushed the solution using branch CR23501_3.

Please review.
Thank you.

san

2013-05-29 18:49

developer   ~0024552

Last edited: 2013-05-29 18:50

 Branch CR23501_3 reviewed without remarks, ready for testing.

NB for Pawel: don't you think that even after your patch the triangulation will be cleaned each time AIS_Shape::Compute() is called? Once a non-default deviation coefficient is set, newcoeff and prevcoeff will always differ.

Pawel

2013-05-29 20:15

developer   ~0024553

Hello Sergey,

the point of this patch is to SEPARATE the flags 'OwnDeviationAngle' and 'OwnDeviationCoefficient' because if one of them is set and the other NOT then OwnDeviationAngle/OwnDeviationCoefficient always returns 0 for the previous value if the 'Own' flag is not set. Thus '0' is different than the 'new value' and so the condition is always true.

Hope it clarifies.

san

2013-05-29 22:46

developer   ~0024554

Dear Pawel,

Yes, that is clear, my concern is that it still does not work as expected (triangulation is still cleaned when it should not be) so we will need to take another step in the same direction.
And this particular patch is OK, it does exactly what it is meant to do.

mkv

2013-06-03 20:27

tester   ~0024617

Dear BugMaster,

Branch CR23501_3 (and products from GIT master) was compiled on Linux and Windows platforms and tested.
SHA-1: 01d8a09cedb65a88c6c425b0432f390c983054b2

Number of compiler warnings:

occt component :
Linux: 2 (2 on master)
Windows: 11 (11 on master)

products component :
Linux: 0 (0 on master)
Windows: 63 (63 on master)

Regressions:
No regressions

Improvements:
No improvements

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 366240644 / 366080260
Total CPU difference: 41852.88000000145 / 44999.77000000119

Testing on Windows:
Total MEMORY difference: 420103112 / 417291612
Total CPU difference: 32347.796875 / 35719.34375

There are not differences in images found by testdiff.

Related Changesets

occt: master bbf847ad

2013-06-06 06:13:30

Pawel

Details Diff		 0023501: Redundant triangulation cleaning/generation (1) in AIS_Shape.cxx
Cleaning triangulation only if the AIS_Shape has either (1) OwnDeviationAngle and
the values 'newangle' and 'prevangle' are different or (2) OwnDeviationCoefficient and the values 'newcoeff' and 'prevcoeff' are different
Found similar problems in further code portions and corrected them.
The same faulty condition found in XCAFPrs_AISObject.cxx		 Affected Issues
0023501
mod - src/AIS/AIS_Shape.cxx Diff File
mod - src/AIS/AIS_TexturedShape.cxx Diff File
mod - src/XCAFPrs/XCAFPrs_AISObject.cxx Diff File

Issue History

Date Modified Username Field Change
2012-10-30 18:01 Pawel New Issue
2012-10-30 18:01 Pawel Assigned To => Pawel
2012-10-30 18:11 Pawel Note Added: 0022022
2012-10-30 18:11 Pawel Assigned To Pawel => bugmaster
2012-10-30 18:11 Pawel Status new => resolved
2012-11-02 17:45 Pawel Relationship added related to 0023200
2012-11-02 18:28 abv Assigned To bugmaster => san
2012-11-02 18:29 abv Note Added: 0022091
2012-11-02 22:17 Pawel Note Added: 0022092
2012-11-06 16:08 Pawel Steps to Reproduce Updated
2013-02-26 18:32 san Target Version 6.6.0 => 6.7.0
2013-05-23 18:47 Pawel Note Added: 0024499
2013-05-27 10:24 san Note Added: 0024518
2013-05-27 10:24 san Assigned To san => bugmaster
2013-05-27 10:24 san Status resolved => reviewed
2013-05-28 14:07 Pawel Note Added: 0024533
2013-05-28 14:07 Pawel Assigned To bugmaster => san
2013-05-28 14:07 Pawel Status reviewed => feedback
2013-05-29 18:49 san Note Added: 0024552
2013-05-29 18:49 san Assigned To san => bugmaster
2013-05-29 18:49 san Status feedback => reviewed
2013-05-29 18:50 san Note Edited: 0024552
2013-05-29 20:15 Pawel Note Added: 0024553
2013-05-29 22:46 san Note Added: 0024554
2013-05-30 15:31 bugmaster Assigned To bugmaster => mkv
2013-06-03 20:27 mkv Note Added: 0024617
2013-06-03 20:28 mkv Test case number => Not needed
2013-06-03 20:28 mkv Assigned To mkv => bugmaster
2013-06-03 20:28 mkv Status reviewed => tested
2013-06-10 10:30 Pawel Changeset attached => occt master bbf847ad
2013-06-10 10:30 Pawel Assigned To bugmaster => Pawel
2013-06-10 10:30 Pawel Status tested => verified
2013-06-10 10:30 Pawel Resolution open => fixed
2013-12-19 13:53 bugmaster Status verified => closed
2013-12-19 13:55 bugmaster Fixed in Version => 6.7.0