feat(reader): add vtkF3DLYSReader to support .lys files natively by srividya-77 · Pull Request #3413 · f3d-app/f3d

srividya-77 · 2026-07-24T17:15:05Z

Describe your changes

This PR adds native support for reading Lychee Slicer scene files (.lys).

Key implementation details:

Implemented vtkF3DLYSReader to parse .lys binary containers.
Extracted geometry payload by correctly locating the 12-byte geometry header offset ([indexCount, coordCount, reserved]).
Registered .lys extension with the native plugin.
Added TestF3DLYSReader unit test along with sample test datasets (bunny.lys, newton.lys).

Issue ticket number and link if any

#3341

Checklist for finalizing the PR

I have performed a self-review of my code
I have added tests for new features and bugfixes
I have added documentation for new features
If it is a modifying the libf3d API, I have updated bindings
If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

AI Disclosure

I have not used AI to generate any of the content of this pull request
I have used AI to generate code in this pull request:
- I have carefully read and understood the AI policy.
- I have carefully reviewed and completely understood every generated line.
- I disclose below which parts of the code were generated and with which AI model:

...

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

srividya-77 · 2026-07-24T17:15:57Z

\ci fast

mwestphal · 2026-07-25T05:59:19Z

 #if VTK_VERSION_NUMBER < VTK_VERSION_CHECK(9, 6, 2)
  if (actor->GetProperty()->GetInterpolation() == VTK_PBR &&
-    this->PrimitiveInfo[this->LastBoundBO].LastLightComplexity == primitiveInfo::NoLighting)
+    this->PrimitiveInfo[this->LastBoundBO].LastLightComplexity == 0)


why this change ?

NoLighting evaluates to 0, and the comparison
is against the integer value anyway. I changed it to == 0 for clarity and
consistency.

clarity and consistency with what ?

mwestphal · 2026-07-25T06:00:41Z

add mention in DATA_LICENSES.md

Will do! I'll add the license and origin details for bunny.lys to DATA_LICENSES.md.

mwestphal · 2026-07-25T06:00:53Z

why is the zip file needed ?

You're right, it's not needed - I'll remove it! I originally downloaded bunny.lys as a .zip from the issue , but since the reader parses .lys directly, the zip file has nothing to do with it. I'll delete it in the next commit.

mwestphal · 2026-07-25T06:01:08Z

I initially included newton.lys to inspect and verify the json metadata format across different files. Since one test file was enough, I didn't end up wiring it into CMake.

mwestphal · 2026-07-25T06:02:11Z

+ * Note: Initially thought LYS was a ZIP archive based on the spec docs, but after inspecting
+ * the actual file with a hex editor, discovered it's actually a custom binary format.
+ * The JSON key is "mangoFiles" not "files" - this was confirmed after checking the  data.
+ * The geometry .bin file is identified by looking for a .bin entry that is NOT "scene.bin".
+ *
+ * Important: The data section offset must be calculated as (16 + jsonBlockSize + offset_from_json).
+ * Don't hardcode offset 0 for the geometry - always read it from the JSON entry.
+ * The mesh .bin header is 12 bytes: indexCount (uint32), coordCount (uint32), reserved (uint32).
+ * The index buffer starts at byte 12, immediately after the mesh header.


lets not mention your own thoughts, instead focus on factual info about the file format and how it is strucutured.
Also add links that helped you figure out that structure

Will rewrite the doc comment to describe only factual format details and remove the personal discovery
notes. I'll also add links to the Lychee Slicer documentation/spec.

mwestphal · 2026-07-25T06:04:22Z

+  // Full string search is used here instead of full JSON parsing.
+  // Performance can be evaluated later if needed.
+  std::vector<char> jsonBuf(jsonLen);


defintely better to do a proper json parsing, its just metadata anyway right ? it does not scale with data size ?

I'll replace the string search in CanReadFile with a proper nlohmann::json::parse - the json block is metadata only, so it won't scale with mesh data size and the overhead is negligible.

mwestphal · 2026-07-25T06:05:22Z

