MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0025534Community[OCCT] OCCT:Application Frameworkpublic2014-11-28 14:192017-01-13 19:24
ReporterVico Liang 
Assigned Tobugmaster 
PrioritynormalSeverityminor 
StatusclosedResolutionfixed 
PlatformWindowsOSVC++ 2013OS Version64 bit
Product Version[OCCT] 6.8.0 
Target Version[OCCT] 7.1.0Fixed in Version[OCCT] 7.1.0 
Summary0025534: TObj_Application unicode path issue.
DescriptionStandard_Boolean TObj_Application::LoadDocument
                        (const char* theSourceFile,
                         Handle(TDocStd_Document)& theTargetDoc)
{
  myIsError = Standard_False;
  TCollection_ExtendedString aPath ((const Standard_CString)theSourceFile); //BUG: the fix can be: TCollection_ExtendedString aPath ((const Standard_CString)theSourceFile, Standard_True);
  ...
}

Standard_Boolean TObj_Application::SaveDocument
                        (const Handle(TDocStd_Document)& theSourceDoc,
                         const char* theTargetFile)
{
  myIsError = Standard_False;
  TCollection_ExtendedString aPath ((const Standard_CString)theTargetFile); // BUG: the fix can be: TCollection_ExtendedString aPath ((const Standard_CString)theSourceFile, Standard_True);
  ...
}

Steps To ReproduceNot required. The fix is just change of API.
TagsNo tags attached.
Test case numberNot needed
Attached Files

- Relationships
parent of 0028039closedapn Open CASCADE Coding rules, TObj - pass file name by reference 
has duplicate 0025535closedbugmaster Community Revise the TObj model to better support Unicode path. 
related to 0028172newkgv Community Replace Standard_CString file path with Unicode form TCollection_ExtendedString 
child of 0022484closedbugmaster Open CASCADE UNICODE characters support. 

-  Notes
(0056882)
git (administrator)
2016-08-19 17:57

Branch CR25534 has been created by mpv.

SHA-1: f74b149aa0b4de513694b3020b6b1a4cc84c5a04


Detailed log of new commits:

Author: mpv
Date: Fri Aug 19 17:56:09 2016 +0300

    0027558: restore support for reading legacy XCAF persistence format. Changed API to support extended strings file paths both internally and externally by TObj_Model and TObj_Application.
(0056883)
mpv (developer)
2016-08-19 18:03

I removed "char*" API from Model and Application classes of TObj. So, this fixes the request of Vico in more general way (now it is the specific application part now to check the conversion from its representation of file path to the ExtendedString argument).
Also this fix removes the necessity to convert from ExtendedString to Acscii string and vice versa in many cases. So, it becomes simpler.
(0056899)
Vico Liang (developer)
2016-08-20 15:54

Thank you mpv, that's a big improvement.
(0057066)
szy (administrator)
2016-08-25 15:46

Reviewed.
(0057075)
git (administrator)
2016-08-25 17:25

Branch CR25534 has been updated forcibly by mkv.

SHA-1: ff54f4314d3456cbd70486324488e55e8301f7ea
(0057104)
mkv (tester)
2016-08-26 15:34

Dear BugMaster,
Branch CR25534 was rebased on IR-2016-08-25 of occt git-repository.
SHA-1: ff54f4314d3456cbd70486324488e55e8301f7ea
(0057105)
mkv (tester)
2016-08-26 15:35

Dear BugMaster,
Branch CR25534 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: ff54f4314d3456cbd70486324488e55e8301f7ea

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 64 (64 on master)
Windows: 0 (0 on master)
MacOS : 1155

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 89966944 / 89981040 [-0.02%]
Total CPU difference: 19210.07999999999 / 19309.729999999934 [-0.52%]
products component :
Total MEMORY difference: 30010832 / 30015953 [-0.02%]
Total CPU difference: 5079.109999999979 / 5043.729999999974 [+0.70%]

Testing on Windows:
occt component :
Total MEMORY difference: 57099450 / 57107084 [-0.01%]
Total CPU difference: 18181.089744698817 / 18053.73052829886 [+0.71%]
products component :
Total MEMORY difference: 21259115 / 21223623 [+0.17%]
Total CPU difference: 4904.562239299948 / 4857.465537399941 [+0.97%]

There are no differences in images found by testdiff.
(0057106)
mkv (tester)
2016-08-26 15:35

