View Issue Details

IDProjectCategoryView StatusLast Update
0031703Open CASCADEOCCT:Data Exchangepublic2020-12-25 13:49
Reporterkgv Assigned Tobugmaster  
PrioritynormalSeverityfeature 
Status closedResolutionfixed 
Target Version7.6.0Fixed in Version7.6.0 
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
TagsNo tags attached.
Test case numberde_mesh/gltf_write/helmetglb,lanternglb

Attached Files

  • Lantern.glb (9,872,848 bytes)

Relationships

duplicate of 0030953 closedbugmaster Open CASCADE Data Exchange - implement export of mesh data into glTF 2.0 format 

Activities

git

2020-11-17 17:39

administrator   ~0096837

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

mkrylova

2020-11-17 17:45

developer  

Lantern.glb (9,872,848 bytes)

kgv

2020-11-17 18:06

developer   ~0096841

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

kgv

2020-11-24 10:29

developer   ~0097023

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

git

2020-11-24 16:54

administrator   ~0097037

Branch CR31703 has been updated forcibly by mkrylova.

SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f

git

2020-11-24 17:04

administrator   ~0097040

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

kgv

2020-11-27 16:28

developer   ~0097111

Last edited: 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).

git

2020-12-02 14:16

administrator   ~0097234

Branch CR31703_1 has been updated forcibly by mkrylova.

SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad

git

2020-12-02 14:22

administrator   ~0097235

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

git

2020-12-02 15:41

administrator   ~0097246

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

git

2020-12-02 16:04

administrator   ~0097249

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

git

2020-12-07 12:43

administrator   ~0097408

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

git

2020-12-14 11:26

administrator   ~0097568

Branch CR31703_3 has been updated forcibly by kgv.

SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef

git

2020-12-14 16:27

administrator   ~0097577

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

git

2020-12-14 16:35

administrator   ~0097578

Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: f2efdc68040fd0741dcc9131c0f58887ef4c3c85

git

2020-12-14 17:14

administrator   ~0097583

Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: a2769e79e3ab2f0cfc7fcdb24494c8036d40a5b7

git

2020-12-14 19:02

administrator   ~0097589

Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: 9f6a511c086f91e2c55e3c22b916f09536a682cc

kgv

2020-12-14 21:00

developer   ~0097592

Please raise the patch
- OCCT branch: CR31703_4.

http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31703_4-master-KGV/

git

2020-12-14 21:01

administrator   ~0097593

Branch CR31703_4 has been updated forcibly by kgv.

SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25

bugmaster

2020-12-19 14:48

administrator   ~0097741

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

git

2020-12-19 15:22

administrator   ~0097779

Branch CR31703_4 has been deleted by inv.

SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25

git

2020-12-19 15:22

administrator   ~0097783

Branch CR31703_3 has been deleted by inv.

SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef

git

2020-12-19 15:23

administrator   ~0097796

Branch CR31703_2 has been deleted by inv.

SHA-1: deb6d5dde952c8b80c7e37f9703615ee64b0de84

git

2020-12-19 15:23

administrator   ~0097797

Branch CR31703_1 has been deleted by inv.

SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad

git

2020-12-19 15:23

administrator   ~0097800

Branch CR31703 has been deleted by inv.

SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f

Related Changesets

occt: master 6eb502b2

2020-11-06 08:33:58

mkrylova


Committer: bugmaster Details Diff
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.
Fixed uninitialized class field RWGltf_CafWriter::myIsForcedUVExport.

Image_Texture::MimeType() - added method returning MIME type based on image file format.
Image_Texture::WriteImage() - added method writing image into stream.
Affected Issues
0031703
mod - src/Image/Image_Texture.cxx Diff File
mod - src/Image/Image_Texture.hxx Diff File
mod - src/RWGltf/RWGltf_CafWriter.cxx Diff File
mod - src/RWGltf/RWGltf_CafWriter.hxx Diff File
mod - src/RWGltf/RWGltf_GltfBufferView.hxx Diff File
mod - src/RWGltf/RWGltf_GltfMaterialMap.cxx Diff File
mod - src/RWGltf/RWGltf_GltfMaterialMap.hxx Diff File
mod - src/RWMesh/RWMesh_MaterialMap.cxx Diff File
mod - src/RWMesh/RWMesh_MaterialMap.hxx Diff File
mod - src/XSDRAWSTLVRML/XSDRAWSTLVRML.cxx Diff File
add - tests/de_mesh/gltf_write/helmetglb Diff File
add - tests/de_mesh/gltf_write/lanternglb Diff File

Issue History

Date Modified Username Field Change
2020-08-10 16:17 kgv New Issue
2020-08-10 16:17 kgv Assigned To => kgv
2020-08-10 16:21 kgv Assigned To kgv => mkrylova
2020-08-10 16:21 kgv Status new => assigned
2020-08-10 16:22 kgv Relationship added duplicate of 0030953
2020-09-18 12:04 kgv Target Version 7.5.0 => 7.6.0
2020-11-17 17:39 git Note Added: 0096837
2020-11-17 17:45 mkrylova File Added: Lantern.glb
2020-11-17 17:46 mkrylova Assigned To mkrylova => gka
2020-11-17 17:46 mkrylova Status assigned => resolved
2020-11-17 17:46 mkrylova Steps to Reproduce Updated
2020-11-17 17:47 mkrylova Assigned To gka => mkrylova
2020-11-17 17:47 mkrylova Status resolved => assigned
2020-11-17 17:48 mkrylova Assigned To mkrylova => osa
2020-11-17 17:48 mkrylova Status assigned => resolved
2020-11-17 18:06 kgv Note Added: 0096841
2020-11-24 10:29 kgv Note Added: 0097023
2020-11-24 10:30 kgv Assigned To osa => mkrylova
2020-11-24 10:30 kgv Status resolved => assigned
2020-11-24 10:32 kgv Note Edited: 0096841
2020-11-24 10:32 kgv Note Edited: 0096841
2020-11-24 16:54 git Note Added: 0097037
2020-11-24 17:04 git Note Added: 0097040
2020-11-25 15:45 mkrylova Assigned To mkrylova => osa
2020-11-25 15:45 mkrylova Status assigned => resolved
2020-11-27 16:28 kgv Note Added: 0097111
2020-11-27 16:28 kgv Assigned To osa => mkrylova
2020-11-27 16:28 kgv Status resolved => assigned
2020-11-27 16:28 kgv Steps to Reproduce Updated
2020-11-27 16:33 kgv Note Edited: 0097111
2020-11-27 16:34 kgv Note Edited: 0097111
2020-11-27 16:35 kgv Note Edited: 0097111
2020-11-27 16:35 kgv Note Edited: 0097111
2020-12-02 14:16 git Note Added: 0097234
2020-12-02 14:22 git Note Added: 0097235
2020-12-02 15:41 git Note Added: 0097246
2020-12-02 16:04 git Note Added: 0097249
2020-12-07 12:43 git Note Added: 0097408
2020-12-07 12:43 mkrylova Assigned To mkrylova => osa
2020-12-07 12:43 mkrylova Status assigned => resolved
2020-12-14 11:26 git Note Added: 0097568
2020-12-14 16:27 git Note Added: 0097577
2020-12-14 16:35 git Note Added: 0097578
2020-12-14 16:52 kgv Assigned To osa => kgv
2020-12-14 17:14 git Note Added: 0097583
2020-12-14 19:02 git Note Added: 0097589
2020-12-14 21:00 kgv Note Added: 0097592
2020-12-14 21:00 kgv Assigned To kgv => bugmaster
2020-12-14 21:00 kgv Status resolved => reviewed
2020-12-14 21:01 git Note Added: 0097593
2020-12-19 14:48 bugmaster Note Added: 0097741
2020-12-19 14:48 bugmaster Status reviewed => tested
2020-12-19 14:55 bugmaster Test case number => de_mesh/gltf_write/helmetglb,lanternglb
2020-12-19 15:06 bugmaster Changeset attached => occt master 6eb502b2
2020-12-19 15:06 bugmaster Status tested => verified
2020-12-19 15:06 bugmaster Resolution open => fixed
2020-12-19 15:22 git Note Added: 0097779
2020-12-19 15:22 git Note Added: 0097783
2020-12-19 15:23 git Note Added: 0097796
2020-12-19 15:23 git Note Added: 0097797
2020-12-19 15:23 git Note Added: 0097800