+//----------------------------------------------------------------------------
+namespace
+{
+uint32_t ReadU32LE(const unsigned char* buf, size_t offset)


not following why this is needed, a reinterpret_cast should work right ?

The helper avoids potential undefined behavior from unaligned reads.But I can replace it with memcpy into a uint32_t local variable, which is both safe and as readable. Or if you prefer, I can use reinterpret_cast — the buffer is std::vector so alignment is technically not guaranteed, but in practice it works. What do you prefer?

just read directly into a uint32_t ?

mwestphal · 2026-07-25T06:06:05Z

+    stream = fileStream;
+  }
+  // Read the 16-byte container header
+  // bytes  0- 3 : unknown


unknown ? what does it contains ? is it always the same ?

I checked the hex dumps for both bunny.lys and newton.lys to analyze the container header:bytes 0–3 constant container version (4). I'll add a check in CanReadFile to validate this version integer.bytes 8–11 Varies per file (always jsonBlockSize - 4), acting as an unpadded/intermediate chunk descriptor size field.I will update the comments in vtkF3DLYSReader.cxx and add the version check to CanReadFile.

mwestphal · 2026-07-25T06:06:19Z

+  // Read the 16-byte container header
+  // bytes  0- 3 : unknown
+  // bytes  4- 7 : padded JSON block size (uint32 LE) - includes trailing alignment bytes
+  // bytes  8-11 : unknown


unknown ? what does it contains ? is it always the same ?

mwestphal · 2026-07-25T06:08:28Z

+      // Assuming string format based on bunny.lys observation.
+      // TODO: may need to handle numeric offset if future LYS versions change this.


I suppose you checked other files ?

I verified both bunny.lys and newton.lys and found that offset is stored as a string,while size is an integer.However, since I can't guarantee how older or future LYS format versions handle this,mentioned it as TODO.

It should be more like a XXX cause there is nothing to do.

mwestphal · 2026-07-25T06:10:52Z

+  for (auto it = mangoFiles.begin(); it != mangoFiles.end(); ++it)
+  {
+    const std::string& key = it.key();
+    if (key.size() >= 4 && key.substr(key.size() - 4) == ".bin" && key != "scene.bin")


Suggested change

if (key.size() >= 4 && key.substr(key.size() - 4) == ".bin" && key != "scene.bin")

// The geomtery is the first entry finishing by `.bin` that is not named `scene.bin`

if (key.size() >= 4 && key.substr(key.size() - 4) == ".bin" && key != "scene.bin")

Also is an entry named .bin without any name in front valid ?

Will add the suggested comment. Regarding a key named exactly .bin - yes, it would currently match since it ends in .bin and isn't scene.bin. In practice this seems unlikely given the LYS format spec, but I can add
key.size() > 4 to require at least one character before .bin as a safeguard.

mwestphal · 2026-07-25T06:12:17Z

+  }
+
+  // Round indexCount down to a multiple of 3 to get only complete triangles
+  const uint32_t indexCount = (::ReadU32LE(geomBuf.data(), 0) / 3) * 3;


what if it contains incomplete triangles ? is the file still valid ?

Currently I silently round down - if indexCount % 3 != 0, trailing indices are ignored. I haven't found anything in the spec about this. I can add a warning log when this happens, or treat it as an error. What would you prefer?

warning sounds fine

mwestphal · 2026-07-25T06:13:57Z

+  // Layout: XYZ float32 triplets, coordCount floats total = coordCount/3 points.
+  const float* coords = reinterpret_cast<const float*>(geomBuf.data() + 12 + indexBufSize);
+  const vtkIdType nPoints = static_cast<vtkIdType>(coordCount / 3);
+  vtkNew<vtkFloatArray> pointData;


lets names this differently, like pointArray

Will rename to pointArray.

mwestphal · 2026-07-25T06:15:47Z

+  pointData->SetNumberOfTuples(nPoints);
+  for (vtkIdType i = 0; i < nPoints; ++i)
+  {
+    pointData->SetTypedTuple(i, coords + i * 3);


I think you can use SetArray instead of copying the data

mwestphal · 2026-07-25T06:16:45Z

+      return 0;
+    }
+
+    triangles->InsertNextCell(3, tri);


