View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0025534CommunityOCCT:Application Frameworkpublic2014-11-28 14:192020-10-14 12:12
ReporterVico Liang Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2013 
Product Version6.8.0 
Target Version7.1.0Fixed in Version7.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

Relationships

parent of 0028039 closedapn Open CASCADE Coding rules, TObj - pass file name by reference 
has duplicate 0025535 closedbugmaster Community Revise the TObj model to better support Unicode path. 
related to 0028172 closedkgv Community Replace Standard_CString file path with Unicode form TCollection_ExtendedString 
related to 0025682 closedbugmaster Community Application Framework - TObj_Model::GetModelName() should store in document in unicode mode instead of ascii mode 
child of 0022484 closedbugmaster Open CASCADE UNICODE characters support. 

Activities

git

2016-08-19 17:57

administrator   ~0056882

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.

mpv

2016-08-19 18:03

developer   ~0056883

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.

Vico Liang

2016-08-20 15:54

developer   ~0056899

Thank you mpv, that's a big improvement.

szy

2016-08-25 15:46

manager   ~0057066

Reviewed.

git

2016-08-25 17:25

administrator   ~0057075

Branch CR25534 has been updated forcibly by mkv.

SHA-1: ff54f4314d3456cbd70486324488e55e8301f7ea

mkv

2016-08-26 15:34

tester   ~0057104

Dear BugMaster,
Branch CR25534 was rebased on IR-2016-08-25 of occt git-repository.
SHA-1: ff54f4314d3456cbd70486324488e55e8301f7ea

mkv

2016-08-26 15:35

tester   ~0057105

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.

mkv

2016-08-26 15:35

tester   ~0057106

Dear BugMaster,
Branch CR25534 is TESTED

kgv

2016-09-01 13:50

developer   ~0057379

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

git

2016-09-01 17:04

administrator   ~0057393

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.

mpv

2016-09-01 17:05

developer   ~0057394

Documentation is updated.

kgv

2016-09-01 17:11

developer   ~0057395 

+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).

git

2016-09-01 17:32

administrator   ~0057398

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.

mpv

2016-09-01 17:32

developer   ~0057399

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.

kgv

2016-09-02 11:14

developer   ~0057407

Last edited: 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?

git

2016-09-05 13:25

administrator   ~0057456

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.

mpv

2016-09-05 13:27

developer   ~0057457

Done: branch CR25534_1 is created, some comments are added/modified.

kgv

2016-09-05 16:40

developer   ~0057470

Please test updated patch.

mkv

2016-09-06 12:58

tester   ~0057506

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.

mkv

2016-09-06 12:58

tester   ~0057507

Dear BugMaster,
Branch CR25534_1 is TESTED.

Vico Liang

2016-09-28 15:21

developer   ~0058266

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&".

kgv

2016-09-28 18:17

developer   ~0058282

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).

git

2016-10-28 21:48

administrator   ~0059544

Branch CR25534 has been deleted by kgv.

SHA-1: 7dd6dad67bbcb74c2f8af5bad68cb3d1ea7bfd8a

git

2016-10-28 21:48

administrator   ~0059545

Branch CR25534_1 has been deleted by kgv.

SHA-1: 1c652a88b9313dccf95268f1cbd2751c0758ff1c

Related Changesets

occt: master 1f9fb707

2016-09-05 10:27:40

mpv


Committer: bugmaster Details Diff		 0025534: TObj_Application unicode path issue. Affected Issues
0025534
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
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
2016-09-02 11:16 kgv Note Edited: 0057407
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 aiv Status verified => closed
2016-12-09 16:39 aiv Fixed in Version => 7.1.0
2017-01-13 19:24 kgv Relationship added has duplicate 0025535
2020-10-14 12:12 kgv Relationship added related to 0025682