refactor!: Make Node opaque and optimize it for size#205
Conversation
…ialization is currently broken
…ndoned experiment
… of Chromium's caching implementation
… have 2 spare bits in the flags field.
|
Almost done. I need to implement the |
|
I was previously asked to implement the builder pattern, where the setter methods look like this: I previously rejected this because I felt it was relying too much on the optimizer to avoid copying the struct around. I also felt that, while the pattern is convenient for hand-written test and demo code, it wouldn't matter much in actual GUI toolkit implementations, and might actually be less convenient in that context. Now I've realized that I was more right with that last point than I knew. In both egui and my Druid proof-of-concept integration, the toolkit stores under-construction |
…s the value of a property but not its identity
…rting kurbo itself
|
The change to eliminate the dependency on kurbo may seem to be over-reaching for this PR. But it's part of my plan to make AccessKit acceptable to even the most dependency-conscious projects, and I figured I should make all the planned breaking API changes in the foundational crate at once. |
…y dependencies in that crate now!
|
This is now ready for review. The impact on a real GUI toolkit can be seen in this egui branch. @federicomenaquintero Since this is based on one of the techniques that you used to reduce memory usage in librsvg, as we previously discussed in the GNOME accessibility chat room, I'd appreciate your review. |
|
There are two things about this branch I'm currently not happy with:
|
federicomenaquintero
left a comment
There was a problem hiding this comment.
This is very interesting; I'm glad that librsvg's optimization also works for you. I learned a couple of tricks from your commits, which may be usable for librsvg (serde magic and such). Thanks for leaving the fine-grained commits around!
common/src/lib.rs
Outdated
| ($($(#[$doc:meta])* ($base_name:ident, $id:ident, $type_method_base:ident, $getter_result:ty, $setter_param:ty))+) => { | ||
| paste! { | ||
| impl Node { | ||
| $($(#[$doc])* |
There was a problem hiding this comment.
Kudos for including the docs via macros this way!
Also, I didn't know about the paste crate. This is really useful.
| #[cfg_attr(feature = "schemars", derive(JsonSchema))] | ||
| #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] | ||
| #[cfg_attr(feature = "serde", enumset(serialize_as_list))] | ||
| enum Flag { |
There was a problem hiding this comment.
Excellent use of enumset 👌
There was a problem hiding this comment.
Ah, well, I see that you removed it later. No problem, the point was compressing bools to bitflags.
…e footprint of handling unexpected errors to a minimum
|
I have permission from @DataTriny (my primary collaborator on this project) to go ahead and merge this, though it hasn't yet been fully reviewed. We agree that the change is necessary, I'm confident that the approach is fundamentally sound, and I don't want to hold up corresponding updates in downstream projects. Please open issues or PRs for any problems found in subsequent reviews of this refactor. |
This is a massive breaking change for all users. But I'm afraid it's unavoidable.
This is still a work in progress. The technique is proven to work, but I still need to do the following:
boolfields