Dear BugMaster,
Branch CR25534 is TESTED
(0057379)
kgv (developer)
2016-09-01 13:50

Dear mpv,

--- a/src/TObj/TObj_Model.hxx
+++ b/src/TObj/TObj_Model.hxx
   //! Load the OCAF model from a file
-  virtual Standard_EXPORT Standard_Boolean Load (const char* theFile);
+  virtual Standard_EXPORT Standard_Boolean Load (const TCollection_ExtendedString theFile);
 
   //! Save the model to a file
-  virtual Standard_EXPORT Standard_Boolean SaveAs (const char* theFile);
+  virtual Standard_EXPORT Standard_Boolean SaveAs (const TCollection_ExtendedString theFile);


the breaking changes in public API should be documented in dox/dev_guides/upgrade/upgrade.md
(0057393)
git (administrator)
2016-09-01 17:04

Branch CR25534 has been updated by mpv.

SHA-1: 4217e9a99fc8a50cd1ba54c226c2a3b3fe641f29


Detailed log of new commits:

Author: mpv
Date: Thu Sep 1 17:04:36 2016 +0300

    0025534: Document the upgrade of TObj_Model API.

(0057394)
mpv (developer)
2016-09-01 17:05

Documentation is updated.
(0057395)
kgv (developer)
2016-09-01 17:11

+Methods *TObj_Model::SaveAs* and *TObj_Model::Load* receive *TCollection_ExtendedString* filename arguments 
instead of char*. This allows to manage non-ASCII symbols explicitly from the application

Description is confusing from my point of view.
All other file APIs taking "const char*" / TCollection_AsciiString in OCCT consider them in UTF-8 (and asking user to pass UTF-16 file name on UNIX platforms sounds like a redundant effort).
(0057398)
git (administrator)
2016-09-01 17:32

Branch CR25534 has been updated by mpv.

SHA-1: 7dd6dad67bbcb74c2f8af5bad68cb3d1ea7bfd8a


Detailed log of new commits:

Author: mpv
Date: Thu Sep 1 17:34:21 2016 +0300

    0025534: update the description due to kgv feedback.

(0057399)
mpv (developer)
2016-09-01 17:32

In OCAF it is normally TCollection_ExtendedString (for an example TDocStd_Application::SaveAs - similar to this method). So, this change makes API conform.
Also internally it converted to ExtendedString the input char* anyway. So, this change clarifies what it is allowed in the argument, since char* (and even TCollection_AsciiString) has no notion of UTF support.

The description is updated.
(0057407)
kgv (developer)
2016-09-02 11:14
edited on: 2016-09-02 11:16

   //! Check whether the document contains the OCAF data.
   //! This implementation checks theFile on NULL only.
-  Standard_EXPORT virtual Standard_Boolean checkDocumentEmpty (const char* theFile);
+  Standard_EXPORT virtual Standard_Boolean 
+    checkDocumentEmpty(const TCollection_ExtendedString theFile);
...
    Handle(TObjDRAW_Model) aModel = new TObjDRAW_Model();
    aModel->Load(0);

Please update description of the method and places where modified methods have been used (e.g. TObjDRAW.cxx).

-  Standard_EXPORT virtual Standard_Boolean checkDocumentEmpty (const char* theFile);
+  Standard_EXPORT virtual Standard_Boolean 
+    checkDocumentEmpty(const TCollection_ExtendedString theFile);
...
   //! Load the OCAF model from a file
-  virtual Standard_EXPORT Standard_Boolean Load (const char* theFile);
+  virtual Standard_EXPORT Standard_Boolean Load (const TCollection_ExtendedString theFile);
 
   //! Save the model to a file
-  virtual Standard_EXPORT Standard_Boolean SaveAs (const char* theFile);
+  virtual Standard_EXPORT Standard_Boolean SaveAs (const TCollection_ExtendedString theFile);
...
   //! Saving the OCAF document to a file
   virtual Standard_EXPORT Standard_Boolean SaveDocument
-                         (const Handle(TDocStd_Document)& theSourceDoc,
-                          const char*                     theTargetFile);
+                         (const Handle(TDocStd_Document)&  theSourceDoc,
+                          const TCollection_ExtendedString theTargetFile);
 
   //! Loading the OCAF document from a file
   virtual Standard_EXPORT Standard_Boolean LoadDocument
-                         (const char*                     theSourceFile,
-                          Handle(TDocStd_Document)&       theTargetDoc);
+                         (const TCollection_ExtendedString theSourceFile,
+                          Handle(TDocStd_Document)&        theTargetDoc);

It is preferred to pass TCollection_ExtendedString by reference (TCollection_ExtendedString&).

0027558: restore support for reading legacy XCAF persistence format. Changed API to support extended 
strings file paths both internally and externally by TObj_Model and TObj_Application.

Commit description does not match bug description - please create a new branch with properly defined description.

Note that file path passed as const char* to methods load/save will be implicitly converted to TCollection_ExtendedString considering multi-byte flag is not set (TCollection_ExtendedString() constructor has no "explicit" keyword).
So existing code will remain buggy without any notice from compiler.

But existing methods TDocStd_Application::Open() and TDocStd_Application::SaveAs() have the same issue.
I would say that it might be better changing TDocStd_Application to take TCollection_AsciiString for consistency with other file APIs in OCCT instead of changing TObj.
But this would be aggressive change.

Maybe it does not worth changing TObj API and just fix file name conversion inside the methods?

(0057456)
git (administrator)
2016-09-05 13:25

Branch CR25534_1 has been created by mpv.

SHA-1: 1c652a88b9313dccf95268f1cbd2751c0758ff1c


Detailed log of new commits:

Author: mpv
Date: Mon Sep 5 13:27:40 2016 +0300

    0025534: TObj_Application unicode path issue.
(0057457)
mpv (developer)
2016-09-05 13:27

Done: branch CR25534_1 is created, some comments are added/modified.
(0057470)
kgv (developer)
2016-09-05 16:40

Please test updated patch.
(0057506)
mkv (tester)
2016-09-06 12:58

Dear BugMaster,
Branch CR25534_1 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
SHA-1: 1c652a88b9313dccf95268f1cbd2751c0758ff1c

Number of compiler warnings:

occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

products component :
Linux: 64 (64 on master)
Windows: 0 (0 on master)
MacOS : 1132

Regressions/Differences/Improvements:
No regressions/differences

Testing cases:
Not needed

Testing on Linux:
occt component :
Total MEMORY difference: 89551574 / 89476793 [+0.08%]
Total CPU difference: 19277.29999999999 / 19241.370000000014 [+0.19%]
products component :
Total MEMORY difference: 30005204 / 29985574 [+0.07%]
Total CPU difference: 5078.389999999977 / 5115.309999999973 [-0.72%]

Testing on Windows:
occt component :
Total MEMORY difference: 57150435 / 57155154 [-0.01%]
Total CPU difference: 18272.66233169886 / 18057.115749998855 [+1.19%]
products component :
Total MEMORY difference: 21261562 / 21225231 [+0.17%]
Total CPU difference: 4893.23656669995 / 4849.119483899957 [+0.91%]

There are no differences in images found by testdiff.
(0057507)
mkv (tester)
2016-09-06 12:58

Dear BugMaster,
Branch CR25534_1 is TESTED.
(0058266)
Vico Liang (developer)
2016-09-28 15:21

Dear mpv,

I see all interfaces are changed to "const TCollection_ExtendedString", it should be better to use "const TCollection_ExtendedString&" with reference version.

There are two reasons to do so:

firstly, reference should be more efficient since it's without temporary object when passing argument.

secondly, it aligns with interface inside TDocStd_Application, TDocStd_Application use reference version "const TCollection_ExtendedString&".
(0058282)
kgv (developer)
2016-09-28 18:17

Dear Vico,

unfortunately the patch has been already integrated, and within our bug advancement workflow we do not reopen bugs.

Therefore, if you want this to be corrected - register dedicated bug (and patches are welcome as well).
(0059544)
git (administrator)
2016-10-28 21:48

Branch CR25534 has been deleted by kgv.

SHA-1: 7dd6dad67bbcb74c2f8af5bad68cb3d1ea7bfd8a
(0059545)
git (administrator)
2016-10-28 21:48

Branch CR25534_1 has been deleted by kgv.

SHA-1: 1c652a88b9313dccf95268f1cbd2751c0758ff1c

- Related Changesets
occt: master 1f9fb707
Timestamp: 2016-09-05 10:27:40
Author: mpv
Committer: bugmaster
Details ] Diff ]
0025534: TObj_Application unicode path issue.
mod - dox/dev_guides/upgrade/upgrade.md Diff ] File ]
mod - src/TObj/TObj_Application.cxx Diff ] File ]
mod - src/TObj/TObj_Application.hxx Diff ] File ]
mod - src/TObj/TObj_Model.cxx Diff ] File ]
mod - src/TObj/TObj_Model.hxx Diff ] File ]
mod - src/TObjDRAW/TObjDRAW.cxx Diff ] File ]

- Issue History
Date Modified Username Field Change
2014-11-28 14:19 Vico Liang New Issue
2014-11-28 14:19 Vico Liang Assigned To => szy
2015-12-15 14:06 abv Target Version 7.0.0 => 7.1.0
2016-02-17 18:19 szy Assigned To szy => mpv
2016-08-19 17:57 git Note Added: 0056882
2016-08-19 18:03 mpv Note Added: 0056883
2016-08-19 18:03 mpv Assigned To mpv => szy
2016-08-19 18:03 mpv Status new => resolved
2016-08-19 18:03 mpv Steps to Reproduce Updated View Revisions
2016-08-20 15:54 Vico Liang Note Added: 0056899
2016-08-25 15:46 szy Note Added: 0057066
2016-08-25 15:46 szy Assigned To szy => bugmaster
2016-08-25 15:46 szy Status resolved => reviewed
2016-08-25 17:25 git Note Added: 0057075
2016-08-25 17:28 mkv Assigned To bugmaster => mkv
2016-08-26 15:34 mkv Note Added: 0057104
2016-08-26 15:35 mkv Note Added: 0057105
2016-08-26 15:35 mkv Note Added: 0057106
2016-08-26 15:35 mkv Assigned To mkv => bugmaster
2016-08-26 15:35 mkv Status reviewed => tested
2016-08-26 15:35 mkv Test case number => Not needed
2016-09-01 13:50 kgv Note Added: 0057379
2016-09-01 13:50 kgv Assigned To bugmaster => mpv
2016-09-01 13:50 kgv Status tested => assigned
2016-09-01 17:04 git Note Added: 0057393
2016-09-01 17:05 mpv Note Added: 0057394
2016-09-01 17:05 mpv Assigned To mpv => kgv
2016-09-01 17:05 mpv Status assigned => resolved
2016-09-01 17:11 kgv Note Added: 0057395
2016-09-01 17:32 git Note Added: 0057398
2016-09-01 17:32 mpv Note Added: 0057399
2016-09-02 11:14 kgv Note Added: 0057407
2016-09-02 11:14 kgv Assigned To kgv => mpv
2016-09-02 11:14 kgv Status resolved => assigned
2016-09-02 11:15 kgv Note Edited: 0057407 View Revisions
2016-09-02 11:16 kgv Note Edited: 0057407 View Revisions
2016-09-05 13:25 git Note Added: 0057456
2016-09-05 13:27 mpv Note Added: 0057457
2016-09-05 13:27 mpv Assigned To mpv => kgv
2016-09-05 13:27 mpv Status assigned => resolved
2016-09-05 16:40 kgv Note Added: 0057470
2016-09-05 16:40 kgv Assigned To kgv => bugmaster
2016-09-05 16:40 kgv Status resolved => reviewed
2016-09-05 17:49 mkv Assigned To bugmaster => mkv
2016-09-06 12:58 mkv Note Added: 0057506
2016-09-06 12:58 mkv Note Added: 0057507
2016-09-06 12:58 mkv Assigned To mkv => bugmaster
2016-09-06 12:58 mkv Status reviewed => tested
2016-09-09 09:40 bugmaster Changeset attached => occt master 1f9fb707
2016-09-09 09:40 bugmaster Status tested => verified
2016-09-09 09:40 bugmaster Resolution open => fixed
2016-09-28 15:21 Vico Liang Note Added: 0058266
2016-09-28 18:17 kgv Note Added: 0058282
2016-10-28 21:48 git Note Added: 0059544
2016-10-28 21:48 git Note Added: 0059545
2016-11-02 12:44 kgv Relationship added related to 0028039
2016-11-02 12:44 kgv Relationship replaced parent of 0028039
2016-11-02 12:57 kgv Relationship added parent of 0022484
2016-11-02 12:58 kgv Relationship replaced child of 0022484
2016-11-29 08:44 kgv Relationship added related to 0028172
2016-12-09 16:30 user533 Status verified => closed
2016-12-09 16:39 user533 Fixed in Version => 7.1.0
2017-01-13 19:24 kgv Relationship added has duplicate 0025535


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker