View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0031703 | Open CASCADE | OCCT:Data Exchange | public | 2020-08-10 16:17 | 2022-11-03 15:18 |
Reporter | kgv | Assigned To | bugmaster | ||
Priority | normal | Severity | feature | ||
Status | closed | Resolution | fixed | ||
Target Version | 7.6.0 | Fixed in Version | 7.6.0 | ||
Summary | 0031703: Data Exchange, RWGltf_CafWriter - add option putting textures inside GLB file as alternative to external references | ||||
Description | Currently, 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 | ||||
Tags | No tags attached. | ||||
Test case number | de_mesh/gltf_write/helmetglb,lanternglb | ||||
|
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 |
2020-11-17 17:45 developer |
Lantern.glb (9,872,848 bytes) |
|
+ 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. |
|
+ 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()); |
|
Branch CR31703 has been updated forcibly by mkrylova. SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f |
|
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 |
|
+ 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). |
|
Branch CR31703_1 has been updated forcibly by mkrylova. SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad |
|
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 |
|
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 |
|
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 |
|
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 |
|
Branch CR31703_3 has been updated forcibly by kgv. SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef |
|
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 |
|
Branch CR31703_4 has been updated forcibly by kgv. SHA-1: f2efdc68040fd0741dcc9131c0f58887ef4c3c85 |
|
Branch CR31703_4 has been updated forcibly by kgv. SHA-1: a2769e79e3ab2f0cfc7fcdb24494c8036d40a5b7 |
|
Branch CR31703_4 has been updated forcibly by kgv. SHA-1: 9f6a511c086f91e2c55e3c22b916f09536a682cc |
|
Please raise the patch - OCCT branch: CR31703_4. http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR31703_4-master-KGV/ |
|
Branch CR31703_4 has been updated forcibly by kgv. SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25 |
|
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 |
|
Branch CR31703_4 has been deleted by inv. SHA-1: 41e55dc636bc4ec56f0b4764c87c0ff558347a25 |
|
Branch CR31703_3 has been deleted by inv. SHA-1: 2db9706c425e355f94939e79ea08601f453cdeef |
|
Branch CR31703_2 has been deleted by inv. SHA-1: deb6d5dde952c8b80c7e37f9703615ee64b0de84 |
|
Branch CR31703_1 has been deleted by inv. SHA-1: 386a23beca82c97337b28e3a4cb2f6b2f0e30dad |
|
Branch CR31703 has been deleted by inv. SHA-1: 5fef75bc1fbf8cd1cef1b6e0c10932fd63eb9b7f |
occt: master 6eb502b2 2020-11-06 08:33:58
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 |
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 |
|
File Added: Lantern.glb | |
2020-11-17 17:46 |
|
Assigned To | mkrylova => gka |
2020-11-17 17:46 |
|
Status | assigned => resolved |
2020-11-17 17:46 |
|
Steps to Reproduce Updated | |
2020-11-17 17:47 |
|
Assigned To | gka => mkrylova |
2020-11-17 17:47 |
|
Status | resolved => assigned |
2020-11-17 17:48 |
|
Assigned To | mkrylova => osa |
2020-11-17 17:48 |
|
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 |
|
Assigned To | mkrylova => osa |
2020-11-25 15:45 |
|
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 |
|
Assigned To | mkrylova => osa |
2020-12-07 12:43 |
|
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 |