View Issue Details

IDProjectCategoryView StatusLast Update
0029372CommunityOCCT:Visualizationpublic2018-06-29 21:19
ReporterVico Liang Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2013 
Product Version7.1.0 
Target Version7.3.0Fixed in Version7.3.0 
Summary0029372: Graphic3d_TransformPers - improve description of Local Coordinate system defined by Transformation Persistence
DescriptionFor Graphic3d_TMF_RotatePers and Graphic3d_TMF_ZoomPers options, There is a bug in method below:

void Graphic3d_TransformPers::Apply (const Handle(Graphic3d_Camera)& theCamera,
                                     const NCollection_Mat4<T>& theProjection,
                                     NCollection_Mat4<T>& theWorldView,
                                     const Standard_Integer theViewportWidth,
                                     const Standard_Integer theViewportHeight) const
{
  ...
  else
  {
    // Compute reference point for transformation in untransformed projection space.
    NCollection_Mat4<Standard_Real> aWorldView = theCamera->OrientationMatrix();
    Graphic3d_TransformUtils::Translate (aWorldView, myParams.Params3d.PntX, myParams.Params3d.PntY, myParams.Params3d.PntZ);
    if ((myMode & Graphic3d_TMF_RotatePers) != 0)
    {
      // lock rotation by nullifying rotation component
      aWorldView.SetValue (0, 0, 1.0);
      aWorldView.SetValue (1, 0, 0.0);
      aWorldView.SetValue (2, 0, 0.0);

      aWorldView.SetValue (0, 1, 0.0);
      aWorldView.SetValue (1, 1, 1.0);
      aWorldView.SetValue (2, 1, 0.0);

      aWorldView.SetValue (0, 2, 0.0);
      aWorldView.SetValue (1, 2, 0.0);
      aWorldView.SetValue (2, 2, 1.0);
    }

    if ((myMode & Graphic3d_TMF_ZoomPers) != 0)
    {
      // lock zooming
      gp_Vec aVecToEye (theCamera->Direction());
      gp_Vec aVecToObj (theCamera->Eye(), gp_Pnt (myParams.Params3d.PntX, myParams.Params3d.PntY, myParams.Params3d.PntZ));
      const Standard_Real aFocus = aVecToObj.Dot (aVecToEye);
      const gp_XYZ aViewDim = theCamera->ViewDimensions (aFocus);
      const Standard_Real aScale = Abs(aViewDim.Y()) / Standard_Real(aVPSizeY);
      Graphic3d_TransformUtils::Scale (aWorldView, aScale, aScale, aScale);
    }
    
       // VICO FIX: Move the object back after apply rotation or scale operation
       Graphic3d_TransformUtils::Translate (aWorldView, -myParams.Params3d.PntX, -myParams.Params3d.PntY, -myParams.Params3d.PntZ);

      theWorldView.ConvertFrom (aWorldView);
}
TagsNo tags attached.
Test case numberNot required

Attached Files

  • TransformPers_MovedAwayFromAnchor.png (18,078 bytes)

Relationships

related to 0029413 closedkgv TransformPers won't work for Aspect_TOTP_LEFT_UPPER 

Activities

Vico Liang

2017-12-05 17:52

developer  

TransformPers_MovedAwayFromAnchor.png (18,078 bytes)

san

2017-12-06 14:57

developer   ~0072737

Dear Vico,

Can you please add a couple of snapshots or sketches that illustrate the correct behavior of your sample presentation (an arrow) when teh scene is rotated, zoomed etc.?
Also we need to know the exact combination of Graphic3d_TransModeFlags you use in this example.
Many thanks in advance!

Vico Liang

2017-12-07 06:57

developer   ~0072743

Dear san,

Please see my fix by adding this line:
       // VICO FIX: Move the object back after apply rotation or scale operation
        Graphic3d_TransformUtils::Translate (aWorldView, -myParams.Params3d.PntX, -myParams.Params3d.PntY, -myParams.Params3d.PntZ);

It's coding bug that we should move the object back after zoom or rotation about the anchor point.

Vico Liang

2017-12-07 07:00

developer   ~0072744

I have just used Graphic3d_TMF_ZoomPers in my example. But it should be the same problem for rotation.

kgv

2017-12-07 07:30

developer   ~0072745

Last edited: 2017-12-07 07:31

Dear Vico,

I suppose you are misunderstanding how transformation persistence works.
The object cannot be locked for zoom/rotation and then just "moved back".

Anchor point defines the origin of Local coordinate system of the object relative to World coordinate system, and the object itself then should be defined relative to this origin.

There are plenty of tests covering transformation persistence functionality and your modification ruins them and breaks logic. Check, for example, a test case bugs/vis/bug26719_2 or this sample:
pload MODELING VISUALIZATION
box b1 20.3 20.3 20.3
box b2 20.3 20.3 20.3
vdisplay b1 -trsfPers zoom -trsfPersPos 200 200 200
vdisplay b2 -trsfPers zoom -trsfPersPos 200 200 200
vsetlocation b2 -20.3 -20.3 -20.3
vpoint p0 0 0 0
vpoint p200 200 200 200
vaxo
vfit


Vico Liang

2017-12-07 08:26

developer   ~0072747

Dear kgv,

Thank your for your helpful explanation. I misunderstood the usage of transformation persistence. From my point of view, the anchor point was the rotation center or zoom center, and i didn't know that it's also define a local coordinate system.

It wolud be helpful by adding what your described here to the document. There might be other users have the same question as me.

Thanks

git

2017-12-07 10:59

administrator   ~0072750

Branch CR29372 has been created by kgv.

SHA-1: 4a3ba6a365d2d64bccb212e9525ca065f81f8f40


Detailed log of new commits:

Author: kgv
Date: Thu Dec 7 10:57:26 2017 +0300

    0029372: Graphic3d_TransformPers - improve description of Local Coordinate system defined by Transformation Persistence

kgv

2017-12-07 10:59

developer   ~0072751

Patch is ready for review.

san

2017-12-07 11:59

developer   ~0072761

Branch CR29372 reviewed without remarks, ready for testing.

Vico Liang

2017-12-08 04:33

developer   ~0072844

It seems the descrition should be switched as below:

Graphic3d_TMF_TriedronPers and Graphic3d_TMF_2d define origin as 2D offset from screen corner in pixels.

while Graphic3d_TMF_ZoomPers, Graphic3d_TMF_RotatePers and Graphic3d_TMF_ZoomRotatePers define Local Coordinate system having origin in specified anchor point defined in World Coordinate system.

git

2017-12-08 09:23

administrator   ~0072845

Branch CR29372 has been updated by kgv.

SHA-1: 186e547129b9da071ee20f4f581e531d71879c99


Detailed log of new commits:

Author: kgv
Date: Fri Dec 8 09:23:06 2017 +0300

    # correct misprint in description

bugmaster

2017-12-08 13:43

administrator   ~0072852

Combination -
OCCT branch : CR29372 SHA - 4a3ba6a365d2d64bccb212e9525ca065f81f8f40
Products branch : master SHA - 415c5096b9b013d1fad1454581bcdbe1e6090b5d
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian70-64:
OCCT
Total CPU difference: 18446.08999999967 / 18469.379999999688 [-0.13%]
Products
Total CPU difference: 7445.010000000002 / 7457.620000000003 [-0.17%]
Windows-64-VC10:
OCCT
Total CPU difference: 17806.593744098558 / 17828.96428749855 [-0.13%]
Products
Total CPU difference: 8016.969390499966 / 8009.6685436999705 [+0.09%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2017-12-11 12:01

administrator   ~0072901

Branch CR29372 has been deleted by kgv.

SHA-1: 186e547129b9da071ee20f4f581e531d71879c99

Related Changesets

occt: master 67160f4e

2017-12-07 07:57:26

kgv


Committer: bugmaster Details Diff
0029372: Graphic3d_TransformPers - improve description of Local Coordinate system defined by Transformation Persistence Affected Issues
0029372
mod - src/Graphic3d/Graphic3d_TransformPers.hxx Diff File
mod - src/PrsMgr/PrsMgr_PresentableObject.hxx Diff File

Issue History

Date Modified Username Field Change
2017-12-05 17:52 Vico Liang New Issue
2017-12-05 17:52 Vico Liang Assigned To => kgv
2017-12-05 17:52 Vico Liang File Added: TransformPers_MovedAwayFromAnchor.png
2017-12-06 14:57 san Note Added: 0072737
2017-12-06 14:57 san Assigned To kgv => Vico Liang
2017-12-06 14:57 san Status new => feedback
2017-12-07 06:57 Vico Liang Note Added: 0072743
2017-12-07 06:57 Vico Liang Assigned To Vico Liang =>
2017-12-07 06:58 Vico Liang Assigned To => san
2017-12-07 06:58 Vico Liang Status feedback => assigned
2017-12-07 07:00 Vico Liang Note Added: 0072744
2017-12-07 07:30 kgv Note Added: 0072745
2017-12-07 07:31 kgv Note Edited: 0072745
2017-12-07 07:31 kgv Status assigned => feedback
2017-12-07 07:31 kgv Note Edited: 0072745
2017-12-07 08:26 Vico Liang Note Added: 0072747
2017-12-07 08:45 kgv Assigned To san => kgv
2017-12-07 08:45 kgv Severity major => minor
2017-12-07 08:45 kgv Status feedback => assigned
2017-12-07 08:45 kgv Summary TransformPers will move the object away from its anchor point. => Graphic3d_TransformPers - improve description of Local Coordinate system defined by Transformation Persistence
2017-12-07 10:59 git Note Added: 0072750
2017-12-07 10:59 kgv Note Added: 0072751
2017-12-07 10:59 kgv Assigned To kgv => san
2017-12-07 10:59 kgv Status assigned => resolved
2017-12-07 10:59 kgv Target Version 7.4.0 => 7.3.0
2017-12-07 11:59 san Note Added: 0072761
2017-12-07 11:59 san Assigned To san => bugmaster
2017-12-07 11:59 san Status resolved => reviewed
2017-12-08 04:33 Vico Liang Note Added: 0072844
2017-12-08 09:23 git Note Added: 0072845
2017-12-08 13:42 bugmaster Test case number => Not required
2017-12-08 13:43 bugmaster Note Added: 0072852
2017-12-08 13:43 bugmaster Status reviewed => tested
2017-12-08 23:00 bugmaster Changeset attached => occt master 67160f4e
2017-12-08 23:00 bugmaster Status tested => verified
2017-12-08 23:00 bugmaster Resolution open => fixed
2017-12-11 12:01 git Note Added: 0072901
2018-01-09 17:51 kgv Relationship added related to 0029413
2018-06-29 21:15 aiv Fixed in Version => 7.3.0
2018-06-29 21:19 aiv Status verified => closed