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

Implement uiTable #45

Open
mischnic opened this issue Aug 8, 2018 · 15 comments
Open

Implement uiTable #45

mischnic opened this issue Aug 8, 2018 · 15 comments

Comments

@mischnic
Copy link
Contributor

mischnic commented Aug 8, 2018

🙈

@parro-it
Copy link
Owner

Good news! I'm invacation rightnow. Will be back on end of august, I will start toadd tables to bindings.

@parro-it
Copy link
Owner

parro-it commented Sep 6, 2018

I'm back on the road again, and I started to work on the new implementation. As a first step, I did a PR where I changed the download process of libui binaries in order to use @andlabs repo

@mischnic
Copy link
Contributor Author

The icon in the table has bits of green at the top and bottom (on right is the original png file), are you handling transparency incorrectly (somehow?) or is this a libui bug?

bildschirmfoto 2018-09-28 um 19 21 03

@mischnic
Copy link
Contributor Author

libui issue: andlabs/libui#425

@mischnic
Copy link
Contributor Author

mischnic commented Oct 10, 2018

How is the progress? Did you implement all libui table features?


Regarding this todo:

libui-napi/src/image.c

Lines 8 to 11 in e3848cf

static void on_img_gc(napi_env env, void *finalize_data, void *finalize_hint) {
uiImage *img = (uiImage *)finalize_data;
/* TODO: uiFreeImage when? */
}

I guess the only way is reference counting when using an image in a table.

@andlabs
Copy link

andlabs commented Oct 10, 2018

uiTable does not copy images; they are used as soon as you give it to them, and then forgotten about.

@mischnic
Copy link
Contributor Author

mischnic commented Oct 10, 2018

That's true. The images are only used in the modelCellValue callback, so when the JS object gets garbage collected, the image can be deleted (no reference is stored inside native code).

@andlabs
Copy link

andlabs commented Oct 10, 2018

Oh. I thought I documented the guarantee on lifetime, but basically by the top of the next main loop iteration uiTable will have finished using it. I'm not sure whether creating a new uiImage each time you extract data is a good idea, but eh.

@mischnic
Copy link
Contributor Author

I'm not sure whether creating a new uiImage each time you extract data is a good idea, but eh.

What do you mean by extracting data? There is a javascript image wrapper which forwards the calls to the actual uiImage and when the wrapper would get gc'ed, the uiImage is freed as well. This underlying uiImage also gets passed to libui in the table callback.

@parro-it
Copy link
Owner

How is the progress? Did you implement all libui table features?

Almost, I have to finish the abstraction layer on top of models.
But basic libui methods binding should be all there. There are some memory management to improve on.... one is the one you discovered here, the other is in order to keep model instances alive while they are used by any or more tables.

I guess the only way is reference counting when using an image in a table.

I was thinking the same, we can increment the reference when an image is returned by the model handler, and someway decrements it in next loop step, as @andlabs suggested...

Another strangeness to solve is here:

// free(cell_value);

AFAIK that string should be freed immediately, but if I do it there, the program SEGFAULT.

@mischnic
Copy link
Contributor Author

AFAIK that string should be freed immediately, but if I do it there, the program SEGFAULT.

Doesn't segfault for me, but according to ui.h:

uiTableValueString() returns the string stored in v. The returned string is owned by v.

So it's not copied and shouldn't get freed.

@jbrodriguez
Copy link

I'm about to build a desktop app and deciding wether to use electron or proton-native.

The only thing missing from a workable proton-native poc is lists/tables (as mentioned in kusti8/proton-native#99)

So I was wondering when do guys think this could become available ?

Early next year ? Or before the end of the year ?

@parro-it
Copy link
Owner

parro-it commented Nov 2, 2018

@jboodriguez : I definitely think before the end of the year.

Anyway, please be aware that I'm doing this in my spare time, for free.
So it greately depends on how I'll be tired/motivated/busy on other stuff.

Don't take that date as strict deadline, only as an informed guess...

@benbucksch
Copy link

If this helps as motivation: I'm trying out Proton Native, and without a table widget with selection (single and multi), I have no chance to build my app.

@benbucksch
Copy link

@parro-it : Here's how an email client without tables looks like: :-)
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants