Expose API for setting window size / title from Orca apps #47

New Issue

MartinFouilleul · 2023-08-21T15:25:56Z

MartinFouilleul commented

2023-08-21 15:25:56 +00:00

No description provided.

rdunnington commented

2023-08-21 23:09:01 +00:00

What sort of API are you thinking? Do you want to share the oc_window_* API between the runtime API and the internals? Or have a simpler one for the runtime, similar to how the oc_surface_* API works?

I'll propose the latter so we have something concrete to talk about:

// Internal API
void oc_window_set_title(oc_window window, const char* title);
void oc_window_set_size(u32 width, u32 height);

// Runtime API - shims that are exposed to wasm and call the internal API using the global __orcaApp
// These would be exposed to wasm without the _stub suffix
void oc_window_set_title_stub(const char* title);
void oc_window_set_size_stub(u32 width, u32 height);

What sort of API are you thinking? Do you want to share the `oc_window_*` API between the runtime API and the internals? Or have a simpler one for the runtime, similar to how the `oc_surface_*` API works? I'll propose the latter so we have something concrete to talk about: ```c // Internal API void oc_window_set_title(oc_window window, const char* title); void oc_window_set_size(u32 width, u32 height); // Runtime API - shims that are exposed to wasm and call the internal API using the global __orcaApp // These would be exposed to wasm without the _stub suffix void oc_window_set_title_stub(const char* title); void oc_window_set_size_stub(u32 width, u32 height); ```

MartinFouilleul commented

2023-08-22 12:03:46 +00:00

Following the same kind of conventions as other "indirect" bindings (e.g. oc_runtime_log), we could have a native side oc_runtime_window_set_title()/oc_runtime_window_set_size() that calls the internal oc_window_set_title()/oc_window_set_size(), and the wasm3 stub can be generated from core_api.json.

Also maybe it would be more consistent if we'd pass an oc_str8 for the window title? (and also a bit easier to check in the binding code)

Following the same kind of conventions as other "indirect" bindings (e.g. `oc_runtime_log`), we could have a native side `oc_runtime_window_set_title()`/`oc_runtime_window_set_size()` that calls the internal `oc_window_set_title()`/`oc_window_set_size()`, and the wasm3 stub can be generated from core_api.json. Also maybe it would be more consistent if we'd pass an `oc_str8` for the window title? (and also a bit easier to check in the binding code)

MartinFouilleul commented

2023-08-22 12:08:24 +00:00

Btw I'm not super happy with the oc_runtime_xxx naming conventions for "functions exposed by the runtime that just marshall arguments and forwards to internal APIs". But the _stub suffix is already taken by the auto-generated wasm3 binding code.
Feel free to propose another naming if you have a better idea!

Btw I'm not super happy with the `oc_runtime_xxx` naming conventions for "functions exposed by the runtime that just marshall arguments and forwards to internal APIs". But the `_stub` suffix is already taken by the auto-generated wasm3 binding code. Feel free to propose another naming if you have a better idea!

rdunnington commented

2023-08-22 23:17:27 +00:00

This sounds good, I wasn't thinking about the potential conflict with the stub name. I'll take a crack at this today/tomorrow.

bvisness commented

2023-08-23 02:42:51 +00:00

Bad naming ideas to spur better naming ideas:

ocr_window_set_title (OrCa Runtime)
_oc_window_set_title (or __oc_window_set_title)
oc_window_set_title_ (even better)
OC_window_set_size (like Go but cooler)

More seriously, I think it's fine for the naming to be a bit more verbose as long as users don't see the clumsier names.

Also seriously, I think it's a bit odd that the binding would go oc_window_set_title -> oc_runtime_window_set_title -> oc_window_set_title. Seems like it would make more sense for the internal-est function to have a different name, and for the stub and the WASM import to share the same name? (Perhaps I'm misunderstanding.)

Bad naming ideas to spur better naming ideas: - `ocr_window_set_title` (OrCa Runtime) - `_oc_window_set_title` (or `__oc_window_set_title`) - `oc_window_set_title_` (even better) - `OC_window_set_size` (like Go but cooler) More seriously, I think it's fine for the naming to be a bit more verbose as long as users don't see the clumsier names. Also seriously, I think it's a bit odd that the binding would go `oc_window_set_title` -> `oc_runtime_window_set_title` -> `oc_window_set_title`. Seems like it would make more sense for the internal-est function to have a different name, and for the stub and the WASM import to share the same name? (Perhaps I'm misunderstanding.)

bvisness added this to the Jam MVP milestone 2023-08-23 02:49:18 +00:00

rdunnington self-assigned this 2023-08-23 03:06:56 +00:00

ilidemi commented

2023-08-23 03:18:46 +00:00

Or oc_internal_window_set_title

Or `oc_internal_window_set_title`

MartinFouilleul commented

2023-08-23 09:40:04 +00:00

Also seriously, I think it's a bit odd that the binding would go oc_window_set_title -> oc_runtime_window_set_title -> oc_window_set_title. Seems like it would make more sense for the internal-est function to have a different name, and for the stub and the WASM import to share the same name? (Perhaps I'm misunderstanding.)

The situation is that some functions on the host and guest side "do the same thing" (hence the same name) but don't take exactly the same parameters (e.g. we don't expose the window API directly, so the window handle is implicit for orca apps), or some checks need to happen in between (e.g. for oc_io_wait_single_req we need to "re-root" absolute paths to the private data folder).

Other bad naming ideas:
oc_window_set_title_guard
oc_window_set_title_bridge

> Also seriously, I think it's a bit odd that the binding would go `oc_window_set_title` -> `oc_runtime_window_set_title` -> `oc_window_set_title`. Seems like it would make more sense for the internal-est function to have a different name, and for the stub and the WASM import to share the same name? (Perhaps I'm misunderstanding.) The situation is that some functions on the host and guest side "do the same thing" (hence the same name) but don't take exactly the same parameters (e.g. we don't expose the window API directly, so the window handle is implicit for orca apps), or some checks need to happen in between (e.g. for `oc_io_wait_single_req` we need to "re-root" absolute paths to the private data folder). Other bad naming ideas: `oc_window_set_title_guard` `oc_window_set_title_bridge`

rdunnington commented

2023-08-23 20:20:29 +00:00

For this change, I wound up going with

oc_runtime_window_set_title()
oc_runtime_window_set_size()
Since the names follow existing conventions. We can do a naming pass later if desired.

PR #61

For this change, I wound up going with * `oc_runtime_window_set_title()` * `oc_runtime_window_set_size()` Since the names follow existing conventions. We can do a naming pass later if desired. PR https://git.handmade.network/hmn/orca/pulls/61

rdunnington referenced this issue

2023-08-23 20:20:41 +00:00

window_size_title_api #61

rdunnington commented

2023-08-25 14:51:38 +00:00

Should we close this now that #61 is in?

Should we close this now that https://git.handmade.network/hmn/orca/pulls/61 is in?

MartinFouilleul commented

2023-08-25 14:58:54 +00:00

Oh yep, didn't see it wasn't closed automatically

MartinFouilleul closed this issue

2023-08-25 14:58:54 +00:00