Master issue for Table discussion

[andlabs]

This thread exists to collect all the requests for Table and coordinate merging the feature back into master now that I'm ready to focus attention on it.

This thread includes by proxy the discussion in #159.

@bcampbell what's your current status on your Table Windows code, and what does it do currently?

---

Table will include Tree eventually, but not initially.

Requests from the Go side:

• Table column widths
• Checkbox part OnToggled
• Options to "activate" rows with double-clicks
• Consider also providing stripped-down listboxes like on Windows as an option (maybe)

Other requests:

• Hyperlink columns

[bcampbell]

@andlabs: I've just brought my branch up-to-date: https://github.com/bcampbell/libui/tree/table
(I've only tested it on Linux so far, but other than the uiFree/Alloc/Realloc rename it wasn't affected by the big utflib-and-attrstr merge).

All three platforms support:

• single or multi-select (set at creation time)
• an OnSelectionChanged callback
• an interface for iterating through the set of selected items
• a noddy example app (examples/table)

The windows version supports only text columns (being based on the rather limited win32 commctl listview).

For me, the outstanding issues are:

1. Some column-width management (not sure what API we'd want for this, if any. Maybe it should all be automatic - with random sampling of rows for big models...)
2. a click event on column headers (eg to pick a column to sort by)
3. the ability to sort by a particular column (with an indicator to show sort column and direction)

But I'm sure everyone would have their own wishlist here...
For me, it's really useful already, as it stands.

Would opening a pull request against the main table branch be a good place to start?

[andlabs]

Sure! The other features you named can always be added later.

I also wonder what your stance on the "part" system currently is.

[bcampbell]

Regarding the part system, we probably need to step back and see what is possible.

The ideal case:

You can define custom layouts for use in cells. QML's listview delegate is the one I've used.
(and see #159 (comment) for a screenshot example, from the windows explorer search)
I'm not sure what the support for such custom layouts is on Cocoa or Gtk+, and it'd certainly take a load of work on the windows side.
I think this is a good long-term aim.

But In the meantime, the existing parts API will certainly scratch a few people's itches.
I'm inclined to think that uiTableAppendTextColumn() should be the only officially-blessed method. All the other parts functions can be left in for everyone who really needs them, marked with a big "here be dragons!" to make it clear that:

1. they're not currently supported on windows
2. they might be replaced in future with a more general API

[bcampbell]

(just to keep everything linked up...)
I'm breaking up my uiTable hacking into smaller chunks. First one is now up, which just adds bare-bones windows table support:

#361

I'll move back on to selection handling etc when the dust settles.

[andlabs]

https://www.codeproject.com/Articles/35197/Undocumented-List-View-Features
OH GOD DAMN IT MICROSOFT THIS IS EXACTLY WHAT WE NEEDED ALL ALONG

I am seriously tempted to say "screw it, this is too valuable to be ignored just because it's undocumented" and use it anyway. I'm not sure if I'll be able to use this directly because it seems to take over entire subitems and for the current parts system wouldn't allow that, but it's still miles better than the alternatives (change libui to LGPL for mCtrl or spend a lot of time rewriting my own table control again)! I wonder how this interacts with @bcampbell's suggestions about the parts system.

Now if only there was an equivalent that also supported tree views with multiple columns and full accessibility support...

[andlabs]

[22:23:21]  <+GerbilSoft>	fair warning: windows 7 has a *different* GUID for IListView
[22:23:55]  <+GerbilSoft>	https://github.com/GerbilSoft/rom-properties/blob/master/src/libwin32common/sdk/IListView.hpp
[22:23:56]  <+GerbilSoft>	https://github.com/GerbilSoft/rom-properties/blob/master/src/libwin32common/sdk/IOwnerDataCallback.hpp
[22:23:58]  <+GerbilSoft>	this has both
[22:24:42]  <+GerbilSoft>	the win7 one does work for win10 though
[22:24:45]  <+GerbilSoft>	and 8

[22:26:23]  <+GerbilSoft>	i have no idea why IListView is still undocumented
[22:26:30]  <+GerbilSoft>	it's effectively part of the ABI now that everyone uses it

[22:28:44]  <+GerbilSoft>	the win7 one adds a function, EnableAlphaShadow()
[22:28:51]  <+GerbilSoft>	...right in the middle of other functions, thus shifting the vtable
[22:29:06]  <+andlabs>	yes, COM requires you to change the GUID in that case
[22:29:08]  <+GerbilSoft>	if it was at the end of the vtable, then they could've done IListView2 instead
[22:29:17]  <+GerbilSoft>	which inherits from IListView

(cc @GerbilSoft)

[bcampbell]

Wow. The ListView Control seems to be the control that just keeps on growing. I fully expect it to gain support for it's own process management, scheduling and device drivers...

Anyway.

At a very brief reading, it looks to me like their ListView can now support arbitrary control layouts, via the ISubItemCallback interface. I think GTK+ and Cocoa can do this too.

I'd guess that this 'undocumented' stuff is already in so much use that it may as well be an ABI. My suspicion is that it is, in fact, more-or-less documented, but only from the .NET (and maybe WinRT) point of view. Microsoft seem to be actively discouraging win32 development these days.

Lots more speculation follows:

There are a whole heap of existing ISubItemCallback controls available (hyperlinks, star ratings, calendar etc), where we could vastly enrich the libui table Parts system. But I don't think this is the ideal way to go - you end up with a two-tier layout system. What you really want is to be able to use a full libui layout to display a cell.
I'm pretty sure there'll be a way of doing this, but I've no idea how the message pump and hwnd stuff would link in.
I think you'd pass in a 'factory' struct, with functions set up for creating, populating and destroying (and editing) layouts, and detailing how the uiTableModel data maps to it. Behind the scenes, libui would wrap this factory up as a COM implementation for use in the IListView.

If IListView is anything like it's counterparts in other GUI toolkit (eg QtQuick/QML), it'll cache and reuse the control layouts, so you should require many more than the number of rows displayed.

So. How does this all affect the libui table API?

Firstly, 'appending' multiple parts to a column probably isn't the right model any more.
You'd instead want to be able to set a single custom layout for each column.
Obviously you'd have shortcuts for common cases - eg a plain text column.

I think a full custom-cell-layout system is a great goal to aim for, if it's possible.
So I'd be inclined to hold back the parts API (or at least mark it 'turbulence likely') for now.
Just implement text columns, which we'd want a shortcut for anyway, no matter what other fancy stuff we go with...

[andlabs]

All right. I'm still not taking any immediate action on the parts system; after I get this PR merged in, I want to go back through both this and your main criticism and see what to do about it.

The factory idea is something I've been thinking about when I do eventually get to headerbar-type controls, because on Windows if I use rebars and on Mac OS X with layout entirely I would be breaking the current rule that each control has a single handle made when created. It's something I'll need to think about for those.

As for this being undocumented, my only theory is that the interface is a mess that did change in COM-violating ways. It just makes me wonder what kept exposing this functionality some other way from beng considered worth exposing (such as via the -100 point system...).

[andlabs]

Also GTK+ uses cell renderers, not real widgets, and cell layouts that operate like GtkBox/uiBox. Cocoa has something like this, but most things now use view-based table views, where you can just shove a bunch of controls into a table cell. You do have to create the controls for every visible row in the table, but the system is smart and lets you reuse existing rows if they aren't in use. Interface Builder has some helpers for all this, but I forget what they are.

[bcampbell]

Just for reference: discussion about the Gtk+ cell layouts, and the (future) possibility to extend them to handle generic widget layouts: https://wiki.gnome.org/Projects/GTK+/GtkTreeView/Ideas#Renderer_objects

[andlabs]

Okay, tested accessibility of that demo program in the CodeProject link. Keyboard accessibility shows we can navigate through subitems, but not interact with them? Assistive technologies accessibility (aka automation) is even weirder: Inspect.exe acts as if the subitems aren't there unless we have already started editing one, but UI Spy does but we can't do anything with them? Definitely needs investigation.

I wonder if it's a 32-bit vs 64-bit issue, since the download included is 32-bit. Would need to build another 64-bit version with some changes I made locally reverted. Perhaps I could also test the 32-bit version of Inspect.exe and UI Spy, though I doubt they would affect things...

[andlabs]

Okay, going to analyze and respond to @bcampbell's stuff on table parts now, because I'm trying to redo the messy OS X table implementation and the parts system is inconsistent in ways that will affect the implementation changes I want to make, so maybe instead of making it consistent there's something better in these suggestions. They are

• Table planning: loose ends #159 (comment)
• https://github.com/bcampbell/libui/blob/5f960109eb6c30716a093340cec36f72eb986a59/TABLE_NOTES.md
• Master issue for Table discussion #310 (comment)

(I thought there was another page somewhere...) My response will be the next comment. Sorry for the delay in this!

[parro-it]

Oh sorry, I missed the link... on GH I only see half window header, but on imgur it's ok...

[msink]

On Windows7, when text size set to 125% - edited text does not fit:

And program crashes after exit.

[msink]

It was Kotlin version:

import libui.*

fun main(args: Array<String>) = appWindow(
    title = "Table",
    width = 800,
    height = 480
) {
    val model = TableModel {
        var row9text = "Part"
        var yellowRow = -1
        var checkStates = IntArray(15)
        val image0 = Image(16.0, 16.0) {
            add(image0_16x16, 16, 16, 64)
            add(image0_32x32, 32, 32, 128)
        }
        val image1 = Image(16.0, 16.0) {
            add(image1_16x16, 16, 16, 64)
            add(image1_32x32, 32, 32, 128)
        }

        numColumns { 9 }
        columnType { when (it) {
            3, 4 -> uiTableDataTypeColor
            5    -> uiTableDataTypeImage
            7, 8 -> uiTableDataTypeInt
            else -> uiTableDataTypeString
        }}
        numRows { 15 }
        getCellValue { row, col -> when (col) {
            0 -> TableDataString("Row $row")
            1 -> TableDataString("Part")
            2 -> TableDataString(if (row == 9) row9text else "Part")
            3 -> when (row) {
                     yellowRow -> TableDataColor(RGBA(1.0, 1.0, 0.0))
                     3         -> TableDataColor(RGBA(1.0, 0.0, 0.0))
                     11        -> TableDataColor(RGBA(0.0, 0.5, 1.0, 0.5))
                     else      -> null
                 }
            4 -> if ((row % 2) == 1) TableDataColor(RGBA(0.5, 0.0, 0.75)) else null
            5 -> if (row < 8) TableDataImage(image0) else TableDataImage(image1)
            6 -> TableDataString("Make Yellow")
            7 -> TableDataInt(checkStates[row])
            8 -> when (row) {
                 0    -> TableDataInt(0)
                 13   -> TableDataInt(100)
                 14   -> TableDataInt(-1)
                 else -> TableDataInt(50)
            }
            else -> null
        }}
        setCellValue { row, col, value -> when (col) {
            2 -> if (row == 9) row9text = value!!.string
            6 -> {
                val prevYellowRow = yellowRow
                yellowRow = row
                if (prevYellowRow != -1)
                    rowChanged(prevYellowRow)
                rowChanged(yellowRow)
            }
            7 -> checkStates[row] = value!!.int
        }}
    }

    add(widget = VerticalBox {
        padded = true
        add(stretchy = true, widget = Table(model, {
            textColumn("Column 1", 0, uiTableModelColumnNeverEditable)
            imageTextColumn("Column 2", 5, 1, uiTableModelColumnNeverEditable, 4)
            textColumn("Editable", 2, uiTableModelColumnAlwaysEditable)
            setRowBackgroundColorModelColumn(3)
            checkboxColumn("Checkboxes", 7, uiTableModelColumnAlwaysEditable)
            buttonColumn("Buttons", 6, uiTableModelColumnAlwaysEditable)
            progressBarColumn("Progress Bar", 8)
        }))
    })
}

But libui test.exe crashes too.

[mischnic]

And program crashes after exit.

On macOS the address sanitizer isn't happy either.

Also, the default column title text color on macOS is black:

[msink]

Well,

$ gdb test.exe
GNU gdb (GDB) 8.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-w64-mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from test.exe...done.
(gdb) r
Starting program: F:\src\kotlin-libui\build\libui\out\test.exe
[New Thread 11772.0x2190]
[New Thread 11772.0x2d98]
warning: [libui] F:\src\kotlin-libui\libui\windows\alloc.cpp:25:uninitAlloc() You have a bug: Some data was leaked; either you left a uiControl lying around or there's a bug in libui itself. Leaked data:
0x2648e10 uiTableModel
0x50e5fe0 uiImage
0x50e7990 uiImage


Thread 1 received signal SIGTRAP, Trace/breakpoint trap.
0x000007fefcf531f3 in KERNELBASE!DebugBreak ()
   from C:\Windows\system32\KernelBase.dll

[andlabs]

Yes, the crash is the test program not being fully updated yet.

The proper size of the edit on Windows is a TODO. The real listview has a lot more space at the right edge than I do, but I can't figure out the source of that space. (Also remember that libui still has DPI awareness turned off. I want to do the changes in this and windows-namespace-and-hresult-cleanup before I switch that on.)

Also I wonder if the icons in 125% mode should still be 16x16 or not.

Not sure about column headers on Mac; need to check both that code and Interface Builder again.

And yes, totally need to run asan on all this :D

[msink]

Trying to fix memory leak, ran into another bug - uiFreeImage not implemented in OSX version.

[msink]

Will there be handlers for onClick/onDoubleClick/onKey ?
Some users do not want to edit in-place, prefer to open a Form instead.

[andlabs]

At some point in the future, yes.

[szanni]

Has any thought gone into sorting and filtering API wise?

GTK has GtkTreeModelSort and GtkTreeModelFilter, abstracting sorting and filtering from the underlying data model. It however seems to me that using these interfaces would just complicate things and make the implementation less flexible. On macOS the mechanism seem to operate directly on the data.

Ideas for sorting:

1. Provide a callback uiTableColumnHeaderClicked(int i) and let the user sort the underlying data model and refresh the table via something like uiTableReloadData()
2. Let the user provide a compare function for each column and sort internally uiTableRowCompare(int row1, int row2)
3. Let the user provide a compare function for the entire table and sort internally uiTableRowCompare(int col, int row1, int row2)

The interface for 2 and 3 might seem a little odd at first, but quite often more than once column is needed to sort properly. Possibly even data that is not shown in the table?

Ideas for filtering

1. User side filtering with a uiTableReloadData() function (no additional functionality needed in libui)
2. A uiTableRowIsVisible(char *filter, int row) callback function.
3. A uiTableFilter(char *filter) and uiTableRowIsVisible(int row) function

Again here for 2 and 3 I would go with a filter by row and not elements necessarily displayed in the table to allow for pre-processing.
I would personally probably go for 3, as pre-processing (diacritics stripping, string splitting) is probably important for most implementations and seems inefficient to do on every isVisible() call.

Comments?

[andlabs]

I could merge this as is now, apart from documentation (which hopefully shouldn't take long to write). However, I can slightly optimize things on GTK+ if I split the model Int type into a ModelBool and ModelProgress types. I'm not sure if I should now, should later (breaking things), or not at all...

[msink]

If you ask for personal opinions - it should be merged, if test-page16 works on all platforms.
On windows it works, on linux and mac I didnt check.

And one more very personal opinion - Table should be named TableView, and TableModel - just Table. Dont like that Model word...

[andlabs]

Tables are now in master.

[andlabs]

I forget, did I mark anything else as blocking Alpha 4? Because I might just go ahead and push that out now. I want to make some deeper changes next, which will make the Windows code (and hopefully all the other code too) more stable and responsive, and then I can do fun stuff like High DPI and text input =P

I'll get to the PRs I've been sitting on tomorrow.

[msink]

For me blocking is only #395

[simplexidev]

The only one (sort of) blocking for me is #364, and you have that one marked as blocking also.

[andlabs]

For CI I might change it to "will test live". It shouldn't hurt if I quickly remove bad files and manually upload fixed ones anyway. AppVeyor being slow can be dealt with separately.

For timer destruction I could just patch up uiUninit(), but again, you don't need to call that on program exit. The tester has it to ensure that libui cleans itself up properly.

[msink]

you don't need to call that on program exit. The tester has it to ensure that libui cleans itself up properly

But if it not called - some global system resources can be not disposed, at least on windows.

[andlabs]

Which global system resources stick around after the program exits? That doesn't seem right to me... Either way, fixing it now.

[bcampbell]

@andlabs: I meant to say: thanks for all your hard work on the new table code - It's much appreciated!
It's all looking great so far.

It's been a little busy here, but I'm finally getting around to collecting the other table changes I had. (First up, #413 )

[cweagans]

I know it's still a maybe, but I'd really love a stripped down table that I can use as a listbox. My use case also involves frequently replacing the entire data set and that's proven to be somewhat tricky. Any ideas about when/if either of those things might be possible?

[nowind]

consider nonheader style table as TableParams?

[AndyObtiva]

SetCellValue is not called on state change of an editable checkbox column in a table on OSX (whether a checkbox column or checkbox text dual-column).

Originally reported in Go UI (andlabs/ui#357), but I also encountered in Ruby LibUI, so it is a general libui issue.

[szanni]

SetCellValue is not called on state change of an editable checkbox column in a table on OSX (whether a checkbox column or checkbox text dual-column).

Fixed in #509 not merged though.

I actually have a collection of table related fixes table-fixes for anyone who might be interested.

I have also continued adding some new table related APIs in table-ng. This includes a more complete tester, row double click, header visible/invisible, header width get/set, and header sort indicators. All not breaking the existing API, although I would like to add an ID to uiTableAppendXColumn to identify them as soon as column reordering hits.

All of these should be in separate PRs, seems @andlabs is sadly busy with other things these days though... Any chance some of us could help with merging PRs and resolving issues via commit access? I know this does require a fair amount of trust but it would be amazing to get this project rolling again. Thanks for all the great work so far!