Add property checker/metadata system and C++ helpers #166
Conversation
abizeauMaxon
left a comment
abizeauMaxon
left a comment
There was a problem hiding this comment.
That super! I see a lot of good thing here! Will be helpfull with time.
include/ofxPropsMetadata.h
Outdated
| std::vector<const char *> values; // for enums | ||
| }; | ||
|
|
||
| const std::vector<struct PropsMetadata> props_metadata { |
There was a problem hiding this comment.
Since this it auto generated, you could consider std::array<struct PropsMetadata, N=FeedByScrip>.
If this header is intended to be use more C++, you should consider having this std::array as constexpr.
maybe:
#if defined(__cplusplus)
#define CONST_SPECIFIER constexpr
#else
#define CONST_SPECIFIER const
#endif
There was a problem hiding this comment.
Thanks @abizeauMaxon -- totally agreed. As per discussion at the TSC today, I've moved these into Support/include and they are 100% C++. I used fixed-size std::arrays where possible, as well as some other updates and cleanups. More feedback will be helpful; let me know if you try it out.
include/ofxPropsMetadata.h
Outdated
| int dimension; | ||
| Writable writable; | ||
| bool host_optional; | ||
| std::vector<const char *> values; // for enums |
There was a problem hiding this comment.
You should be consistant, if you use std::string for name, you should also use std::vector<std::string>
There was a problem hiding this comment.
I've tried now to move to char *, to avoid allocations where possible during startup. I should move to std::string_view in some places. I'll do that.
include/ofxPropsBySet.h
Outdated
|
|
||
| namespace OpenFX { | ||
| // Properties for property sets | ||
| const std::map<const char *, std::vector<const char *>> prop_sets { |
There was a problem hiding this comment.
again, here you should consider consistency, pick std::string or const char*
| @@ -0,0 +1,1508 @@ | |||
| ######################################################################## | |||
There was a problem hiding this comment.
Is this auto generated? If so maybe add it into the header
There was a problem hiding this comment.
ofx-props.yml is hand-created -- it's now the source of truth for all info about properties and property sets. That's the lion's share of the work in this PR (and probably the source of remaining bugs, where I may have gotten some stuff wrong). None of this info was machine-readable before, so I extracted it from the docs and header file comments.
garyo
force-pushed
the
props-metadata
branch
2 times, most recently
from
a758e36 to
f62c4be
Compare
garyo
changed the title
* include/ofx-props.yml is the property reference/metadata * scripts/gen-props.py is the checker and source generator All props are included in the YML definition, and the script generates C++ files with all the property metadata and suite assignments. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Also include them into prop tester plugin, to test whether they compile properly. TODO: prop tester should use the metadata to do better tests. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
The strings are the true spec, not the #define names. This also adds a set of static_assert tests to ensure the #define names match their strings. But doing this brought up that there are some mismatches between the strings and their #defines. Those will be accounted for in a following commit. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
See `get_cname` and `find_stringname` in gen-props.py, and the new `cname` member of the props metadata. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Replaced a vector with a std::array for reduced memory & startup time. Also add static_assert checks for all action names vs. their #defines. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Also separate out Actions from prop sets in ofx-props.yml, for cleanliness. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Update testProperties sample to use it (WIP) Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
- New script gen-props-doc.py to generate RST doc for all props and
prop sets
- Generated docs are checked in, just like the generated props
headers.
- Use it in Documentation/build.sh
- Fix some other doc I noticed along the way
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Also cleaned up a few doc typos I noticed. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
- Added to ofx-props.yml - Ran gen-props.py script to update property set headers Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Switch to using spans in a few places where it makes sense, mostly for property type arrays. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Hosts can set up properties in their own namespaces and give plugins type-safe access just like built-in properties. Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
To be extended to all standard prop sets Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
|
I'm interested in this PR landing, because I think the YAML could also be used to generate idiomatic Rust bindings to the OpenFX API. I maintain a video effect written in Rust that has an OpenFX plugin, and it currently uses FFI to the C headers. It'd be nice to be able to autogenerate a set of idiomatic Rust bindings. Related to that, I have a question about the C++17 "host-extensible properties" system. As I understand it, the C++ helper is solely about adding type-safety to host-defined properties, atop the existing plugin layer defined by the C ABI. Property lookups are still done via strings, and it is still possible to use host-defined custom properties via the C API without requiring C++17. Is that correct? (Adding on to my concern is that the README for the host-extensible properties example appears to be entirely AI-generated.) It may be good to encourage hosts to ship the .yml files used to generate the host-defined properties headers as well, since if they are in fact independent of C++, they could also be used to provide an idiomatic Rust interface to host-defined properties. |
|
Hi @valadaptive , thanks for the interest! Enabling rust bindings (and python) were one of the original goals of this work, so I'm glad you're thinking the same way. And yes, I'm hoping hosts will ship the .yml files and not just the headers. You're right that this is just a type-safety layer on top of the C API based on using the .yaml file as the source of truth for properties and their types; nothing is changing at the C level. And yeah, I'll rewrite that header properly soon. |
Yes. If it's possible to land this in stages rather than waiting for a complete overhaul, I think that would be good. I might open an issue going over what would help for Rust bindings. I'm still experimenting and trying to come up with the best API. |
include/ofx-props.ymlis the property reference/metadatascripts/gen-props.pyis the checker and source generatorAll props are included in the YML definition, and the script generates C++ files with all the property metadata and suite assignments. The C++ files aren't yet checked in, but you can build them easily:
python scripts/gen-props.py -vand then look ininclude/gen_*.hxx.