Skip to content
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: C bindings #404

Draft
wants to merge 57 commits into
base: main
Choose a base branch
from
Draft

WIP: C bindings #404

wants to merge 57 commits into from

Conversation

nicoburns
Copy link
Collaborator

@nicoburns nicoburns commented Mar 21, 2023

Objective

Fixes #399

Feedback wanted

My main questions at this time are around the API design.

Todo

  • Ability to set measure functions on a node
  • Refactor API to remove null checks and result types
  • Refactor API to add enum validity checks
  • Ability to set grid_{template,auto}_{rows,columns} styles
  • Ability to manipulate the structure of the tree other than the single "append child" function which currently exists.
  • Ability to configure global settings such as whether to use rounding or not.
  • Run the gentest suite against the C API. Yoga has a setup using gtest here (https://github.com/facebook/yoga/tree/main/tests) and taffy_cpp has another one here (https://github.com/inobelar/taffy_cpp/tree/main/tests). Although in either case we would still need to implement our own test generation (as our API is of course different!).

@nicoburns nicoburns added enhancement New feature or request controversial This work requires a heightened standard of review due to implementation or design complexity labels Mar 21, 2023
@nicoburns nicoburns force-pushed the c-bindings branch 4 times, most recently from 235f9e9 to b88707d Compare March 21, 2023 03:45
@nicoburns
Copy link
Collaborator Author

@triniwiz They're super incomplete, but what do you think of these bindings so far? They're based on the ones from nativescript-mason, but I'm hoping to make them much smaller/simpler.

@triniwiz
Copy link

Always looking good 🕺🏽

@nicoburns
Copy link
Collaborator Author

Dumping Yoga's public API here for reference:

// Structs typedefs
typedef struct YGSize {
  float width;
  float height;
} YGSize;
typedef struct YGConfig* YGConfigRef;
typedef const struct YGConfig* YGConfigConstRef;
typedef struct YGNode* YGNodeRef;
typedef const struct YGNode* YGNodeConstRef;

// Function typedef
typedef YGSize (*YGMeasureFunc)(YGNodeConstRef node,float width,YGMeasureMode widthMode,float height,YGMeasureMode heightMode);
typedef float (*YGBaselineFunc)(YGNodeConstRef node, float width, float height);
typedef void (*YGDirtiedFunc)(YGNodeConstRef node);
typedef void (*YGPrintFunc)(YGNodeConstRef node);
typedef void (*YGNodeCleanupFunc)(YGNodeConstRef node);
typedef int (*YGLogger)(YGConfigConstRef config,YGNodeConstRef node,YGLogLevel level,const char* format,va_list args);
typedef YGNodeRef (*YGCloneNodeFunc)(YGNodeConstRef oldNode,YGNodeConstRef owner,size_t childIndex);

// Utility
bool YGFloatIsUndefined(float value);
float YGRoundValueToPixelGrid(double value,double pointScaleFactor,bool forceCeil,bool forceFloor);

// Compute layout
void YGNodeCalculateLayout(YGNodeRef node,float availableWidth,float availableHeight,YGDirection ownerDirection);

// YGNode
YGNodeRef YGNodeNew(void);
YGNodeRef YGNodeNewWithConfig(YGConfigConstRef config);
YGNodeRef YGNodeClone(YGNodeConstRef node);
void YGNodeFree(YGNodeRef node);
void YGNodeFreeRecursiveWithCleanupFunc(YGNodeRef node,YGNodeCleanupFunc cleanup);
void YGNodeFreeRecursive(YGNodeRef node);
void YGNodeReset(YGNodeRef node);

// YGNodeConfig
YGConfigConstRef YGNodeGetConfig(YGNodeRef node);
void YGNodeSetConfig(YGNodeRef node, YGConfigRef config);
void YGConfigSetPrintTreeFlag(YGConfigRef config, bool enabled);
void YGNodePrint(YGNodeConstRef node, YGPrintOptions options);

// Tree manipulation / inspection
void YGNodeInsertChild(YGNodeRef node, YGNodeRef child, size_t index);
void YGNodeSwapChild(YGNodeRef node, YGNodeRef child, size_t index);
void YGNodeRemoveChild(YGNodeRef node, YGNodeRef child);
void YGNodeRemoveAllChildren(YGNodeRef node);
YGNodeRef YGNodeGetChild(YGNodeRef node, size_t index);
YGNodeRef YGNodeGetOwner(YGNodeRef node);
YGNodeRef YGNodeGetParent(YGNodeRef node);
size_t YGNodeGetChildCount(YGNodeConstRef node);
void YGNodeSetChildren(YGNodeRef owner, const YGNodeRef* children, size_t count);


// YGNode isDirty / markDirty
bool YGNodeIsDirty(YGNodeConstRef node);
void YGNodeMarkDirty(YGNodeRef node);
void YGNodeMarkDirtyAndPropagateToDescendants(YGNodeRef node);

// YGNode non-style setters/getters
void YGNodeCopyStyle(YGNodeRef dstNode, YGNodeConstRef srcNode);
void* YGNodeGetContext(YGNodeConstRef node);
void YGNodeSetContext(YGNodeRef node, void* context);
void YGNodeSetIsReferenceBaseline(YGNodeRef node,bool isReferenceBaseline);
bool YGNodeIsReferenceBaseline(YGNodeConstRef node);
bool YGNodeHasMeasureFunc(YGNodeConstRef node);
void YGNodeSetMeasureFunc(YGNodeRef node, YGMeasureFunc measureFunc);
bool YGNodeHasBaselineFunc(YGNodeConstRef node);
void YGNodeSetBaselineFunc(YGNodeRef node,YGBaselineFunc baselineFunc);
YGDirtiedFunc YGNodeGetDirtiedFunc(YGNodeConstRef node);
void YGNodeSetDirtiedFunc(YGNodeRef node, YGDirtiedFunc dirtiedFunc);
void YGNodeSetPrintFunc(YGNodeRef node, YGPrintFunc printFunc);
bool YGNodeGetHasNewLayout(YGNodeConstRef node);
void YGNodeSetHasNewLayout(YGNodeRef node, bool hasNewLayout);
YGNodeType YGNodeGetNodeType(YGNodeConstRef node);
void YGNodeSetNodeType(YGNodeRef node, YGNodeType nodeType);


// Style property getters/setters
void YGNodeStyleSetDirection(YGNodeRef node, YGDirection direction);
YGDirection YGNodeStyleGetDirection(YGNodeConstRef node);
void YGNodeStyleSetFlexDirection(YGNodeRef node,YGFlexDirection flexDirection);
YGFlexDirection YGNodeStyleGetFlexDirection(YGNodeConstRef node);
void YGNodeStyleSetJustifyContent(YGNodeRef node,YGJustify justifyContent);
YGJustify YGNodeStyleGetJustifyContent(YGNodeConstRef node);
void YGNodeStyleSetAlignContent(YGNodeRef node, YGAlign alignContent);
YGAlign YGNodeStyleGetAlignContent(YGNodeConstRef node);
void YGNodeStyleSetAlignItems(YGNodeRef node, YGAlign alignItems);
YGAlign YGNodeStyleGetAlignItems(YGNodeConstRef node);
void YGNodeStyleSetAlignSelf(YGNodeRef node, YGAlign alignSelf);
YGAlign YGNodeStyleGetAlignSelf(YGNodeConstRef node);
void YGNodeStyleSetPositionType(YGNodeRef node,YGPositionType positionType);
YGPositionType YGNodeStyleGetPositionType(YGNodeConstRef node);
void YGNodeStyleSetFlexWrap(YGNodeRef node, YGWrap flexWrap);
YGWrap YGNodeStyleGetFlexWrap(YGNodeConstRef node);
void YGNodeStyleSetOverflow(YGNodeRef node, YGOverflow overflow);
YGOverflow YGNodeStyleGetOverflow(YGNodeConstRef node);
void YGNodeStyleSetDisplay(YGNodeRef node, YGDisplay display);
YGDisplay YGNodeStyleGetDisplay(YGNodeConstRef node);
void YGNodeStyleSetFlex(YGNodeRef node, float flex);
float YGNodeStyleGetFlex(YGNodeConstRef node);
void YGNodeStyleSetFlexGrow(YGNodeRef node, float flexGrow);
float YGNodeStyleGetFlexGrow(YGNodeConstRef node);
void YGNodeStyleSetFlexShrink(YGNodeRef node, float flexShrink);
float YGNodeStyleGetFlexShrink(YGNodeConstRef node);
void YGNodeStyleSetFlexBasis(YGNodeRef node, float flexBasis);
void YGNodeStyleSetFlexBasisPercent(YGNodeRef node, float flexBasis);
void YGNodeStyleSetFlexBasisAuto(YGNodeRef node);
YGValue YGNodeStyleGetFlexBasis(YGNodeConstRef node);
void YGNodeStyleSetPosition(YGNodeRef node, YGEdge edge, float position);
void YGNodeStyleSetPositionPercent(YGNodeRef node, YGEdge edge, float position);
YGValue YGNodeStyleGetPosition(YGNodeConstRef node, YGEdge edge);
void YGNodeStyleSetMargin(YGNodeRef node, YGEdge edge, float margin);
void YGNodeStyleSetMarginPercent(YGNodeRef node, YGEdge edge, float margin);
void YGNodeStyleSetMarginAuto(YGNodeRef node, YGEdge edge);
YGValue YGNodeStyleGetMargin(YGNodeConstRef node, YGEdge edge);
void YGNodeStyleSetPadding(YGNodeRef node, YGEdge edge, float padding);
void YGNodeStyleSetPaddingPercent(YGNodeRef node, YGEdge edge, float padding);
YGValue YGNodeStyleGetPadding(YGNodeConstRef node, YGEdge edge);
void YGNodeStyleSetBorder(YGNodeRef node, YGEdge edge, float border);
float YGNodeStyleGetBorder(YGNodeConstRef node, YGEdge edge);
void YGNodeStyleSetGap(YGNodeRef node, YGGutter gutter, float gapLength);
float YGNodeStyleGetGap(YGNodeConstRef node, YGGutter gutter);
void YGNodeStyleSetWidth(YGNodeRef node, float width);
void YGNodeStyleSetWidthPercent(YGNodeRef node, float width);
void YGNodeStyleSetWidthAuto(YGNodeRef node);
YGValue YGNodeStyleGetWidth(YGNodeConstRef node);
void YGNodeStyleSetHeight(YGNodeRef node, float height);
void YGNodeStyleSetHeightPercent(YGNodeRef node, float height);
void YGNodeStyleSetHeightAuto(YGNodeRef node);
YGValue YGNodeStyleGetHeight(YGNodeConstRef node);
void YGNodeStyleSetMinWidth(YGNodeRef node, float minWidth);
void YGNodeStyleSetMinWidthPercent(YGNodeRef node, float minWidth);
YGValue YGNodeStyleGetMinWidth(YGNodeConstRef node);
void YGNodeStyleSetMinHeight(YGNodeRef node, float minHeight);
void YGNodeStyleSetMinHeightPercent(YGNodeRef node, float minHeight);
YGValue YGNodeStyleGetMinHeight(YGNodeConstRef node);
void YGNodeStyleSetMaxWidth(YGNodeRef node, float maxWidth);
void YGNodeStyleSetMaxWidthPercent(YGNodeRef node, float maxWidth);
YGValue YGNodeStyleGetMaxWidth(YGNodeConstRef node);
void YGNodeStyleSetMaxHeight(YGNodeRef node, float maxHeight);
void YGNodeStyleSetMaxHeightPercent(YGNodeRef node, float maxHeight);
YGValue YGNodeStyleGetMaxHeight(YGNodeConstRef node);
void YGNodeStyleSetAspectRatio(YGNodeRef node, float aspectRatio);
float YGNodeStyleGetAspectRatio(YGNodeConstRef node);

// Layout getters
float YGNodeLayoutGetLeft(YGNodeConstRef node);
float YGNodeLayoutGetTop(YGNodeConstRef node);
float YGNodeLayoutGetRight(YGNodeConstRef node);
float YGNodeLayoutGetBottom(YGNodeConstRef node);
float YGNodeLayoutGetWidth(YGNodeConstRef node);
float YGNodeLayoutGetHeight(YGNodeConstRef node);
YGDirection YGNodeLayoutGetDirection(YGNodeConstRef node);
bool YGNodeLayoutGetHadOverflow(YGNodeConstRef node);
float YGNodeLayoutGetMargin(YGNodeConstRef node, YGEdge edge);
float YGNodeLayoutGetBorder(YGNodeConstRef node, YGEdge edge);
float YGNodeLayoutGetPadding(YGNodeConstRef node, YGEdge edge);

// Config getters/setters
YGConfigRef YGConfigNew(void);
void YGConfigFree(YGConfigRef config);
void YGConfigSetLogger(YGConfigRef config, YGLogger logger);
void YGConfigSetPointScaleFactor(YGConfigRef config,float pixelsInPoint);
float YGConfigGetPointScaleFactor(YGConfigConstRef config);
void YGConfigSetExperimentalFeatureEnabled(YGConfigRef config,YGExperimentalFeature feature,bool enabled);
bool YGConfigIsExperimentalFeatureEnabled(YGConfigConstRef config,YGExperimentalFeature feature);
void YGConfigSetUseWebDefaults(YGConfigRef config, bool enabled);
bool YGConfigGetUseWebDefaults(YGConfigConstRef config);
void YGConfigSetCloneNodeFunc(YGConfigRef config, YGCloneNodeFunc callback);
YGConfigConstRef YGConfigGetDefault(void);
void YGConfigSetContext(YGConfigRef config, void* context);
void* YGConfigGetContext(YGConfigConstRef config);
void YGConfigSetErrata(YGConfigRef config, YGErrata errata);
YGErrata YGConfigGetErrata(YGConfigConstRef config);

@nicoburns nicoburns force-pushed the c-bindings branch 2 times, most recently from 4e122b1 to c9fa593 Compare September 23, 2023 00:03
@inobelar
Copy link
Contributor

@nicoburns greetings! For inspiration I can show 'bindings from alternate universe' (from cpp world):

You may be confused that everything is in huge files - this is done for user convenience :D Everything written manually, not generated.

I'm still not sure about C bindings - should I leave them as is or use 'placement new', so the next api:

 typedef struct taffy_Option_float taffy_Option_float;

/* constructors */
taffy_Option_float* taffy_Option_float_new_default(void);
taffy_Option_float* taffy_Option_float_new(float* value);
taffy_Option_float* taffy_Option_float_new_some(float value);

/* copy constuctor */
taffy_Option_float* taffy_Option_float_new_copy(const taffy_Option_float* other);

becomes like this (added void* memory in constructors & copy constructors - the destination where object must created):

 typedef struct taffy_Option_float taffy_Option_float;
 
size_t taffy_Option_float_sizeof(void); /* Users allocates memory for us - we need to let them know how much to store */

/* constructors */
taffy_Option_float* taffy_Option_float_new_default(void* memory);
taffy_Option_float* taffy_Option_float_new(void* memory, float* value);
taffy_Option_float* taffy_Option_float_new_some(void* memory, float value);

/* copy constuctor */
taffy_Option_float* taffy_Option_float_new_copy(void* memory, const taffy_Option_float* other);

Its a little bit ... non-comfortable to use by end-user, or even painful & unusable at all :D Trying to imagine how to set a lot of 'Style' properties in this way (with a lot of manual memory management) can be depressing :D

For example (Lua-C-API for taffy.Option_float.new() - describes Option<float> from rust):

taffy_Option_float* object_ptr = taffy_Option_float_new_default(); /* ALLOCATION + CONSTRUCTOR call*/
if(object_ptr != NULL)
{
    taffy_Option_float** user_data = (taffy_Option_float**)lua_newuserdata(L, sizeof(taffy_Option_float*)); /* ALLOCATION */
    *user_data = object_ptr;

    luaL_setmetatable(L, LUA_META_OBJECT_taffy_Option_float);

    return 1; /* number of results */
}
else
{
    return luaL_error(L, "Failed to create taffy_Option_float : taffy_Option_float_new_default() failed");
}

with 'placement new'-way may be rewritten in the next way (single allocation):

taffy_Option_float** user_data = (taffy_Option_float**)lua_newuserdata(L, taffy_Option_float_sizeof()); /* ALLOCATION */

taffy_Option_float* object_ptr = taffy_Option_float_new_default(user_data); /* CONSTRUCTOR call*/
if(object_ptr != NULL)
{
    luaL_setmetatable(L, LUA_META_OBJECT_taffy_Option_float);

    return 1; /* number of results */
}
else
{
    /* ... */
}

Unfortunately I still haven't finished 'Lua bindings' due to lack of time, and did not fix bugs in usage of 'lua c api'. 'C bingins" I designed by using in 'Lua bingings' - first I acted as the “customer” of the 'C API', and secondly as the 'designer' of this API.

'C API', I view it primarily as 'de-magical' interface (means - easiest & fastest way to compile and use, without any templates & other magic) - the 'face' of the library, that completely hide internals & easy to write bindings. But not as a tool that needs to be used directly and write logic with it, with manual memory management :D

Have a nice day :)

@nicoburns nicoburns changed the title WIP: C bindings (experimental) WIP: C bindings Sep 23, 2023
@nicoburns nicoburns force-pushed the c-bindings branch 3 times, most recently from c00d2be to d604c2f Compare September 23, 2023 21:18
@nicoburns
Copy link
Collaborator Author

@kkukshtel (CC: @shirakaba)

These C bindings are now in a somewhat usable state:

Header file is here: https://github.com/nicoburns/taffy/blob/c-bindings/ctaffy/include/taffy.h
Example usage is here: https://github.com/nicoburns/taffy/blob/c-bindings/ctaffy/examples/basic.c

Feedback on the API design would be very helpful. In particular, I have these C-related questions:

  • Would you expect a C library to null-check input pointers? I'm currently checking pointers in each individual getter/setter like TaffyStyle_SetMarginRight(*TaffyStyle raw_style, float value, enum TaffyUnit unit);, so this could potentially be quite a lot of null checks in total...
  • Would that be different for a "handle" (opaque struct wrapping an index into a slotmap)? (would you expect some indication if you try to operate on a node which no longer exists?)
  • For functions that return a value, I'm currently returning "result structs" that contain both the value and the return code. Does that seem like a good idea?
  • A lot of these issues seem like they would be easier if I could expose a C++ API rather than a C one. But I'm under the impression that C APIs are more universally consumable?

And a couple of general API design questions:

  • Currently we have just a single TaffyDimension enum rather than separate ones like LengthPercentage that we have in the Rust API. Would splitting this up be useful?
  • Currently the API for setting styles is a single function per property (see examples). Would a great big "set all the styles" function be useful? IMO the function-per-property API reads quite nicely. But it could be less efficient without LTO?

Functionality that is still missing:

  • Ability to set grid_{template,auto}_{rows,columns} styles
  • Ability to set measure functions on a node
  • Ability to manipulate the structure of the tree other than the single "append child" function which currently exists.
  • Ability to configure global settings such as whether to use rounding or not.

It has also occurred to me that:

  • We could (and probably should) run the gentest suite against the C API. Yoga has a setup using gtest here (https://github.com/facebook/yoga/tree/main/tests) and taffy_cpp has another one here (https://github.com/inobelar/taffy_cpp/tree/main/tests). Although in either case we would still need to implement our own test generation (as our API is of course different!).
  • The C API doesn't necessarily need to follow the same release cycle as the rust crate (esp. in this early phase), and we could potentially put out a preview release of this fairly soon and then iterate.

@nicoburns
Copy link
Collaborator Author

@inobelar taffy_cpp looks really cool! I see it has a lot of generated tests. Is it passing the entirety of Taffy's test suite? If so then I'm very impressed you've managed to port the whole thing!

I'm not quite so sure about your binding API design. I feel like the using opaque types to represent things like taffy_Option_JustifyContent makes things quite complicated. I've tried to create representations in terms of plain C types, and then used conversion function inside the library instead. For example:

  • Option<JustifyContent> becomes a plain C enum, with an extra variant (compared to the Rust version) to represent None.
  • Option<f32> for aspect ratio becomes just a float, with NaN representing None
  • Separate getters/setters rather than dealing with Size or Rect
  • Dimension,LengthPercentage,etc become a struct made up of a plain C enum for the unit and a float for the value (the value is ignored if the unit is set to something like Auto that doesn't require a value).

Although I think I'm going to need to use an opaque type for the GridTrack type.

In any case, I would love to have your review on the bindings in this PR if you have the time and inclination to review them.

ctaffy/README.md Outdated
</tr>
</table>

Functions that return Result structs will either return a `TAFFY_RETURN_CODE_OK` along with a meaningful value, or a error variant of `TaffyReturnCode` along with

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this sentence is unfinished.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, the README is very much unfinished. I set out to document the API, but then I started questioning my design decisions as I did so!

@shirakaba
Copy link

shirakaba commented Sep 26, 2023

@nicoburns Thanks for having a go at tackling this beast! And sorry for the delay – I didn't think I'd be able to help much, but had a look again and found that I did have a few comments to make.

I'm not a C programmer (and barely an Objective-C programmer, which is the level at which I'd be consuming the C bindings… and even then, only to map to an easier language like Swift or JS), but will share my thoughts:

Would you expect a C library to null-check input pointers?

I'm not familiar with conventions, so would defer to yoga if it has any conventions for this.

And personally, it would suit my use-cases to leave the C library do no checking (because as you say, checking has an overhead), but maybe to annotate the higher-level bindings like Obj-C with nonnull (which I guess is overhead-free?) or equivalent annotations.

But, come to think of it, it feels like the React team's way of handling this kind of case is to check it during debug builds but not during release. Could we make it so the guards are wrapped with a precompiler directive that checks whether we're in debug/release mode (and I'm afraid I don't know the conventions for setting this up 😅)?

Would that be different for a "handle"?

I suppose for this case, I'd lean more towards the C library doing that check, as it's not a check we can simply do with a nonnull annotation in higher-level bindings.

But again, we could remove the checks in release builds.

For functions that return a value, I'm currently returning "result structs" that contain both the value and the return code. Does that seem like a good idea?

The Node-API approach to this is to return the status code, but write the value into the pointer that you pass in:

napi_status status;
napi_value object, string;
status = napi_create_object(env, &object);
if (status != napi_ok) {
  napi_throw_error(env, ...);
  return;
}

status = napi_create_string_utf8(env, "bar", NAPI_AUTO_LENGTH, &string);
if (status != napi_ok) {
  napi_throw_error(env, ...);
  return;
}

status = napi_set_named_property(env, object, "foo", string);
if (status != napi_ok) {
  napi_throw_error(env, ...);
  return;
} 

I recall that SQLite does it, too (code courtesy of ChatGPT):

#include <stdio.h>
#include <sqlite3.h>

int main() {
    sqlite3 *db;
    sqlite3_stmt *stmt;
    const char *sql = "SELECT * FROM my_table";

    // Open the SQLite database file
    int rc = sqlite3_open("my_database.db", &db);

    if (rc != SQLITE_OK) {
        fprintf(stderr, "Cannot open database: %s\n", sqlite3_errmsg(db));
        sqlite3_close(db);
        return 1;
    }

    // Prepare the SQL statement
    rc = sqlite3_prepare_v3(db, sql, -1, 0, &stmt, NULL);

    if (rc != SQLITE_OK) {
        fprintf(stderr, "Error preparing SQL statement: %s\n", sqlite3_errmsg(db));
        sqlite3_close(db);
        return 1;
    }

    // Execute the prepared statement or do other operations here
    // ...

    // Finalize the statement when you're done with it
    sqlite3_finalize(stmt);

    // Close the database
    sqlite3_close(db);

    return 0;
}

I can't tell whether it's any more memory-efficient or anything, but they're classic C projects, so it's probably a sensible pattern.

A lot of these issues seem like they would be easier if I could expose a C++ API rather than a C one. But I'm under the impression that C APIs are more universally consumable?

Yes, C++ may be more expressive, but C is ABI-stable. This is important for offering prebuilds of Taffy that are forwards-compatible. This is actually one reason that the React Native team are rewriting the internals of Hermes (at least the JSI APIs) from C++ to C – to allow prebuilds of TurboModules to be distributed.

@shirakaba
Copy link

shirakaba commented Sep 26, 2023

Currently we have just a single TaffyDimension enum rather than separate ones like LengthPercentage that we have in the Rust API. Would splitting this up be useful?

While we're on this topic – as above, things like TaffyDimensionResult should probably be handled by returning the result code but accepting a pointer to write the dimension value into.

As for TaffyDimension, it looks nice to me as it is. Means that the user can just directly switch through all the possible TaffyUnit values rather than first checking whether the dimension refers to a length or a percent or auto before deciding how to interpret the value.

… Though I may be missing something. If so, if you'd provide an example of the alternative, I could give thoughts.

Currently the API for setting styles is a single function per property (see examples). Would a great big "set all the styles" function be useful? IMO the function-per-property API reads quite nicely. But it could be less efficient without LTO?

I think it would be good to offer both. In NativeScript, we often talk about the overhead of having to cross from JS to native. While the function-per-property API certainly reads nicely (and would make code clean when calling C from C), a "set all the styles" API would potentially be very useful for setting a bunch of things in one go in a single trip from JS to native.

@triniwiz will surely have war stories about this 😄

@nicoburns nicoburns force-pushed the c-bindings branch 4 times, most recently from 2a7cc5a to 3bb289e Compare October 22, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial This work requires a heightened standard of review due to implementation or design complexity enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C bindings
6 participants