8000 `api.json` Improvements · Issue #116 · orca-app/orca · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

api.json Improvements #116

New issue

H 8000 ave a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Open
16 tasks
Parzival-3141 opened this issue Apr 15, 2025 · 5 comments
Open
16 tasks

api.json Improvements #116

Parzival-3141 opened this issue Apr 15, 2025 · 5 comments
Assignees
@Parzival-3141

Comments

@Parzival-3141
Copy link
Contributor
Parzival-3141 commented Apr 15, 2025 •
edited
Loading

This is a list of changes to api.json I compiled while working on the Zig bindings. I've split them into two categories: "Improvements" for straightforward fixes, and "Proposals" for bigger changes I'd like feedback on. Also, I'm preemptively volunteering myself to do all the work accepted in this issue.

Improvements

  • Json format documentation
  • Json format and API versioning
    • Include the version of the format and API (in that order) before any modules.
    • Including the API version helps consumers validate contents and explicitly handle older or newer versions.
    • Giving the json format a version helps binding generators for the same reasons as above. It's worth doing this future-proofing now rather than later.
  • Change module brief field to doc for consistency
  • Remove unnamed enums, use const kind instead
  • Add missing OC_UNICODE_RANGE values
    • These could be added in the same manner as OC_UI_THEMEs.
  • Add missing oc_clock API
    • Some orca samples (e.g. clock) are impossible to port correctly without this API.
  • Add missing oc_window API
    • oc_window typename is referenced in the API but not declared.
  • Create oc_mat2x3 oc_mat2x3_scale(f32 x, f32 y)
    • This one isn't even in the C API. It's useful enough that it really should be included.
  • Add documentation for UI API

Proposals

  • Specify pointer attributes in api.json #117
  • Merge *_enum types with their "parents"
    • E.g. convert oc_file_perm into an enum type and remove oc_file_perm_enum. This simplifies the API surface while still allowing consumers to separate them if desired. It would also prevent the backing integer of the enum straying from the "parent" integer type, as is the case with oc_file_perm (though that issue can be resolved regardless of this proposal).
  • Differentiate bitflag enum types from normal enums
    • Consumers could implement flags using a more suitable primitive without defining overrides for each typename. E.g. A Zig bindings generator would automatically generate packed structs.
  • Document what errors can be triggered from individual IO API functions
    • Documented errors should be the superset of possible errors for every platform.
    • This would allow consumers to confidently handle all possible error cases. The Zig bindings would use this to create error sets, adding additional type safety.

The rest of these are organizational changes to modules and their contents. They're more nitpick-y but IMO feel more natural. I don't expect them to break anyone's code.

  • Remove Utility module and move it's children into the root
    • Utility is a poor name due to it's vagueness and redundancy (all code in the API has utility, or else it wouldn't be included). This is already reflected in function names, as none have them have an oc_util_ prefix. The name "utility" usually also implies the contents are small helpers for the main API, yet it encompasses half the API surface.
  • Move UI module contents (not Core or Widgets) into Application module, and move UI Core & Widgets under UI
    • Just from looking at the API, it seems like all this is really general application state. You could just move ui/input_state.h into app/ as well.
  • Move oc_font (+ metrics), oc_color, and oc_image (+ region and atlas) types and functions into the Graphics module
    • These types are consumed by the Canvas API, but don't really belong to it (i.e. none of them are created or manipulated via Canvas functions). However their "usage" functions (e.g. oc_set_color or oc_image_draw) should remain in Canvas.
@Parzival-3141 Parzival-3141 mentioned this issue Apr 15, 2025
Open
@martinfouilleul
Copy link
Collaborator

Thanks for volunteering! ;) I generally like your points here, just a couple remarks:

Improvements:

  • The idea of the brief field was that you could have a short description (brief) and a longer documentation (doc). In the documentation the brief appears next to module/submodules names in the overview, and both the brief and doc is shown in the module's page. I didn't have an immediate use for brief in functions/structs, that's why there's only doc fields here. Although it could be useful to have brief there to generate shorter "sheetsheets".

  • I think unnamed enums still have a use, because they are grouped as a specific type (not just their underlying type like i32). Otoh we could have a typename that just forwards to the underlying type for that, (although it doesn't necessarily convey the same sense of grouping)... What do you think?

  • For unicode range values, you'll have to come up with a schema for expressing values besides strings and primitive types. It will be super useful later on, even if the unicode ranges specifically won't be needed when we merge the text api rework branch.

Proposals:

  • pointer attributes: I like the idea, will comment on the separate issue.
  • Yeah making a separate _enum type wasn't great idea, it's good to remove it, though you'll have to watch for cases where we need a specific size in structs (and not whatever the C enum ends up being).
  • Could you elaborate on "Differentiate bitflag enum types from normal enums"?

I'll comment on the organizational changes shortly!

@martinfouilleul
Copy link
Collaborator

  • I'm not sure I want to put all the "Utilities" modules under the root. They're a bunch of small-ish datastructure and helpers that are used throughout the codebase, and it makes sense to me to group them as such. But I agree utility may not be a good name, and I'm open to better suggestions.

  • I'm open to moving the general UI input stuff to App and call it "Input State" or something. I'd rather keep UI core and UI widgets close though (maybe even move them under a common parent, esp. since we might want some preamble/overview in the docs explaining how one is built on top of the other).

  • I could see fonts/color/image being children of Graphics instead of Canvas, although that would make for rather small modules, and their only real use is in the canvas api, so... not sure about it yet.

Generally the concept of "modules" in the api.json is more a way of organizing the API docs, than a code organization thing (we don't really have "code modules" other than in a loose sense of grouping anyways). If you find that it is confusing we could think of a way to reorganize api.json and indicate docs "chapters" in another way (or just find a better name). Let me know what you think!

@Parzival-3141
Copy link
Contributor Author

On the organizational changes:

the concept of "modules" in the api.json is more a way of organizing the API docs, than a code organization thing...

Ah that makes much more sense! I was assuming the latter case. Is that distinction advertised anywhere?

I'm not sure I want to put all the "Utilities" modules under the root.

I'm fine with keeping Utilities then, especially given the previous info.

I'm open to moving the general UI input stuff to App and call it "Input State"...

Sounds good. I think reparenting UI Core & Widgets under UI is the move as well.

I could see fonts/color/image being children of Graphics instead of Canvas, although that would make for rather small modules, and their only real use is in the canvas api, so...

I see what you mean. I was thinking they'd belong to the graphics module directly instead of being children. That said I'm fine with leaving them in Canvas if they won't be used elsewhere.

@Parzival-3141
Copy link
Contributor Author

Unnamed enums

The only usage in api.json right now is OC_INPUT_TEXT_BACKING_SIZE, which doesn't have any siblings. This is basically just a const type with extra steps, and another edge case for bindings generators to handle. I think if you have a group of constants with a shared type, it should be a "real" (named) enum, otherwise they should be separate consts. I like the typename workaround as well.

For unicode range values, you'll have to come up with a schema for expressing values besides strings and primitive types.

What do you think of this?

{
    "kind": "const",
    "name": "OC_UNICODE_BASIC_LATIN",
    "type": {
        "kind": "namedType",
        "name": "oc_unicode_range"
    },
    "init": {
        "kind": "struct",
        "value": {
            "firstCodePoint": 0,
            "count": 127,
        }
    }
}

I'm pretty sure this can handle all the types in the API. Bindings generators should have enough information to parse the value field when they hit it. Unions can be represented by defining only the active field in the value map.

I'm curious to know of any other projects that have solved this problem.

Could you elaborate on "Differentiate bitflag enum types from normal enums"?

Sure. By marking enums intended to be used as bitflags (e.g. oc_file_open_flags_enum), a bindings generator could generate code specific to bitflags without having to define an override or match the enum-constants against a heuristic. We could reuse the normal enum syntax but just replace "kind": "enum" with "kind": "bitflags-enum".

@martinfouilleul
Copy link
Collaborator
martinfouilleul commented May 21, 2025 •
edited
Loading

Unnamed enums
The only usage in api.json right now is OC_INPUT_TEXT_BACKING_SIZE, which doesn't have any siblings. This is basically just a const type with extra steps, and another edge case for bindings generators to handle. I think if you have a group of constants with a shared type, it should be a "real" (named) enum, otherwise they should be separate consts. I like the typename workaround as well.

Ah I remember why I put this as an enum instead of a const, it's because MSVC doesn't support using a const as an array size... We could replace them with const in the api.json, but that would make it hard to check api.json against the C headers. I agree this is quite a dumb reason, but I'm not sure how to work around it... In the meantime we can just give a name to that enum though.
Edit: we can probably remove this and use const globals when we merge the zig build system, since at this point we will be using clang (well, zig cc) across all platforms

What do you think of this?

Yes this looks good! Maybe we could have some shorthand for initializing oc_str8 from string literals (and differentiate from null terminated strings) but otherwise I think it will cover all our usecases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Parzival-3141 Parzival-3141

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Parzival-3141 @martinfouilleul
0