-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Java bindings #705
base: main
Are you sure you want to change the base?
WIP: Java bindings #705
Conversation
…, but I think the main parts are done (mainly parsing Style, returning Layout))
Do you think it is a good idea keeping Taffy in name of every class (I am asking the question because I feel like java doesn't have as much concise ways to express certain ideas which leads to some code looking quite long) (not even talking about the fact that in Java the types of variables are usually explicitly written). |
@adjabaev very excited to see this. I'm on vacation (with limited access to computers / internet) atm. But I will definitely review this when I get back. One thing that I have noticed from writing C and WASM Bindings (neither finished/merged yet), and that I also see here, is that there is a lot of repetitive code related to defining enums, converting enums, fields on Style, methods on TaffyTree, etc that is duplicated in each binding and in the main Rust implementation. And I have been thinking it might be good from a maintenance perspective to use code generation to automatically generate this code from a metadata definition similar to Yoga's enums.py script (this would be pretty helpful when we add new style fields for example). Regarding naming conventions, I'm not sure. Just Java have namespaces? C doesn't, which is the main reason why it has everything prefixed. |
Glad you like the initiative! :)
It is definitely something I have noticed and I would be more than glad to find a way to automate this stuff, for enum related stuff, I feel like its not gonna be very difficult (thanks to Java's Enum.ordinal(). But in the same time I am wondering if we couln't just Serialise/ Deserialise stuff as Json strings. At least I feel like it would lower the implied work by a lot, however I cannot tell what it is going to be performance wise.
Yes, Java (and Kotlin) have packages which are basically namespaces, so I removed Taffy from every class (every file still start with "package com.dioxuslabs.taffy;" (except for TaffyTree because it is its original name) |
Hi :) Also for now, it only generates classic enums without any parameter because in Java there is not such notion as enums that have different parameters (signatures) Also another question, the MeasureFunction/ NodeContext stuff seems very hard to translate to Java dynamically as NodeContext can by anything, if anyone has an idea, i'm all ears! :) |
bindings/java/src/enums.rs
Outdated
@@ -0,0 +1,251 @@ | |||
use crate::conversions::f_get_value; | |||
use crate::primitives::f_i32_from_primitive; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file could be auto-generated too. My suggested strategy would be:
- Create a trait:
trait FromJavaEnum {
const JAVA_CLASS : &'static str;
fn from_ordinal(ordinal: i32) -> Option<RustType>;
}
-
Auto-generate an impl of this trait for each rust enum when generating the java enum.
-
This file should then just need two (generic) functions: one for T (that returns default if the value is null) and one for Option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was actually an incredible idea, just commited an implem of this, would be very interested in knowing what you do think about it :)
Also i did a little refactor, so we can split different steps (enum generation/ transformers) and different languages (just java.rs 's for now, but we can imaging variants for js, c, cpp, etc)
private float scrollbarWidth; | ||
|
||
private Position position; | ||
private Rect<LengthPercentageAuto> inset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could consider "flattening" the fields that use Rect
, Size
, and Line
(so have width
and height
fields instead of size
). I reckon that would simplify the API quite a bit (and mean you wouldn't need to define the geom
java types or conversions at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I could look at it, but what about situations where only a part of components of a geom
is defined? (let's say only left
is defined in a Rect
)
* @param margin The size of the margin of the node | ||
*/ | ||
public record Layout( | ||
int order, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess if you flatten Style
you'll probably also want to flatten Layout
.
…p insertion order and isn't consistent
Also clippy doesn't validate but it looks like it isn't due to my code (and it wasn't the case before i wrote it, I tried looking for the reasons but i do not manage to understand it and would like to avoid modifying Taffy code if it isn't necessary) |
Objective
Make taffy usable in Java programs
Feedback wanted
Feel free to give me advice over what I am doing as it is the first time I am actually writing Rust/ jni bindings
TODO