Simple struct based domain by mwestphal · Pull Request #3232 · f3d-app/f3d

mwestphal · 2026-06-11T06:30:00Z

Describe your changes

Add range and enum domain concepts to the options through a dedicated json syntax and related cmake gen.
Add options::hasDomain, options::getDomain, options::increase, options::decrease, options::cycle
Use enum domains for command completion
Add increase and decrease commands for range domain options
Add cycle commands for enum domain options
Update all cycle_ and increase_ commands to use increase and cycle whenever possible
Add dynamic enumeration domain for camera index
Update impacted tests
Add unit options domain unit tests
Add command script tests
Add/update doc

Json syntax:

      "opacity": {
        "type": "double",
        "default_value": "0.9",
        "domain": {
          "range": ["0.0", "1.0"],
          "increment": "0.05"
        }

        "mode": {
          "type": "string",
          "default_value": "none",
          "domain": {
            "enum": ["none", "fxaa", "ssaa", "taa"]
          }

Added Domain list:

TODO:

Issue ticket number and link if any

Fix #2334

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 did not use AI to generate any of the content of that pull request
I used AI to generate code in that pull request, if yes please disclose which part of the code was generated and with which model.
...

Continuous integration

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

github-actions · 2026-06-11T06:30:09Z

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: c, python, java, webassembly.

mwestphal · 2026-06-14T16:54:58Z

\ci fast

mwestphal · 2026-06-18T19:16:00Z

  }

-  const options& Options;
+  options& Options;


needed in order to be able to modify the domain

If the user wants to modify the domain, it's probably better to do it from the engine? It's better to keep a const reference in the scene I think.

its not the user, its us modifying the domain.

codecov · 2026-06-22T06:11:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.08%. Comparing base (133a195) to head (76351d0).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3232      +/-   ##
==========================================
+ Coverage   97.06%   97.08%   +0.01%     
==========================================
  Files         213      213              
  Lines       17656    17673      +17     
==========================================
+ Hits        17137    17157      +20     
+ Misses        519      516       -3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Meakk · 2026-06-22T11:49:54Z

 CLI: `--animation-time`.

-### `scene.camera.index` (_int_, optional, **on load**)
+### `scene.camera.index` (_int_, optional, **on load**, domain: enumeration: dynamic)


Should we be more explicit about dynamic value with something like that?

Suggested change

### `scene.camera.index` (_int_, optional, **on load**, domain: enumeration: dynamic)

### `scene.camera.index` (_int_, optional, **on load**, domain: enumeration: `0, 1, ...`)

hum, this could work, but in any case it requires some kind of clarification somewhere in the option doc.

I suggest to drop the domain for indexing for now?

no, its useful.

Meakk · 2026-06-22T12:07:37Z

  }

+  // TODO keep here?
+  inline ratio_t& operator+=(const double& incr)


Well yeah I don't like the fact ratio_t only exist for parsing reason. Maybe we should remove it and do in increase() API instead: option = ratio_t(static_cast<double>(option) + increment)

yes, fine by me.

I removed the operator but it means creating a new f3d::ratio_t

Meakk · 2026-06-22T12:11:14Z

-        "type": "int"
+        "type": "int",
+        "domain": {
+          "enum": []


I suppose this dynamic domain would prevent potential improvement in the future. Maybe it should be its own domain like domain_dynamic_indexing_t?

it could, but it would be for a single option so I dont think its worth it.

All domains are dynamic btw, nothing prevent a user from changing a domain using the struct API.

The only one the libf3d changes is this one.

Ok I haven't thought about the use case where a libf3d user would want to change the range or increment. That's valid indeed.
I'm still a bit reluctant to add a range domain for camera indexing. It feels like it's a completely different logic.

It feels like it's a completely different logic.

It is indeed! I agree it could be a different domain but since the enum works as well, i think we can add that when we have more index domain.

That being said, it will mean deprecation, so maybe better to do it now.

That being said, it will mean deprecation, so maybe better to do it now.

I also think so.

github-actions · 2026-06-25T06:13:06Z

Style Checks CI failed:

diff --git a/doc/libf3d/03-OPTIONS.md b/doc/libf3d/03-OPTIONS.md
index f05e66e..122daaa 100644
--- a/doc/libf3d/03-OPTIONS.md
+++ b/doc/libf3d/03-OPTIONS.md
@@ -603,7 +603,6 @@ Show corresponding keys when notifications are triggered by bindings key press e
 
 ## Domains
 
-
 ## APIs
 
 There are three APIs to access the options
diff --git a/doc/libf3d/06-MIGRATION.md b/doc/libf3d/06-MIGRATION.md
index bc63a84..8bfb366 100644
--- a/doc/libf3d/06-MIGRATION.md
+++ b/doc/libf3d/06-MIGRATION.md
@@ -32,14 +32,14 @@ When running F3D, it was possible to specify the path for loading plugins using
 
 The following commands have been removed and should be replaced:
 
- - `cycle_anti_aliasing` -> `cycle render.effect.antialiasing.mode`
- - `cycle_blending` -> `cycle render.effect.blending.mode`
- - `cycle_point_sprites` -> `cycle model.point_sprites.type`
- - `increase_light_intensity` -> `increase render.light.intensity` (increments are different)
- - `decrease_light_intensity` -> `decrease render.light.intensity` (increments are different)
- - `increase_opacity` -> `increase model.color.opacity`
- - `decrease_opacity` -> `decrease model.color.opacity`
- - `cycle_interactor_style` -> `cycle interactor.style`
+- `cycle_anti_aliasing` -> `cycle render.effect.antialiasing.mode`
+- `cycle_blending` -> `cycle render.effect.blending.mode`
+- `cycle_point_sprites` -> `cycle model.point_sprites.type`
+- `increase_light_intensity` -> `increase render.light.intensity` (increments are different)
+- `decrease_light_intensity` -> `decrease render.light.intensity` (increments are different)
+- `increase_opacity` -> `increase model.color.opacity`
+- `decrease_opacity` -> `decrease model.color.opacity`
+- `cycle_interactor_style` -> `cycle interactor.style`
 
 The `jump_to_frame` and `jump_to_keyframe` commands no longer take a second boolean argument and now always perform an absolute jump. To perform a relative jump, use the new `jump_to_frame_relative` and `jump_to_keyframe_relative` commands:
 
diff --git a/library/private/options_generated.h.in b/library/private/options_generated.h.in
index 44f03cc..397185c 100644
--- a/library/private/options_generated.h.in
+++ b/library/private/options_generated.h.in
@@ -152,7 +152,8 @@ std::vector<std::string> getEnumDomain(const f3d::options& opt, std::string_view
 /**
  * Generated method, see `options::getRangeDomain`
  */
-std::pair<std::array<std::string, 2>, std::string> getRangeDomain(const f3d::options& opt, std::string_view name)
+std::pair<std::array<std::string, 2>, std::string> getRangeDomain(
+  const f3d::options& opt, std::string_view name)
 {
   // clang-format off
   ${_options_get_range_domain};
diff --git a/library/private/options_tools.h b/library/private/options_tools.h
index f33f3b8..cc8cf76 100644
--- a/library/private/options_tools.h
+++ b/library/private/options_tools.h
@@ -901,7 +901,8 @@ bool hasDomain(f3d::options::domain_style& styleVar, const f3d::options::domain_
  * Get provided domain_range as a string vector
  */
 template<typename T>
-std::pair<std::array<std::string, 2>, std::string> getRangeDomain(const f3d::options::domain_range_t<T>& domain)
+std::pair<std::array<std::string, 2>, std::string> getRangeDomain(
+  const f3d::options::domain_range_t<T>& domain)
 {
   std::pair<std::array<std::string, 2>, std::string> ret;
   ret.first[0] = std::move(format(domain.range[0]));
@@ -914,7 +915,8 @@ std::pair<std::array<std::string, 2>, std::string> getRangeDomain(const f3d::opt
 /**
  * Get provided domain_range<std::string> as a string vector
  */
-std::pair<std::array<std::string, 2>, std::string> getRangeDomain(const f3d::options::domain_range_t<std::string>& domain)
+std::pair<std::array<std::string, 2>, std::string> getRangeDomain(
+  const f3d::options::domain_range_t<std::string>& domain)
 {
   std::pair<std::array<std::string, 2>, std::string> ret;
   ret.first = domain.range;
@@ -944,7 +946,6 @@ std::vector<std::string> getEnumDomain(const f3d::options::domain_enum_t<std::st
   return domain.enumeration;
 }
 
-
 //----------------------------------------------------------------------------
 /**
  * Needed for increase implementation for ratio_t
diff --git a/library/public/options.h.in b/library/public/options.h.in
index 59737e8..5921b6c 100644
--- a/library/public/options.h.in
+++ b/library/public/options.h.in
@@ -172,7 +172,8 @@ public:
    * Throw options::incompatible_exception if option doesn't have a range domain.
    * Throws an options::inexistent_exception if option does not exist.
    */
-  [[nodiscard]] std::pair<std::array<std::string, 2>, std::string> getRangeDomain(std::string_view name) const;
+  [[nodiscard]] std::pair<std::array<std::string, 2>, std::string> getRangeDomain(
+    std::string_view name) const;
 
   /**
    * Return a the enumeration of the enum domain of provided option.
diff --git a/library/src/options.cxx b/library/src/options.cxx
index 49dc471..b9f6ce9 100644
--- a/library/src/options.cxx
+++ b/library/src/options.cxx
@@ -195,7 +195,8 @@ bool options::hasDomain(std::string_view name, domain_style& style) const
 }
 
 //----------------------------------------------------------------------------
-std::pair<std::array<std::string, 2>, std::string> options::getRangeDomain(std::string_view name) const
+std::pair<std::array<std::string, 2>, std::string> options::getRangeDomain(
+  std::string_view name) const
 {
   return options_generated::getRangeDomain(*this, name);
 }
diff --git a/library/testing/TestSDKOptionsDomains.cxx b/library/testing/TestSDKOptionsDomains.cxx
index 50eb173..05ecb89 100644
--- a/library/testing/TestSDKOptionsDomains.cxx
+++ b/library/testing/TestSDKOptionsDomains.cxx
@@ -19,11 +19,12 @@ int TestSDKOptionsDomains([[maybe_unused]] int argc, [[maybe_unused]] char* argv
     "hasDomain inexistent", [&]() { std::ignore = opt.hasDomain("inexistent", style); });
 
   // Test getRangeDomain
-  std::pair<std::array<std::string, 2>, std::string> rangeDomain = opt.getRangeDomain("scene.animation.speed_factor");
-  test("getRangeDomain range", rangeDomain.first, {"0", "2"});
+  std::pair<std::array<std::string, 2>, std::string> rangeDomain =
+    opt.getRangeDomain("scene.animation.speed_factor");
+  test("getRangeDomain range", rangeDomain.first, { "0", "2" });
   test("getRangeDomain increment", rangeDomain.second, std::string("0.1"));
-  test.expect<f3d::options::incompatible_exception>(
-    "getRangeDomain incompatible", [&]() { std::ignore = opt.getRangeDomain("model.scivis.cells"); });
+  test.expect<f3d::options::incompatible_exception>("getRangeDomain incompatible",
+    [&]() { std::ignore = opt.getRangeDomain("model.scivis.cells"); });
   test.expect<f3d::options::inexistent_exception>(
     "getRangeDomain inexistent", [&]() { std::ignore = opt.getRangeDomain("inexistent"); });

mwestphal self-assigned this Jun 14, 2026

mwestphal added the ci:fast label Jun 14, 2026

mwestphal force-pushed the options_domain_struct branch 3 times, most recently from 6135506 to 39b547a Compare June 18, 2026 05:43

mwestphal commented Jun 18, 2026

View reviewed changes

Comment thread library/public/types.h Outdated

mwestphal commented Jun 18, 2026

View reviewed changes

Comment thread library/private/options_tools.h Outdated

mwestphal commented Jun 18, 2026

View reviewed changes

Comment thread library/options.json

mwestphal commented Jun 18, 2026

View reviewed changes

Comment thread library/src/scene_impl.cxx

mwestphal marked this pull request as ready for review June 18, 2026 19:16

mwestphal requested a review from a team as a code owner June 18, 2026 19:16

mwestphal marked this pull request as draft June 18, 2026 19:16

mwestphal commented Jun 21, 2026

View reviewed changes

Comment thread vtkext/private/module/vtkF3DRenderer.cxx

mwestphal commented Jun 21, 2026

View reviewed changes

Comment thread testing/baselines/TestInteractionLightIntensity.png

mwestphal force-pushed the options_domain_struct branch from 8790656 to 7c543e3 Compare June 21, 2026 13:40

mwestphal commented Jun 21, 2026

View reviewed changes

Comment thread doc/libf3d/03-OPTIONS.md

mwestphal commented Jun 21, 2026

View reviewed changes

Comment thread doc/user/07-COMMANDS.md

mwestphal added ci:full and removed ci:fast labels Jun 21, 2026

mwestphal marked this pull request as ready for review June 21, 2026 14:54

mwestphal requested review from Meakk and snoyer and removed request for a team June 21, 2026 14:54

mwestphal added ci:main ci:full and removed ci:full ci:main labels Jun 21, 2026

Meakk reviewed Jun 22, 2026

View reviewed changes

mwestphal commented Jun 23, 2026

View reviewed changes

Comment thread library/src/interactor_impl.cxx Outdated

mwestphal added 23 commits June 25, 2026 08:27

Simple struct based domain

0bc4b44

double computation fixup

6deb638

use domains for completion

bdf0e95

unit sdk test

10b5d88

Update outdated tests

745ebb4

proper functionnal testing

775dfbd

cmake doc and cleanup

ea12537

API doc

71bd2e3

adding doc first pass

f766dab

dynamic

2f4c46b

patchclip

658bf22

clf

be4d818

iiiiiiiiiiiiii

a8b46e5

patch again

699fbcf

Fix remaining tests

97d8b91

cppcheck

3c15ffc

fixup baseline

97ddd19

fixups

055d893

patch

c3dd098

test with ratio_t

84f970f

split domain getter

6127dd1

unfinished doc

8909574

move ratio operator

d7df19d

mwestphal force-pushed the options_domain_struct branch from 8bb6ca7 to d7df19d Compare June 25, 2026 06:29

	### `scene.camera.index` (_int_, optional, on load, domain: enumeration: dynamic)
	### `scene.camera.index` (_int_, optional, on load, domain: enumeration: `0, 1, ...`)

Uh oh!

Uh oh!

Conversation

mwestphal commented Jun 11, 2026 • edited 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!

github-actions Bot commented Jun 11, 2026

Uh oh!

mwestphal commented Jun 14, 2026

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

mwestphal Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

github-actions Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mwestphal commented Jun 11, 2026 •

edited

Loading

codecov Bot commented Jun 22, 2026 •

edited

Loading

mwestphal Jun 25, 2026 •

edited

Loading

github-actions Bot commented Jun 25, 2026 •

edited

Loading