use SetCells instead, you may even be able to use SetArray depending on memory layout

for points : I’ll use pointArray->SetArray with save=1 (transferring ownership) so vtk frees the buffer directly, avoiding per-tuple loops.For cells: I’ll validate indices in a quick pass first, then bulk-populate the connectivity arrays and call vtkCellArray::SetData. This gets rid of the InsertNextCell loop while keeping bounds checking safe

mwestphal · 2026-07-25T06:17:46Z

+f3d_plugin_declare_reader(
+  NAME LYS
+  EXTENSIONS lys
+  MIMETYPES application/vnd.lys


missing mimetype declaration

Will add the missing system mimeype declaration. I'll look at how other formats ( the spz reader) declare their mimetypes and follow the same pattern.

mwestphal

Missing:

Config
Thumbnail config
Config test
Thumbnail test
Piped test

mwestphal · 2026-07-28T05:32:53Z

 - Added a `--dpi-aware` option to rescale font automatically on HiDPI screens (Windows only)
 - Added a `webifc` plugin to add support for .ifc files
 - Added a `pdal` plugin to add support for many point cloud formats, including .las and .laz
+- Added support for `.lys` (Lychee Slicer) files in the native plugin


why did you add that ?

My mistake -I misunderstood the workflow. The changelog entry for v3.5.0 is maintained by the F3D team, not by contributors. I should not have added the lys line myself. I'll remove it in the next commit

mwestphal · 2026-07-28T05:33:34Z

 - Box_draco\*: glTF-Sample-Models/Cesium: [CC-BY 4.0](https://creativecommons.org/licenses/by/4.0/)
 - BoxAnimated\*: glTF-Sample-Models/Cesium: [CC-BY 4.0](https://creativecommons.org/licenses/by/4.0/)
- Cameras.gltf: glTF-Sample-Models: Public Domain
- cow.vtk: VTK Data: BSD-3-Clause


why did you remove a file ?

I didn't intentionally remove any file - I accidentally deleted a newline when removing the newton.lys entry.

mwestphal added the ci:fast label Jul 24, 2026

srividya-77 marked this pull request as ready for review July 24, 2026 20:22

srividya-77 requested a review from a team as a code owner July 24, 2026 20:22

mwestphal linked an issue Jul 25, 2026 that may be closed by this pull request

Lychee LYS file format support #3341

Open

mwestphal reviewed Jul 25, 2026

View reviewed changes

Comment thread plugins/native/module/vtkF3DLYSReader.cxx Outdated

mwestphal reviewed Jul 25, 2026

View reviewed changes

mwestphal requested changes Jul 25, 2026

View reviewed changes

mwestphal reviewed Jul 28, 2026

View reviewed changes

srividya-77 force-pushed the add-lys-reader branch from 64216e2 to ce4722e Compare August 2, 2026 04:34

feat(reader): support Lychee Slicer LYS format

ae2da38

srividya-77 force-pushed the add-lys-reader branch from 577cfc4 to ae2da38 Compare August 2, 2026 05:35

Merge branch 'master' into add-lys-reader

e415cc1

		// Assuming string format based on bunny.lys observation.
		// TODO: may need to handle numeric offset if future LYS versions change this.

	if (key.size() >= 4 && key.substr(key.size() - 4) == ".bin" && key != "scene.bin")
	// The geomtery is the first entry finishing by `.bin` that is not named `scene.bin`
	if (key.size() >= 4 && key.substr(key.size() - 4) == ".bin" && key != "scene.bin")

Uh oh!

Uh oh!

Conversation

srividya-77 commented Jul 24, 2026 • edited by mwestphal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number and link if any

Checklist for finalizing the PR

AI Disclosure

Continuous integration

Uh oh!

srividya-77 commented Jul 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srividya-77 commented Jul 24, 2026 •

edited by mwestphal

Loading

mwestphal Jul 28, 2026 •

edited

Loading