MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0023501Community[OCCT] OCCT:Visualizationpublic2012-10-30 18:012013-12-19 13:55
ReporterPawel 
Assigned ToPawel 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformWindowsOSVC++ 2008OS Version32 bit
Product Version[OCCT] 6.5.3 
Target Version[OCCT] 6.7.0Fixed in Version[OCCT] 6.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
Attached Files

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

-  Notes
(0022022)
Pawel (developer)
2012-10-30 18:11

Branch CR23501 pushed. Please review.
(0022091)
abv (manager)
2012-11-02 18:29

It seems the fix is in branch CR23200, not CR23501
(0022092)
Pawel (developer)
2012-11-02 22:17

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.
(0024499)
Pawel (developer)
2013-05-23 18:47

I have found similar problems in AIS package and corrected them in branch CR23501_2. Ready to review.
(0024518)
san (developer)
2013-05-27 10:24

Branch CR23501_2 reviewed without remarks, ready for testing.
(0024533)
Pawel (developer)
2013-05-28 14:07

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

Please review.
Thank you.
(0024552)
san (developer)
2013-05-29 18:49
edited on: 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.

(0024553)
Pawel (developer)
2013-05-29 20:15

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.
(0024554)
san (developer)
2013-05-29 22:46

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.
(0024617)
mkv (tester)
2013-06-03 20:27

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
Timestamp: 2013-06-06 06:13:30
Author: 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
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 View Revisions
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 View Revisions
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


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker