MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0031703Open CASCADE[OCCT] OCCT:Data Exchangepublic2020-08-10 16:172020-12-02 16:04
Reporterkgv 
Assigned Tomkrylova 
PrioritynormalSeverityfeature 
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 7.6.0*Fixed in Version 
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 number
Attached Files? file icon Lantern.glb (9,872,848 bytes) 2020-11-17 17:45

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

-  Notes
(0096837)
git (administrator)
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 (developer)
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 (developer)
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 (administrator)
2020-11-24 16:54

Branch CR31703 has been updated forcibly by mkrylova.

SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f
(0097040)
git (administrator)
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 (developer)
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 (administrator)
2020-12-02 14:16

Branch CR31703_1 has been updated forcibly by mkrylova.

SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad
(0097235)
git (administrator)
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 (administrator)
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 (administrator)
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


- 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 View Revisions
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 View Revisions
2020-11-24 10:32 kgv Note Edited: 0096841 View Revisions
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 View Revisions
2020-11-27 16:33 kgv Note Edited: 0097111 View Revisions
2020-11-27 16:34 kgv Note Edited: 0097111 View Revisions
2020-11-27 16:35 kgv Note Edited: 0097111 View Revisions
2020-11-27 16:35 kgv Note Edited: 0097111 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker