MantisBT - Open CASCADE
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0031703Open CASCADE[OCCT] OCCT:Data Exchangepublic2020-08-10 16:172020-12-25 13:49
Reporterkgv 
Assigned Tobugmaster 
PrioritynormalSeverityfeature 
StatusverifiedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.6.0*Fixed in Version 
Test case numberde_mesh/gltf_write/helmetglb,lanternglb
Summary0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
DescriptionCurrently, binary glTF (glb) export preserves texture images as external file references. It is desired providing an option which would put textures inside glb file.
Steps To Reproduce
pload XDE OCAF VISUALIZATION
ReadGltf D Lantern.glb
vinit View
XDisplay -dispMode 1 D
vfit
WriteGltf D LanternWithTextures.glb -texturesInside
Additional information
and documentation updates
TagsNo tags attached.
Relationships
duplicate of 0030953closed bugmaster Open CASCADE Data Exchange - implement export of mesh data into glTF 2.0 format 
Attached Files? Lantern.glb (9,872,848) 2020-11-17 17:45
https://tracker.dev.opencascade.org/
Issue History
Date ModifiedUsernameFieldChange
2020-08-10 16:17kgvNew Issue
2020-08-10 16:17kgvAssigned To => kgv
2020-08-10 16:21kgvAssigned Tokgv => mkrylova
2020-08-10 16:21kgvStatusnew => assigned
2020-08-10 16:22kgvRelationship addedduplicate of 0030953
2020-09-18 12:04kgvTarget Version7.5.0 => 7.6.0*
2020-11-17 17:39gitNote Added: 0096837
2020-11-17 17:45mkrylovaFile Added: Lantern.glb
2020-11-17 17:46mkrylovaAssigned Tomkrylova => gka
2020-11-17 17:46mkrylovaStatusassigned => resolved
2020-11-17 17:46mkrylovaSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=24024#r24024
2020-11-17 17:47mkrylovaAssigned Togka => mkrylova
2020-11-17 17:47mkrylovaStatusresolved => assigned
2020-11-17 17:48mkrylovaAssigned Tomkrylova => osa
2020-11-17 17:48mkrylovaStatusassigned => resolved
2020-11-17 18:06kgvNote Added: 0096841
2020-11-24 10:29kgvNote Added: 0097023
2020-11-24 10:30kgvAssigned Toosa => mkrylova
2020-11-24 10:30kgvStatusresolved => assigned
2020-11-24 10:32kgvNote Edited: 0096841bug_revision_view_page.php?bugnote_id=96841#r24063
2020-11-24 10:32kgvNote Edited: 0096841bug_revision_view_page.php?bugnote_id=96841#r24064
2020-11-24 16:54gitNote Added: 0097037
2020-11-24 17:04gitNote Added: 0097040
2020-11-25 15:45mkrylovaAssigned Tomkrylova => osa
2020-11-25 15:45mkrylovaStatusassigned => resolved
2020-11-27 16:28kgvNote Added: 0097111
2020-11-27 16:28kgvAssigned Toosa => mkrylova
2020-11-27 16:28kgvStatusresolved => assigned
2020-11-27 16:28kgvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=24078#r24078
2020-11-27 16:33kgvNote Edited: 0097111bug_revision_view_page.php?bugnote_id=97111#r24080
2020-11-27 16:34kgvNote Edited: 0097111bug_revision_view_page.php?bugnote_id=97111#r24081
2020-11-27 16:35kgvNote Edited: 0097111bug_revision_view_page.php?bugnote_id=97111#r24082
2020-11-27 16:35kgvNote Edited: 0097111bug_revision_view_page.php?bugnote_id=97111#r24083
2020-12-02 14:16gitNote Added: 0097234
2020-12-02 14:22gitNote Added: 0097235
2020-12-02 15:41gitNote Added: 0097246
2020-12-02 16:04gitNote Added: 0097249
2020-12-07 12:43gitNote Added: 0097408
2020-12-07 12:43mkrylovaAssigned Tomkrylova => osa
2020-12-07 12:43mkrylovaStatusassigned => resolved
2020-12-14 11:26gitNote Added: 0097568
2020-12-14 16:27gitNote Added: 0097577
2020-12-14 16:35gitNote Added: 0097578
2020-12-14 16:52kgvAssigned Toosa => kgv
2020-12-14 17:14gitNote Added: 0097583
2020-12-14 19:02gitNote Added: 0097589
2020-12-14 21:00kgvNote Added: 0097592
2020-12-14 21:00kgvAssigned Tokgv => bugmaster
2020-12-14 21:00kgvStatusresolved => reviewed
2020-12-14 21:01gitNote Added: 0097593
2020-12-19 14:48bugmasterNote Added: 0097741
2020-12-19 14:48bugmasterStatusreviewed => tested
2020-12-19 14:55bugmasterTest case number => de_mesh/gltf_write/helmetglb,lanternglb
2020-12-19 15:06bugmasterChangeset attached => occt master 6eb502b2
2020-12-19 15:06bugmasterStatustested => verified
2020-12-19 15:06bugmasterResolutionopen => fixed
2020-12-19 15:22gitNote Added: 0097779
2020-12-19 15:22gitNote Added: 0097783
2020-12-19 15:23gitNote Added: 0097796
2020-12-19 15:23gitNote Added: 0097797
2020-12-19 15:23gitNote Added: 0097800
2020-12-25 13:49kgvRelationship addedrelated to 0030548

Notes
(0096837)
git   
2020-11-17 17:39   
Branch CR31703 has been created by mkrylova.

SHA-1: c8a2bc3048c317b25a5dae4f5b572d648f9ee13a


Detailed log of new commits:

Author: mkrylova
Date: Fri Nov 6 11:33:58 2020 +0300

    0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
    
    - Added opportunity ti put textures inside GLB file
    - Added new flag to WriteGltf
(0096841)
kgv   
2020-11-17 18:06   
(edited on: 2020-11-24 10:32) 
+      theWriter->Key ("mimeType");
+      theWriter->String ("image/png");

Why all images are expected to be in PNG format?
Image_Texture::ProbeImageFileFormat() could be useful here.

@@ -301,8 +338,10 @@ bool RWGltf_CafWriter::Perform (const Handle(TDocStd_Document)& theDocument,
                                 
const TDF_LabelSequence& theRootLabels,
                                 const TColStd_MapOfAsciiString* theLabelFilter,
                                 const TColStd_IndexedDataMapOfStringString& theFileInfo,
-                                const Message_ProgressRange& theProgress)
+                                const Message_ProgressRange& theProgress,
+                                Standard_Boolean theIsTexturesInside)

The new flag should not be passed through Perform() parameter - it should be set via dedicated method.

+  Standard_Boolean                              myToPutTexturesInside;

New class field is undefined within constructor.
(0097023)
kgv   
2020-11-24 10:29    
+      std::ofstream aFileOut;
+      aBinFile.write ((const char*)aBuffer->Data(), aBuffer->Size());
+      aFileOut.close();

What is the purpose for unused aFileOut?


+        OSD_OpenStream (aFileIn, aCurrImageTexture->FilePath().ToCString(), std::ios::in | std::ios::binary);

+        aBuffer = new NCollection_Buffer(NCollection_BaseAllocator::CommonBaseAllocator(), aLen);
+      aBinFile.write ((const char*)aBuffer->Data(), aBuffer->Size());



This is counter-efficient always reading entire image file into temporary buffer. The usual way is reading file by chunks of reasonable size into temporary buffer / writing into output file.
(0097037)
git   
2020-11-24 16:54   
Branch CR31703 has been updated forcibly by mkrylova.

SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f
(0097040)
git   
2020-11-24 17:04   
Branch CR31703_1 has been created by mkrylova.

SHA-1: 57f252e34f124726827923f7e061e2c4ef5b8675


Detailed log of new commits:

Author: mkrylova
Date: Fri Nov 6 11:33:58 2020 +0300

    0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
    
    - Added opportunity ti put textures inside GLB file
    - Added new flag to WriteGltf
(0097111)
kgv   
2020-11-27 16:28   
(edited on: 2020-11-27 16:35) 
+  Standard_Boolean myToPutTexturesInside;//!< flag to write image textures into binary gltf format 
(.glb)

myToEmbedTexturesInGlb;

+  void SetTexturesInside (Standard_Boolean theToPutTexturesInside) { myToPutTexturesInside = theToPutTexturesInside; 
}

Getter is also expected.

+  myToPutTexturesInside (Standard_False)
...
+  Standard_Boolean isTexturesInside = Standard_False;

TRUE will be more natural default for GLB format (intended to pack all model data into single file).

+  if (myToPutTexturesInside)

The writer should handle this option only in combination with GLB output (embedding textures into text glTF format requires different approach with base64 encoding, which is another feature).

+  Standard_Integer aGltfImgBufferKey = myNbImages + 4;

What does +4 means?

   Standard_EXPORT void addImage (RWGltf_GltfOStreamWriter* theWriter,
                                  const Handle(Image_Texture)& theTexture,
-                                 Standard_Boolean& theIsStarted);
+                                 Standard_Boolean& theIsStarted,
+                                 Standard_Boolean theIsTexturesInside = Standard_False);

It looks more like a separate function rather an extension to existing function.

+      Handle(NCollection_Buffer) aBuffer = aCurrImageTexture->DataBuffer();
+      if (aBuffer.IsNull())
+      {

if (Handle(NCollection_Buffer) aBuffer = aCurrImageTexture->DataBuffer())
{}
else{}

+        for (Standard_Size anIdx = 0; anIdx < aLen; anIdx += 16)
+          aBuffer = new NCollection_Buffer(NCollection_BaseAllocator::CommonBaseAllocator(), aCurrSize);

Why re-creating a buffer in a loop?
It should be enough allocating a buffer of meaningful size like 4 KiB once.

+          aBinFile.write ((const char*)aBuffer->Data(), aBuffer->Size());
+          if (!aBinFile.good())

It is desired checking writing status after closing the file stream, as write() might fill in an internal buffer and flush data to real file later.

   RWGltf_GltfBufferView                         myBuffViewNorm;      //!< current buffer view with 
nodes normals
   RWGltf_GltfBufferView                         myBuffViewTextCoord; //!< current buffer view with 
nodes UV coordinates
   RWGltf_GltfBufferView                         myBuffViewInd;       //!< current buffer view with 
triangulation indexes
+  NCollection_Vector<RWGltf_GltfBufferView>     myBuffViewImages;    //!< current buffer views 
with images

Could logic writing images follow the same approach as writing vertex data?

+    if (std::find (myImageVector.begin(), myImageVector.end(), aMetallicRoughnessTexture) == myImageVector.end())

Please use structures like NCollection_IndexedDataMap for filtering duplicates.

+      std::string anImageFormat = "image/";

std::string looks unexpected here - please use TCollection_AsciiString if possible.
In fact, it is RWGltf_GltfMaterialMap that is currently supposed to track image duplicates and to write image files;
it might be reasonable reconsidering approach and moving logic writing a file with aggregated image buffer there as alternative to or extension of RWMesh_MaterialMap::CopyTexture().

+      anImageFormat += theTexture->ProbeImageFileFormat().ToCString();

Image_Texture::ProbeImageFileFormat() returns short names of image format, not mime-types.
Please consider adding a method converting short image format name into a registered or well-known mime-type when possible
(for instance, "image/vnd-ms.dds" is specified for DDS format in MSDN documentation, not "image/dds").
https://docs.microsoft.com/en-us/windows/win32/wic/dds-format-overview [^]
https://www.iana.org/assignments/media-types/media-types.xhtml#image [^]

According to glTF 2.0 specification, only "image/jpeg" and "image/png" are valid image formats without glTF extensions enabled:
https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/README.md#file-extensions-and-mime-types [^]

Real models might use images in other formats.
In future it would be desired properly implementing extensions like MSFT_texture_dds/EXT_texture_webp
and to convert textures into compatible formats using Image_AlienPixMap otherwise:
https://github.com/KhronosGroup/glTF/blob/master/extensions/2.0/Vendor/MSFT_texture_dds/README.md [^]
https://github.com/KhronosGroup/glTF/blob/master/extensions/2.0/Vendor/EXT_texture_webp/README.md [^]

But for the moment - please log a warning about potentially incompatible glTF file and keep writing images unmodified
(the issue is the same for existing glTF writer copying image files - hence, could be handled as a separate bug).
(0097234)
git   
2020-12-02 14:16   
Branch CR31703_1 has been updated forcibly by mkrylova.

SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad
(0097235)
git   
2020-12-02 14:22   
Branch CR31703_2 has been created by mkrylova.

SHA-1: 8432d0c34cd2e6b4a0e344b8aa276e599d50d4b8


Detailed log of new commits:

Author: mkrylova
Date: Fri Nov 6 11:33:58 2020 +0300

    0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
    
    - Added opportunity ti put textures inside GLB file
    - Added new flag to WriteGltf
(0097246)
git   
2020-12-02 15:41   
Branch CR31703_2 has been updated by mkrylova.

SHA-1: f73580183d2a0e579454acbc0c45bbea396240ee


Detailed log of new commits:

Author: mkrylova
Date: Wed Dec 2 15:43:44 2020 +0300

    kgv remarks:
     - fixed code style
(0097249)
git   
2020-12-02 16:04   
Branch CR31703_2 has been updated by mkrylova.

SHA-1: deb6d5dde952c8b80c7e37f9703615ee64b0de84


Detailed log of new commits:

Author: mkrylova
Date: Wed Dec 2 16:06:18 2020 +0300

    kgv remarks:
     - fixed code style
(0097408)
git   
2020-12-07 12:43   
Branch CR31703_3 has been created by mkrylova.

SHA-1: 48e29bfd178642608f77fc62eccaa5914399b68a


Detailed log of new commits:

Author: mkrylova
Date: Fri Nov 6 11:33:58 2020 +0300

    0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
    
    - Added opportunity to put textures inside GLB file
    - Added new flag to WriteGltf
(0097568)
git   
2020-12-14 11:26   
Branch CR31703_3 has been updated forcibly by kgv.

SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef
(0097577)
git   
2020-12-14 16:27   
Branch CR31703_4 has been created by kgv.

SHA-1: fb27f1c7aa721e78115f744dc090e125cb2f3f0f


Detailed log of new commits:

Author: mkrylova
Date: Fri Nov 6 11:33:58 2020 +0300

    0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references
    
    RWGltf_CafWriter::ToEmbedTexturesInGlb() - added option embedding textures
    into GLB file enabled by default.
    
    Image_Texture::MimeType() - added method returning MIME type based on image file format.
    Image_Texture::WriteImage() - added method writing image into stream.
    
    
    
    - Added opportunity to put textures inside GLB file
    - Added new flag to WriteGltf
(0097578)
git   
2020-12-14 16:35   
Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: f2efdc68040fd0741dcc9131c0f58887ef4c3c85
(0097583)
git   
2020-12-14 17:14   
Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: a2769e79e3ab2f0cfc7fcdb24494c8036d40a5b7
(0097589)
git   
2020-12-14 19:02   
Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: 9f6a511c086f91e2c55e3c22b916f09536a682cc
(0097592)
kgv   
2020-12-14 21:00   
Please raise the patch
- OCCT branch: CR31703_4.

http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31703_4-master-KGV/ [^]
(0097593)
git   
2020-12-14 21:01   
Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25
(0097741)
bugmaster   
2020-12-19 14:48   
Combination -
OCCT branch : IR-2020-12-18
master SHA - 04114fd201c20efe9fbe85f00bec9a99ae3747ad
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-12-18 SHA - 290e5c74e8fef71947cadf90acb8e43c81ed10a1
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17709.570000000054 / 17744.450000000124 [-0.20%]
Products
Total CPU difference: 12258.480000000121 / 12330.210000000125 [-0.58%]
Windows-64-VC14:
OCCT
Total CPU difference: 19272.796875 / 19429.1875 [-0.80%]
Products
Total CPU difference: 13712.859375 / 13755.5 [-0.31%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0097779)
git   
2020-12-19 15:22   
Branch CR31703_4 has been deleted by inv.

SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25
(0097783)
git   
2020-12-19 15:22   
Branch CR31703_3 has been deleted by inv.

SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef
(0097796)
git   
2020-12-19 15:23   
Branch CR31703_2 has been deleted by inv.

SHA-1: deb6d5dde952c8b80c7e37f9703615ee64b0de84
(0097797)
git   
2020-12-19 15:23   
Branch CR31703_1 has been deleted by inv.

SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad
(0097800)
git   
2020-12-19 15:23   
Branch CR31703 has been deleted by inv.

SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f